Roundup Tracker - Issues

Issue 2550898

classification
Title: recent setup.py change breaks installation of scripts without setuptools
Type: Severity: urgent
Components: Installation Versions: devel
process
Status: fixed Resolution:
Dependencies: Superseder:
Assigned To: jerrykan Nosy List: ThomasAH, ber, jerrykan, rouilj, techtonik, tonich
Priority: urgent Keywords:

Created on 2016-01-06 16:09 by ThomasAH, last changed 2016-06-20 09:04 by ThomasAH.

Messages
msg5414 Author: [hidden] (ThomasAH) Date: 2016-01-06 16:09
Installation is broken for me by:
changeset:   4963:cdfb1a3fb56f
user:        John Kristensen <john@jerrykan.com>
date:        Tue Feb 10 14:08:01 2015 +1100
summary:     Fix setup.py to allow "development mode" (issue2550866)

During installation on Debian wheezy I get:
/usr/lib/python2.7/distutils/dist.py:267: UserWarning: Unknown
distribution option: 'entry_points'

The result is that scripts are not installed into bin, so no
roundup-admin, no roundup-mailgw, etc.

A short search shows that to use this option, you need to use
setuptools, not just distutils. I haven't tested if installing
setuptools helps, but as this would be a new requirement, this should
not be introduced lightly.
msg5416 Author: [hidden] (ber) Date: 2016-01-06 16:18
If hg4963:cdfb1a3fb56f is an issue, maybe we could
introduce a code path that only uses the new code
if "pip" is used?
msg5420 Author: [hidden] (jerrykan) Date: 2016-01-07 06:54
So after digging into this a bit...


roundup defines its own 'build_script' located in:

  roundup/dist/command/build_scripts.py

The 'copy_scripts()' method takes a list of script file names via the
'scripts' keyword argument for 'setup()'. These file names are
essentially the desired names of the scripts to be installed (ie.
original script name with underscores replaced with hyphens and the
'.py' suffix dropped). The 'copy_scripts()' method performs some lookups
to find the original scripts located in:

  roundup/scripts/

manipulates the contents of the script and bit and then outputs it to
the 'bin/' directory.


All this works fine using either setuptools or distutils when runnning:

  python setup.py install


The issue arises when running 'pip install' with the '-e' option, ie:

  pip install -e .  # while in the project root dir

this invokes a 'develop' command that is only supported by setuptools,
so the same issue can be triggered by running:

  python setup.py develop

assuming the 'setup()' method in 'setup.py' is imported from
'setuptools' instead of 'distutils.core'

The 'develop' command invokes a different code path to install the
scripts which doesn't use the custom 'build_script' so it all falls in a
heap because setuptools doesn't know where to find the original scripts.



There are a couple of possible solutions:

1. perform a check in setup.py to see if 'develop' appears in
sys.argv[1:] and pass the 'entry_points' keyword argument into
'setup()', otherwise pass in 'scripts'.

This is a bit hacky but should work.


2. Rename the scripts in 'roundup/scripts/' to drop the '.py' suffix and
replace underscores with hyphens, and pass in the script names
(including the path) to 'setup()' in the 'scripts' keyword argument.
msg5422 Author: [hidden] (jerrykan) Date: 2016-01-07 13:36
A third option is to just revert hg4963:cdfb1a3fb56f for the v1.5.1
release and migrate to setuputils for the next release...

As far as I can tell the 'develop' command available in setuptools is
mainly used for doing a pseudo install of a package so that it appears
to be installed but still points to the original code to make it easier
for development/testing/debugging. This functionality probably isn't
really that useful in a release tarball.

Reading up a bit on distutils/setuptools it seems that distutils is all
but deprecated in favour of setuptools. Here is a good summary of the
"current" state of python packaging tools:

 
http://stackoverflow.com/questions/6344076/differences-between-distribute-distutils-setuptools-and-distutils2#14753678

As mentions on stackoverflow the official documentation[1] references
the Python Packaging User Guide[2] which recommends setuptools over
distutils (see the footnotes[3] for the reasoning)

[1] https://docs.python.org/2/distutils/
[2] https://packaging.python.org/en/latest/current/
[3] https://packaging.python.org/en/latest/current/#id13
msg5434 Author: [hidden] (jerrykan) Date: 2016-01-10 00:49
Given that no one else has voiced an opinion, I would suggest reverting
hg4963:cdfb1a3fb56f - I'll try to do it later today when I have a bit
more time.

I have created issue2550899 for tracking a possible migration to
setuptools after a v1.5.1 release.

