New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

install_egg_info command causes the --record argument to 'install' to create a wrong entry #118

Closed
bb-migration opened this Issue Nov 27, 2013 · 19 comments

Comments

Projects
None yet
1 participant
@bb-migration

bb-migration commented Nov 27, 2013

Originally reported by: marcusva (Bitbucket: marcusva, GitHub: marcusva)


python setup.py install --record=<somefile>

will create a file, which keeps a list of all files being installed by the distutils command.
The install_egg_info class of setuptools also puts a directory into the list, which by definition of the --record argument is not allowed.

Package management tools, which rely on --record and assume only files to be installed, will break.

The offending line:

https://bitbucket.org/pypa/setuptools/src/b480829f68381870bf3a7fef1964b413d7dc3c32/setuptools/command/install_egg_info.py?at=default#cl-26

The same line is also available in distutils' install_egg_info class, the .egg-info in that class however is a plain file.

The fix should be easy enough by simply changing it to something like

self.outputs = []


@bb-migration

This comment has been minimized.

bb-migration commented Aug 27, 2014

Original comment by JesseWeinsteinZonar (Bitbucket: JesseWeinsteinZonar, GitHub: Unknown):


Confirmed. Specifically, the error messages I see are:

#!shell

running install_egg_info
running egg_info
writing the_project.egg-info/PKG-INFO
writing top-level names to the_project.egg-info/top_level.txt
writing dependency_links to the_project.egg-info/dependency_links.txt
reading manifest file 'the_project.egg-info/SOURCES.txt'
writing manifest file 'the_project.egg-info/SOURCES.txt'
Copying the_project.egg-info to /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info
running install_scripts
creating /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project
creating /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project/bin
copying build/scripts-2.7/the_project -> /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project/bin
copying build/scripts-2.7/serve.py -> /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project/bin
changing mode of /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project/bin/the_project to 755
changing mode of /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILDROOT/the_project-0.2.0-1.el6.x86_64/the_prefix/the_project/bin/serve.py to 755
writing list of installed files to 'INSTALLED_FILES'
+ /usr/lib/rpm/find-debuginfo.sh --strict-build-id /home/jweinstein/the_project/build/bdist.linux-x86_64/rpm/BUILD/the_project-0.2.0
+ /usr/lib/rpm/check-buildroot
+ /usr/lib/rpm/redhat/brp-compress
+ /usr/lib/rpm/redhat/brp-strip-static-archive /usr/bin/strip
+ /usr/lib/rpm/redhat/brp-strip-comment-note /usr/bin/strip /usr/bin/objdump
+ /usr/lib/rpm/brp-python-bytecompile
+ /usr/lib/rpm/redhat/brp-python-hardlink
+ /usr/lib/rpm/redhat/brp-java-repack-jars
Processing files: the_project-0.2.0-1.el6.noarch
warning: File listed twice: /the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info/PKG-INFO
warning: File listed twice: /the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info/SOURCES.txt
warning: File listed twice: /the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info/dependency_links.txt
warning: File listed twice: /the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info/not-zip-safe
warning: File listed twice: /the_prefix/python/python-2.7/lib/python2.7/site-packages/the_project-0.2.0-py2.7.egg-info/top_level.txt

I'll try and make a pull request soon.

@bb-migration

This comment has been minimized.

bb-migration commented Aug 28, 2014

@bb-migration

This comment has been minimized.

bb-migration commented Sep 26, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Fix issue #118: Prevent the egg-info directory from being redundantly included in the list of modified files.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


In #262, the patch for this issue seems to be implicated in breaking installations on pip. Perhaps this is a good reason to accept the approach of @Ivoz instead.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


6.0.2 backs out the patch for this issue. The motivations for it need to be revisited.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


which by definition of the --record argument is not allowed.

Can you site a reference for this claim? Why should record not include directories?

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by marcusva (Bitbucket: marcusva, GitHub: marcusva):


Standard distutils setup.py:

# python setup.py --help install
Common commands: (see '--help-commands' for more)

[...]
Options for 'install' command:
[...]
  --record            filename in which to record list of installed files

