Print a warning when mixing $EBPYTHONPREFIXES and $PYTHONPATH modules
#3521
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Flamefire
force-pushed
the
check-ebpythonprefixes
branch
2 times, most recently
from
2777a95 to
2323b76
Compare
boegel
changed the title
$EBPYTHONPREFIXES and $PYTHONPATH modules
boegel
reviewed
Dec 4, 2024
| conflicts = [dep['name'] for dep in self.cfg.dependencies() if is_in_pythonpaths(dep)] | ||
| if conflicts: | ||
| print_warning('The following modules set $PYTHONPATH but $%s is also used: %s\n' | ||
| 'Mixing such modules might cause unexpected results during the build of Python packages. ' |
There was a problem hiding this comment.
Should we use problems instead of results here?
There was a problem hiding this comment.
I wanted to be more generic because the result may not be a problem but still not what is expected or even intended
|
Looks OK to me at first glance, but I would like @Micket to pitch in here too... |
|
ping @Micket |
Micket
previously approved these changes
Dec 18, 2024
|
Hm, i wanted to test this in conjunction with --prefer-python-search-path, it would help me if this targeted 5.0.x instead for testing. |
$PYTHONPATH takes precedence which might lead to unexpected or wrong packages being used especially when a dependency includes a newer package than another dependency. As it is very hard to find this as the cause print a warning when this situation is found.
Flamefire
force-pushed
the
check-ebpythonprefixes
branch
from
2323b76 to
df5a827
Compare
|
Rebased and retargeted to 5.0x |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
$PYTHONPATH takes precedence which might lead to unexpected or wrong packages being used especially when a dependency includes a newer package than another dependency.
As it is very hard to find this as the cause print a warning when this situation is found.
I ran into this while building
scipyof theSciPy-bundlewhich includes patches that require a newerCythonas the build dependency then what is available in thePython-bundle-PyPImodule. As only Python-bundle used$PYTHONPATHthe package from theCythonmodule was ignored even thoughwhich cythonpoints to the right binary.The error wasn't helpful:
The least we can do is to detect such issues and warn the user.
The check is triggered for direct dependencies when both
$PYTHONPATHand$EBPYTHONPREFIXESare set and the Python module is loaded.I only check for modules that have a
lib/python*/site-packagesfolder and specifically exclude the Python module which sets$PYTHONPATHfor thesite-customize.pyThis should catch most false positives while still catching enough real issues