Skip to content
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

Run Python in isolated mode unless in dev mode #4604

Merged
merged 6 commits into from
Nov 3, 2022

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Oct 19, 2022

Description

conda build generates some shell scripts wrappers to run build.sh and bld.bat. This includes setting a number of environment variables and activating the host and build environments in a stacked way. To do this, it employs python -m conda shell.posix hook.

However, the -m semantics implies that the current working directories (SRC_DIR for conda-build!) and other user installation paths are considered as part of sys.path. This is usually ok, but there can be corner cases:

  • If we are packaging conda and the version being packaged on SRC_DIR happens to change how shell.posix hook works, it might produce different code than what the released conda-build expects.
  • Not sure, but if we are packaging a dependency needed during the hook generation steps, it might also break something?

This discrepancy of which conda is used (the one on SRC_DIR vs the one installed next to conda-build) introduces a fragile behavior, but it can be easily fixed by just adding -I (isolated mode) to prevent leakage from non-installed locations:

-I
Run Python in isolated mode. This also implies -E and -s. In isolated mode sys.path contains neither the script’s directory nor the user’s site-packages directory. All PYTHON* environment variables are ignored, too. Further restrictions may be imposed to prevent the user from injecting malicious code.

-m <module-name>
Search sys.path for the named module and execute its contents as the main module.
...
-I option can be used to run the script in isolated mode where sys.path contains neither the current directory nor the user’s site-packages directory. All PYTHON* environment variables are ignored, too.

Why?

This bug has been discovered at conda/conda#11995. The sequence goes as:

  • Without this PR, conda-build uses python -m conda for its shell integration. This means that it first finds conda the working directory (SRC_DIR) executes the hook as produced by the version being packaged.
  • If we package Use python -m conda instead of PREFIX/bin/conda in shell functions  conda#11995, the hook will generate shell code with the form of python -I -m conda ... instead of CONDA_PREFIX/bin/conda (the Python entry point) as the Python command to avoid shebang issues.
  • The -I flag will force the next conda activate commands to run from the conda installation placed next to conda-build, not the one from the working directory!
  • In this particular timing, we are mixing two shell code generation strategies. The former one uses CONDA_PREFIX/bin/conda and unsets _CE_M and _CE_CONDA (the shell variables that hold -m and conda strings, respectively).
  • This causes the hook code to be run like python shell.posix activate, where it should have been python -I -m conda shell.posix activate. Of course, shell.posix is interpreted as a path to a file that doesn't exist and Python errors out.

I know this is a corner case, but in my opinion, conda-build shouldn't rely on software being packaged as part of its implementation :)

Checklist - did you ...

  • Add a file to the news directory (using the template) for the next release's release notes?
  • Add / update necessary tests?
  • Add / update outdated documentation?

@conda-bot conda-bot added the cla-signed [bot] added once the contributor has signed the CLA label Oct 19, 2022
@jaimergp
Copy link
Contributor Author

The failure above was due to temporary connection issues, but I don't have permissions to issue a re-run there. Otherwise, ready for review!

@jaimergp jaimergp marked this pull request as ready for review October 20, 2022 17:22
@jaimergp
Copy link
Contributor Author

cc @conda/conda-core (there's no conda-build team, right?)

Copy link
Contributor

@kenodegard kenodegard left a comment

Choose a reason for hiding this comment

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

@jaimergp hehe, you ran into the same issue we've been running into during recent releases, but unfortunately, we found workarounds at the time, so further work was postponed:

conda_build/build.py Outdated Show resolved Hide resolved
conda_build/build.py Outdated Show resolved Hide resolved
'&& set CONDA_EXE={}'
'&& set CONDA_EXE={python_exe}'
'&& set CONDA_PYTHON_EXE={python_exe}'
'&& set _CE_I={}'
Copy link
Contributor

Choose a reason for hiding this comment

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

I want to change this to _CE_FLAGS? Is that possible, or will that result in too many other changes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

This PR follows the code changes suggested at conda/conda#11995. I'd definitely appreciate less env vars exported, but we need to start with that PR... and I don't know if we can just get rid of env vars like that, given the possible backwards compatibility issues.

conda_build/build.py Outdated Show resolved Hide resolved
@dholth
Copy link
Contributor

dholth commented Nov 1, 2022

Are the CI tests run in debug mode?

conda_build/build.py Outdated Show resolved Hide resolved
Co-authored-by: Ken Odegard <kodegard@anaconda.com>
@kenodegard kenodegard added the in-progress issue is actively being worked on label Nov 1, 2022
@kenodegard kenodegard merged commit 104a0f2 into conda:main Nov 3, 2022
@jaimergp
Copy link
Contributor Author

jaimergp commented Nov 3, 2022

Thanks!

@kenodegard kenodegard added this to the 3.23.0 milestone Nov 3, 2022
@github-actions github-actions bot added the locked [bot] locked due to inactivity label Nov 4, 2023
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 4, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
cla-signed [bot] added once the contributor has signed the CLA in-progress issue is actively being worked on locked [bot] locked due to inactivity
Projects
Archived in project
Development

Successfully merging this pull request may close these issues.

4 participants