Fix uninject removing shared dependencies from other injected packages #1694
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
When multiple injected packages share a dependency in the same pipx venv,
pipx uninjectcould uninstall that shared dependency, even if it is stillrequired by other injected packages. This could break the remaining packages.
This PR makes
uninjectmore conservative when cleaning up dependencies.Changes
In
uninject_dep, after uninstalling the injected package, we still computethe set of packages that became "not required" using
venv.list_installed_packages(not_required=True), but before uninstallingthem we:
The user-facing behaviour is unchanged when there are no shared dependencies,
but shared dependencies used by other packages in the venv are now preserved.
Added a regression test
test_uninject_keeps_shared_dependenciesto ensurethat uninjection of one injected package does not remove another injected
package from the same venv.
Added a changelog fragment describing the bugfix.
Testing
Local (Windows 11):
python -m nox -s tests-3.10 -- -k "uninject"Result:
uninject-related tests pass:covernox session fails locally because only a subset of tests is run(global coverage 56% < fail-under=70). I expect full coverage to be validated
by the CI when running the complete test suite.