pythonCatchConflictsHook: scan $out, not sys.path (2) #287957
Conversation
DavHau
force-pushed
the
python
branch
2 times, most recently
from
ed5a679
to
7d978f2
Compare
ofborg
bot
added
10.rebuild-darwin-stdenv
|
Thanks! Hope to take a closer look this week. One thing we should most probably do before a merge is to handle |
I now made the hook compatible to all python 3 version we still support (from 3.8). Also tested that it works on pypy. After this, only python2.7 still uses the legacy hook, which seems fine, as it's only used for a handful of packages anymore. I feel mentioning the difference between python 2.7 and 3.x in the manual might confuse people more than it actually helps, but happy to change my mind on this. |
This changes the non-legacy version of pythonCatchConflictsHook to recursively scan the output of the target derivation as well as its propagatedBuildInputs for duplicate dependencies. Previously, we did scan sys.path but did prove problematic as it produced false positives i.e. when build-time dependencies of hooks - such as setuptools in pythonCatchConflictsHook itself - where mistakenly flagged as duplicates; even though the are not included in the outputs of the target dervation. As all python runtime-dependencies are currently passed via propagatedBuildInputs in nixpkgs, scanning that plus site-packages seems sufficient to catch all conflicts that matter at runtime and less likely to produce false positives. The legacyHook in catch_conflicts_py2.py needs to be migrated as well, if it's still needed.
DavHau
force-pushed
the
python
branch
2 times, most recently
from
8b0d495
to
eb39b6a
Compare
There was a problem hiding this comment.
Just to recap for readers: Python 2.7 would still scan sys.path instead of the actual outputs. As the motivation of this PR are false-positives which mostly occur if you use pyproject.toml without setuptools, this seems ok for me - but happy to hear more opinions or even suggestions to for documentation :)
Code LGTM!
delroth
added
the
12.approvals: 1
delroth
removed
the
12.approvals: 1
Based on #284067
Fixes #283695
cc @mweinelt @phaer @FRidh
Description of changes
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.