@ThomasAH if you have any concerns about setuptools then please note
them in issue2550899 so we weigh them up when considering a migration.
msg5436 Author: [hidden] (techtonik) Date: 2016-01-10 07:17
It could save me some hours if I discovered it earlier. =) Preparing a 
proper fix right now, which is:

Move "scripts: scripts" from setup() to different section.

This will help, because:

1. entries there are not scripts, but module names (removes hack)
2. nobody will try to do anything on them
3. custom build_scripts will find its files easily
msg5438 Author: [hidden] (techtonik) Date: 2016-01-10 07:45
Raised issue upstream that setuptools develop should detect and ignore  
build_scripts steps if it doesn't know how to handle them.

https://bitbucket.org/pypa/setuptools/issues/480/commanddevelop-doesnt-
call-build_scripts
msg5439 Author: [hidden] (techtonik) Date: 2016-01-10 09:58
I don't see how to pass modules names to `build_scripts` in setup() call 
without scripts:scripts - it gives error for any unknown kwarg and I could 
not find any `user_*` kwarg to stuff it there so far.

http://stackoverflow.com/questions/34704106/custom-distribution-option-in-
setup-py
msg5440 Author: [hidden] (jerrykan) Date: 2016-01-10 11:58
On 10/01/16 18:17, anatoly techtonik wrote:
> Move "scripts: scripts" from setup() to different section.
>
> This will help, because:
>
> 1. entries there are not scripts, but module names (removes hack)
> 2. nobody will try to do anything on them
> 3. custom build_scripts will find its files easily

This would break standard behaviour of distutils. If you have a look at 
the distutils build_scripts.copy_scripts() in the standard library[1] 
you will see that the scripts kwarg is referenced. If possible I think 
we should stick as close as possible to how the standard library does 
things while we are using distutils.

As mentioned, there is no urgent need to support 'python setup.py 
develop' in the release (it a feature for developers), so I would 
suggest just reverting hg4963:cdfb1a3fb56f instead of trying to land 
some last minutes changes to setup.py that run the risk of introducing 
new bugs just before a release... especially if migrating to setuptools 
might end up making any new changes redundant anyway.

[1] 
https://github.com/python/cpython/blob/2.7/Lib/distutils/command/build_scripts.py#L53
msg5441 Author: [hidden] (techtonik) Date: 2016-01-10 12:29
> This would break standard behaviour of distutils. If you have a look at
> the distutils build_scripts.copy_scripts() in the standard library[1]
> you will see that the scripts kwarg is referenced. If possible I think
> we should stick as close as possible to how the standard library does
> things while we are using distutils.

Yes, but we already provide our own copy_scripts() function, and
everything else (including setuptools) should use get_source_files()
API.

https://github.com/python/cpython/pull/27/files

Also the standard behaviour of distutils is already broken, because of
module names instead of paths in `scripts`, which is also the reason
why "develop" command failed.

> As mentioned, there is no urgent need to support 'python setup.py
> develop' in the release (it a feature for developers), so I would
> suggest just reverting hg4963:cdfb1a3fb56f instead of trying to land
> some last minutes changes to setup.py that run the risk of introducing
> new bugs just before a release... especially if migrating to setuptools
> might end up making any new changes redundant anyway.

I want "develop" thing to go into this release, maybe without
generated scripts, because I myself felt a great urge to add
livereload to demo.py while debugging Roundup Jinja2 issue.

I am trying to reduce the risk to minimum. At least I am following
distutils code to see that I broke anything.

Migrating to setuptools will likely to break those old CentOS installs
with ancient Python versions and pip versions, so if needed I would
do this only on major release.

I am not going to abandon this, so release is imminent, just need
to solve this one without creating more technical debt than it is
already there in this issue.

By the way, one way to reduce that debt would be to document distutils
`build_scripts` behaviour - what is expected input, output, and when it
is expected to be called.
msg5442 Author: [hidden] (techtonik) Date: 2016-01-10 16:23
> Move "scripts: scripts" from setup() to different section.

This approach didn't work, because then build_scripts step is not executed 
at all by `setup.py install`.
msg5443 Author: [hidden] (techtonik) Date: 2016-01-10 17:21
On Sun, Jan 10, 2016 at 7:23 PM, anatoly techtonik
<issues@roundup-tracker.org> wrote:
>
>> Move "scripts: scripts" from setup() to different section.
>
> This approach didn't work, because then build_scripts step is not executed
> at all by `setup.py install`.

Found a way to override this with monkeypatching Distutils.has_scripts method
(we already did this monkeypatching for classifiers in Python 2.3)
msg5444 Author: [hidden] (techtonik) Date: 2016-01-10 23:30
Patch is ready. Will submit it tomorrow with a fresh head.
msg5445 Author: [hidden] (techtonik) Date: 2016-01-11 10:17
I got it working with "setup.py install", but it still fails with "pip". 
Debugging it met this bug in pip https://github.com/pypa/pip/pull/2689
msg5446 Author: [hidden] (ber) Date: 2016-01-11 10:39
On Monday 11 January 2016 at 11:17:33, anatoly techtonik wrote:
> Debugging it

