Skip to content

bugfix: make _source_single_file work in venvs#14569

Merged
tgamblin merged 4 commits intospack:developfrom
alalazo:fix/environment_mods_fail_in_venvs
Jan 25, 2020
Merged

bugfix: make _source_single_file work in venvs#14569
tgamblin merged 4 commits intospack:developfrom
alalazo:fix/environment_mods_fail_in_venvs

Conversation

@alalazo
Copy link
Copy Markdown
Member

@alalazo alalazo commented Jan 20, 2020

See issue #14568

Currently unit tests for Spack don't run cleanly under venvs. This aims at fixing the issue while not regressing on #14252 and #14491. Bonus if anybody could suggest a way to test this patch for all these issues.

@alalazo alalazo added the bugfix Something wasn't working, here's a fix label Jan 20, 2020
@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jan 20, 2020

@zzotta @s-sajid-ali

# and read the JSON back in the parent process to update os.environ
module_cmd += ' > /dev/null; PYTHONHOME="{0}" "{1}" -c "{2}"'.format(
sys.prefix, sys.executable, py_cmd)
module_cmd += ' >/dev/null;' + sys.executable + ' -c %s' % py_cmd
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@adamjstewart I'm not sure this needs changing, as probably it's always called before the modifications from packages are applied. Need to double check though.

@adamjstewart
Copy link
Copy Markdown
Member

Oops, I’ve never used venvs before, only Spack Envs. You could search for python3, python2, python.

@alalazo
Copy link
Copy Markdown
Member Author

alalazo commented Jan 20, 2020

Oops, I’ve never used venvs before, only Spack Envs. You could search for python3, python2, python.

I don't think it's 100% bulletproof, but the reasoning in this PR is:

  1. Using python is safe for all cases where a python is in the PATH, since Spack installed Python packages always provide a python executable
  2. If we don't find python (e.g. RHEL8 when we don't build a package that has python in the DAG) then use sys.executable

@adamjstewart
Copy link
Copy Markdown
Member

since Spack installed Python packages always provide a python executable

This is only true for python@2 and python@3+pythoncmd, not python@3~pythoncmd

@tgamblin
Copy link
Copy Markdown
Member

Related to #11989.

Now we search for `python3`, `python`, `python2`
before resorting to `sys.executable`.
@tgamblin
Copy link
Copy Markdown
Member

I think we should follow this on with some consideration of what we were trying to do with #11989 (i.e. we should remember the LD_LIBRARY_PATH, etc. needed to run sys.executable and run sys.executable with its own settings, instead of trying to rely on a Python in a spack-created environment). But this PR is better than what we have now and I think we should merge it as-is and deal with that later.

@tgamblin tgamblin changed the title Fix for environment modifications tests in venvs bugfix: make _source_single_file work in venvs Jan 25, 2020
@tgamblin tgamblin merged commit 4d7d657 into spack:develop Jan 25, 2020
@alalazo alalazo deleted the fix/environment_mods_fail_in_venvs branch January 25, 2020 08:32
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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

bugfix Something wasn't working, here's a fix impact-medium

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants