Fix use of sys.executable for module/env commands#14496
Merged
adamjstewart merged 4 commits intospack:developfrom Jan 16, 2020
Merged
Fix use of sys.executable for module/env commands#14496adamjstewart merged 4 commits intospack:developfrom
adamjstewart merged 4 commits intospack:developfrom
Conversation
Contributor
greenc-FNAL
approved these changes
Jan 15, 2020
Member
greenc-FNAL
left a comment
There was a problem hiding this comment.
Subject to #14252 (review) which I believe is still valid, I think this is good.
Member
Author
|
@chissg I completely agree, we should be running this without |
alalazo
added a commit
to alalazo/spack
that referenced
this pull request
Jan 20, 2020
This reverts commit 808c80d.
tgamblin
pushed a commit
that referenced
this pull request
Jan 25, 2020
Using `sys.executable` to run Python in a sub-shell doesn't always work in a virtual environment as the `sys.executable` Python is not necessarily compatible with any loaded spack/other virtual environment. - revert use of sys.executable to print out subshell environment (#14496) - try instead to use an available python, then if there *is not* one, use `sys.executable` - this addresses RHEL8 (where there is no `python` and `PYTHONHOME` issue in a simpler way
tgamblin
pushed a commit
that referenced
this pull request
Feb 7, 2020
* Fix use of sys.executable for module/env commands * Fix unit tests * More consistent quotation, less duplication * Fix import syntax
tgamblin
pushed a commit
that referenced
this pull request
Feb 7, 2020
Using `sys.executable` to run Python in a sub-shell doesn't always work in a virtual environment as the `sys.executable` Python is not necessarily compatible with any loaded spack/other virtual environment. - revert use of sys.executable to print out subshell environment (#14496) - try instead to use an available python, then if there *is not* one, use `sys.executable` - this addresses RHEL8 (where there is no `python` and `PYTHONHOME` issue in a simpler way
This file contains hidden or 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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
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.
@zzotta can you see if this fixes #14491 for you?
I'm not sure whether the quotes are necessary or not. I wanted to be robust in case
pythonis installed in a directory containing spaces. Honestly, we should probably rewrite all of this stuff so that it doesn't useshell=True, and then it shouldn't be necessary anymore.@s-sajid-ali @chissg