Anatoly,
reverting the change and pushing this byond te 1.5.1 release
seems to right thing to do to me.
msg5451 Author: [hidden] (techtonik) Date: 2016-01-11 14:16
On Mon, Jan 11, 2016 at 1:39 PM, Bernhard Reiter
<issues@roundup-tracker.org> wrote:

> Anatoly,
> reverting the change and pushing this byond te 1.5.1 release
> seems to right thing to do to me.

I will likely to push it beyond the 1.5.1 release, but I am not sure I
will get an opportunity to finish it later, so give me a little more
time to get rid of this technical debt. I've already identified a few
issues in setuptools, so if everything goes right, the "pip install
--editable ." issue will be fixed in next setuptools release.
msg5456 Author: [hidden] (techtonik) Date: 2016-01-11 21:58
Fixed with 1.5.1 release by reverting issue2550866
msg5608 Author: [hidden] (rouilj) Date: 2016-06-19 01:26
Is this ticket the reason for the closed pip branch in the roundup repo?
msg5615 Author: [hidden] (ThomasAH) Date: 2016-06-20 09:04
It seems so:

$ hg log -b pip
changeset:   5023:5e2888db6c48
branch:      pip
user:        anatoly techtonik <techtonik@gmail.com>
date:        Mon Jan 11 14:43:20 2016 +0300
summary:     build_scripts: self.announce --> log.info because it is visible

changeset:   5022:9250620c7219
branch:      pip
user:        anatoly techtonik <techtonik@gmail.com>
date:        Mon Jan 11 01:28:45 2016 +0300
summary:     build_scripts: Fix long term bug with setting
self.target_platform

changeset:   5021:70dde3937c9b
branch:      pip
user:        anatoly techtonik <techtonik@gmail.com>
date:        Mon Jan 11 01:20:29 2016 +0300
summary:     setup.py: Remove legacy support code for Python<2.3

changeset:   5020:a4d7e495f2f6
branch:      pip
user:        anatoly techtonik <techtonik@gmail.com>
date:        Sat Jan 09 20:44:56 2016 +0300
summary:     Rollback cdfb1a3 - support "pip install --editable ."
(issue2550866)


The branch was merged. That's the reason you see "(inactive)", which just
means that all heads of the branch are part of other branches, i.e. there is
no unmerged development in them:

$ hg log -r 'children("pip")'
changeset:   5024:edf62f78605f
parent:      5019:9bb20454f409
parent:      5023:5e2888db6c48
user:        anatoly techtonik <techtonik@gmail.com>
date:        Mon Jan 11 17:43:37 2016 +0300
summary:     Merging reverted "pip develop" compatibility into default


If this bothers you, you could explicitly close the branch using:
hg commit --close-branch
History
Date User Action Args
2016-06-20 09:04:36ThomasAHsetmessages: + msg5615
2016-06-19 01:26:11rouiljsetnosy: + rouilj
messages: + msg5608
2016-01-11 22:00:57techtoniksetstatus: new -> fixed
2016-01-11 21:58:47techtoniksetmessages: + msg5456
2016-01-11 14:16:01techtoniksetmessages: + msg5451
2016-01-11 10:39:02bersetmessages: + msg5446
2016-01-11 10:17:33techtoniksetmessages: + msg5445
2016-01-10 23:30:17techtoniksetmessages: + msg5444
2016-01-10 17:21:42techtoniksetmessages: + msg5443
2016-01-10 16:23:58techtoniksetmessages: + msg5442
2016-01-10 12:29:44techtoniksetmessages: + msg5441
2016-01-10 11:58:41jerrykansetmessages: + msg5440
2016-01-10 09:58:06techtoniksetmessages: + msg5439
2016-01-10 07:45:28techtoniksetmessages: + msg5438
2016-01-10 07:17:20techtoniksetnosy: + techtonik
messages: + msg5436
2016-01-10 07:13:35techtoniklinkissue2550866 dependencies
2016-01-10 00:49:25jerrykansetmessages: + msg5434
2016-01-07 13:36:27jerrykansetmessages: + msg5422
2016-01-07 06:54:17jerrykansetmessages: + msg5420
2016-01-06 16:18:38bersetnosy: + ber
messages: + msg5416
2016-01-06 16:10:06ThomasAHlinkissue2550863 dependencies
2016-01-06 16:09:28ThomasAHcreate