Skip to content
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

Provide --egg option to opt out --single-version-externally-managed #541

Merged
merged 3 commits into from May 24, 2012

Conversation

k4ml
Copy link

@k4ml k4ml commented May 22, 2012

Discussion here - #3 (comment)

@travisbot
Copy link

This pull request fails (merged 66ff8e0 into e0db44e).

@carljm
Copy link
Contributor

carljm commented May 22, 2012

The Travis CI failure is an unrelated intermittent failing test, ignore that, tests pass on this.

Re making this option work via config or env var, good news -- you don't have to do anything, it already works. All pip options automatically work as either env vars (in this case, set PIP_EGG=1) or config file options (egg=1).

I do notice though that there's error output later in the install method because pip can't find where to put its installed-files record. I think we should still include the installed-files record and place it at the top-level inside the unzipped egg. This also means we should use the always-unzip flag when installing as an egg, which I think we should do anyway; I'm not aware of any good justification for installing zipped eggs.

Other than those fixes, this looks ready for merge! Good work, thanks!

@k4ml
Copy link
Author

k4ml commented May 23, 2012

I can't find a clean way to get the egg dir from list of files in record_filename. One I can think of right now:-

diff --git a/pip/req.py b/pip/req.py
index 0e50672..c86030e 100644
--- a/pip/req.py
+++ b/pip/req.py
@@ -595,6 +595,9 @@ exec(compile(open(__file__).read().replace('\\r\\n', '\\n'), _
                 if line.endswith('.egg-info'):
                     egg_info_dir = line
                     break
+                if line.endswith('EGG-INFO/PKG-INFO'):
+                    egg_info_dir = line[:-len('EGG-INFO/PKG-INFO')]
+                    break
             else:
                 logger.warn('Could not find .egg-info directory in install record
                 ## FIXME: put the record somewhere

But since pip now can properly uninstall packages installed by easy_install, do we really need to keep installed-files.txt ? Packages installed with pip install --egg should be no different than the one installed by easy_install.

@carljm
Copy link
Contributor

carljm commented May 23, 2012

I'm not totally opposed to just leaving out installed-files.txt, but I think it's better to have a full record of installed files than not, even though that makes the installation slightly different than what easy_install would do. Pip makes a best-effort to uninstall packages installed by easy_install, but without a proper uninstall log it depends on cobbling together various metadata to try to find installed scripts, data files, etc. A real installed-files log takes all the guessing out.

I think finding it should be just as easy as the current code to find the egg-info dir; directories are included too, so we can just look for a line.endswith('.egg') in addition to line.endswith('.egg-info'), right?

@k4ml
Copy link
Author

k4ml commented May 23, 2012

There's no .egg dir in record_filename. Here's list of files that I get when iterating record_filename while installing argparse module:-

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/argparse.py

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/argparse.pyc

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/EGG-INFO/PKG-INFO

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/EGG-INFO/not-zip-safe

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/EGG-INFO/dependency_links.txt

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/EGG-INFO/SOURCES.txt

/home/kamal/python/env/pip/lib/python2.6/site-packages/argparse-1.2.1-py2.6.egg/EGG-INFO/top_level.txt

Also I found out that there's no always--unzip option to setup.py install command:-

http://python.6.n6.nabble.com/setuptools-zip-safe-as-command-line-option-td2003942.html

I've try the workaround setup.py easy_install -Z . as suggested in the thread. It work if I run it from command line but not through pip, it simply said no command easy_install.

@carljm
Copy link
Contributor

carljm commented May 23, 2012

Meh, that's unfortunate that there's no --always-unzip for setup.py install. In that case, I think it's just not worth worrying about installed-files.txt. But I would like to get rid of that error, so can you add the appropriate checks so pip doesn't try to look for a place for installed-files.txt when --egg is used?

@k4ml
Copy link
Author

k4ml commented May 23, 2012

Sorry, the commit message a bit wrong. Should be "do not write installed-files.txt if using install --egg".

@travisbot
Copy link

This pull request passes (merged becfb83 into e0db44e).

carljm pushed a commit that referenced this pull request May 24, 2012
Provide --egg option to opt out of --single-version-externally-managed
@carljm carljm merged commit 673c709 into pypa:develop May 24, 2012
@dstufft dstufft mentioned this pull request Apr 24, 2014
@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 5, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 5, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
auto-locked Outdated issues that have been locked by automation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

4 participants