The --record option mentions files, not directories. It might be worth to get into touch with the distutils and pip maintainers about that, to create
a unique behaviour (see also http://legacy.python.org/dev/peps/pep-0376/ for pip RECORD files)

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by JesseWeinsteinZonar (Bitbucket: JesseWeinsteinZonar, GitHub: Unknown):


In https://pythonhosted.org/setuptools/easy_install.html it says:

--record=FILENAME (New in 0.5a4) Write a record of all installed files to FILENAME. This is basically the same as the same option for the standard distutils “install” command, and is included for compatibility with tools that expect to pass this option to “setup.py install”.

Note the word "files". I presume that was what Marcus was referring to.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by marcusva (Bitbucket: marcusva, GitHub: marcusva):


Exactly. easy_install/setuptools and distutils claim to have the same behaviour for --record.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by toshio (Bitbucket: toshio, GitHub: toshio):


In some cases, directories are merely a subset of files. However, since .egg-info is the only directory that's listed in record, that's probably not the intention here.

pip currently depends on the egg-info directory being explicitly listed in the record file but it could be changed to find PKG-INFO or another file within the egg-info directory instead: pypa/pip#2075

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by marcusva (Bitbucket: marcusva, GitHub: marcusva):


Personally, I'm also fine with having all directories recorded as single entries, as long as the behaviour is consistent (ideally even across all three major default packaging mechanisms).

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by toshio (Bitbucket: toshio, GitHub: toshio):


One note on changing pip, though -- Unless pip has a special-case for updating itself, people on Windows have to be able to get the updated pip before getting the updated setuptools. Otherwise pip won't be able to update itself. (On other platforms, pip should throw a non-fatal warning. On Windows, the OS will refuse to remove the open file so pip issues a traceback).

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by JesseWeinsteinZonar (Bitbucket: JesseWeinsteinZonar, GitHub: Unknown):


Yes, if it would break pip, we could simply remove the actual files from the --record output, and leave the directory. The warnings (from rpm) comes from having both the directory and its files listed, so removing either one would be fine. We'll just need to update the documentation.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by toshio (Bitbucket: toshio, GitHub: toshio):


I think removing the files would probably be a greater violation of the documented behaviour of --record, though.

For rpm specifically, you really want rpm to know about both the directories and the files (for instance, so that it can remove the directories when the package is uninstalled). The problem is in how you can specify that in your %files section. With both directories and files, you need to list the directories as "%dir /directory/path/" and the files as normal: " /file/path". With just files, you'd have to manually determine what directories to include (also as %dir). When I packaged for Fedora we didn't rely on --record at all. Some manually determined the filelist but most of us chose to sidestep these issues and just list the toplevel directories: "/usr/lib/python2.7/site-packages/pip/" and "/usr/lib/python2.7/site-packages/pip*egg-info/' for instance. That allowed rpm to recursively include all the files and subdirectories itself.

@bb-migration

This comment has been minimized.

bb-migration commented Sep 29, 2014

Original comment by marcusva (Bitbucket: marcusva, GitHub: marcusva):


On FreeBSD, we rely on --record to create automatic packaging lists. Directory handling is directly integrated into the package management tools (previously, we just removed the last part of the path entries to create a list of directories). The output of --record however must be consistent, so that we do not have to introduce hacks to circumvent potentially wrong behaviour.

@bb-migration

This comment has been minimized.

bb-migration commented Feb 7, 2016

Original comment by koobs (Bitbucket: koobs, GitHub: koobs):


Upstream pip has fixed their side of what caused the 'issue' last time this was correctly fixed in setuptools.

pypa/pip#2076

Should be good to proceed now.

@bb-migration

This comment has been minimized.

bb-migration commented Feb 7, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Merge backout of 1ae2a75724bb; fixes #118.

@bb-migration

This comment has been minimized.

bb-migration commented Feb 7, 2016

Original comment by jaraco (Bitbucket: jaraco, GitHub: jaraco):


Released as 20.0.

@bb-migration

This comment has been minimized.

bb-migration commented Feb 9, 2016

Original comment by koobs (Bitbucket: koobs, GitHub: koobs):


\o/

Thank you!

PS: Next we fix install_data ;)~

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment