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

don't assume egg-link corresponds to dist to uninstall #3904

Merged
merged 2 commits into from
Oct 29, 2016

Conversation

minrk
Copy link
Contributor

@minrk minrk commented Aug 11, 2016

by checking if develop_egg_link exists prior to checking dist_info,
the .egg-link file is attempted to be uninstalled,
without checking whether it corresponds to the dist scheduled to be uninstalled.

Lowering the priority of the egg-link uninstall fixes this.

To reproduce, use setuptools 25, or SETUPTOOLS_SYS_PATH_TECHNIQUE=raw, and:

pip install .
pip install -e . # doesn't remove previous installation (pending #3898)
pip uninstall packagename

With SETUPTOOLS_SYS_PATH_TECHNIQUE=raw, and without #3898, both installations are present and the 'regular' install has higher priority, and is the dist found to be uninstalled. The order of the elifs meant that the egg-link would attempt to be uninstalled, even though it doesn't correspond to that dist.

By lowering the priority, the egg-link is only removed if the dist to be removed isn't wheel-installed, which I think is more consistent with the other conditions in this switch.

A more rigorous fix, perhaps, would be to check directly that the egg-link corresponds to the dist-to-be-removed, and remove it from consideration if not. I'm not 100% sure how to do that, though. I suppose this would be approximately promoting the assert that's currently failing to a condition, which if met means the egg-link matches the dist, otherwise it should not be included in the uninstall.

@xavfernandez
Copy link
Member

A test demonstrating the failing case would be nice

@faassen
Copy link

faassen commented Aug 25, 2016

I also got bitten by this -- doing a pip install -r requirements.txt first installs some versions from PyPI, then doing pip install -e /my/source says it successfully installed the version that came out of requirements.txt, as opposed to be the newer project version. When I downgrade setuptools it starts working again.

Doing a pip uninstall first doesn't work for me. It still installs the wrong version. When I do pip uninstall after I've tried (unsuccessfully) to install with -e it gets even worse: I get a traceback.

AssertionError: Egg-link /home/faassen/projects/morepath does not match installed location of morepath (at /home/faassen/projects/welcome-bench/env/lib/python2.7/site-packages)

When I downgrade setuptools to version 24, it starts working again, even though it gives misleading output; it claims it installed the release version while really it did install the development checkout. But I've seen that behavior from pip before.

by checking if develop_egg_link exists prior to checking dist_info,
the `.egg-link` file is attempted to be uninstalled,
without checking whether it corresponds to the dist scheduled to be uninstalled.

Lowering the priority of the egg-link uninstall fixes this.
@minrk minrk force-pushed the egg-link-priority branch 2 times, most recently from 94e562a to 9482b9c Compare August 26, 2016 12:39
this used to fail with SETUPTOOLS_SYS_PATH_TECHNIQUE=raw
@minrk minrk force-pushed the egg-link-priority branch from 9482b9c to b8f31ac Compare August 26, 2016 12:40
@minrk
Copy link
Contributor Author

minrk commented Aug 27, 2016

@xavfernandez sorry for the delay. Test added.

@fungi
Copy link
Contributor

fungi commented Sep 1, 2016

The OpenStack community would love to see this (or something) merged to solve the editable installs problem introduced by setuptools 25.0.0. We only just now discovered that projects using tox with usedevelop=True in our CI have not actually been testing the source code we expected if something else has already dragged in a packaged version of them as a transitive dependency.

itsjeyd added a commit to open-craft/problem-builder that referenced this pull request Sep 12, 2016
XBlock is listed as a dependency in requirements files for xblock-sdk,
xblock-utils, and problem-builder itself. These files list different (but compatible)
versions of the XBlock repo. Because of that, XBlock gets installed multiple times
when installing dependencies for CircleCI builds.

Starting with setuptools 25.0.0, the behavior for installing packages on top of
existing installs changed; see pypa/setuptools#729
for an example issue that this change in behavior caused.

For problem-builder CI builds, the issue was that after installing all dependencies,
the egg link for XBlock ended up pointing to the installation of problem-builder itself,
creating a situation where the code that ships with XBlock was not accessible from modules
that depend on it. This caused imports for any classes defined in the XBlock repo to fail,
and CircleCI builds errored out.

To fix this, we temporarily pin setuptools to the most recent version
that does not exhibit this behavior (24.3.1).

An upstream fix seems to be underway (via pypa/pip#3904),
but it hasn't been merged/released yet.
@xavfernandez xavfernandez reopened this Oct 27, 2016
@xavfernandez xavfernandez merged commit c96f850 into pypa:master Oct 29, 2016
@xavfernandez
Copy link
Member

Thanks for your work 👍

@minrk minrk deleted the egg-link-priority branch November 3, 2016 07:38
@minrk
Copy link
Contributor Author

minrk commented Nov 3, 2016

@xavfernandez thanks!

@lock lock bot added the auto-locked Outdated issues that have been locked by automation label Jun 3, 2019
@lock lock bot locked as resolved and limited conversation to collaborators Jun 3, 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.

4 participants