-
Notifications
You must be signed in to change notification settings - Fork 762
ignore user-installed packages when running python in sanitycheck #21362
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
ignore user-installed packages when running python in sanitycheck #21362
Conversation
|
@boegelbot please test @ jsc-zen3 |
|
@boegelbot please test @ generoso |
|
@SebastianAchilles: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2340745407 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
|
@boegelbot please test @ jsc-zen3 |
|
@SebastianAchilles: Request for testing this PR well received on jsczen3l1.int.jsc-zen3.fz-juelich.de PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2342989130 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
|
@boegelbot please test @ generoso |
|
@Flamefire We should also add a check in the easyconfigs test suite to avoid that Are you up for looking into that in this PR? |
|
@boegel: Request for testing this PR well received on login1 PR test command '
Test results coming soon (I hope)... - notification for comment with ID 2344275155 processed Message to humans: this is just bookkeeping information for me, |
|
Test report by @boegelbot |
Running a plain `python -c ...` or `python -m ...` in ECs not using `PythonPackage` might pick up packages installed in `$HOME/.local/lib/python*`. As we haven't set `$PYTHONNOUSERSITE` in those ECs/EasyBlocks we need to pass `-s` to `python` to the same effect. This avoids unexpected failures or success caused by that user environment.
d8aa881 to
238f3de
Compare
Done, although it is not trivial. |
506d07a to
8fab322
Compare
|
Test report by @Flamefire Failures:
|
| # Make sure the user packages in $HOME/.local/lib/python*/ are ignored when running python commands | ||
| # For the EasyBlocks above this is handled automatically by setting $PYTHONNOUSERSITE | ||
| # Detect any code or module invocation (-m or -c), `python cc` and `python <filepath>` | ||
| python_re = re.compile(r'\bpython (-c|-m|cc|[^ ]*\w+.py) ') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Suggested in EasyBuild conf call:
- just define
$PYTHONNOUSERSITEglobally in framework; - or introduce a template to run
python -c -scommands
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And the other 2 variables too? Do we have anything like that already, if so where?
Enforcing that template might be tricky. And we can only have it python -s as we need -m, -c, cc and maybe others which makes using the template more typing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What other two variables do you mean exactly?
We have similar stuff, though not for Python yet, see for example setting of $CUDA_CACHE_DISABLE in easyblock.py
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is also $PIP_DISABLE_PIP_VERSION_CHECK and $XDG_CACHE_HOME and later $PIP_REQUIRE_VIRTUALENV, see https://github.com/easybuilders/easybuild-easyblocks/pull/3460/files#diff-254aa63061aafed567ef5cc25be7b809dc9a1e4e41b5de85c2e607ea95cf33cbR123
$CUDA_CACHE_DISABLE is set in the prepare_step which IIRC isn't run for sanity-check-only. I think we can set $PYTHONNOUSERSITE and $PIP_DISABLE_PIP_VERSION_CHECK without issues but I'm not sure where. $PIP_REQUIRE_VIRTUALENV would need to be set globally and after loading modules (including fake modules)
|
Test report by @Flamefire OR-Tools fails due to license issues, so OK |
|
@boegel @SebastianAchilles Can this be merged? Just found another instance of this issue being introduced by an updated EC |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM
|
Going in, thanks @Flamefire! |
Running a plain
python -c ...orpython -m ...in ECs not usingPythonPackagemight pick up packages installed in$HOME/.local/lib/python*.As we haven't set
$PYTHONNOUSERSITEin those ECs/EasyBlocks we need to pass-stopythonto the same effect.This avoids unexpected failures or success caused by that user environment.