-
Notifications
You must be signed in to change notification settings - Fork 3.1k
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
Conversation
A test demonstrating the failing case would be nice |
I also got bitten by this -- doing a Doing a
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.
94e562a
to
9482b9c
Compare
this used to fail with SETUPTOOLS_SYS_PATH_TECHNIQUE=raw
9482b9c
to
b8f31ac
Compare
@xavfernandez sorry for the delay. Test added. |
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. |
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.
Thanks for your work 👍 |
@xavfernandez thanks! |
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:
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.