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 conda in isolated mode only if opted in #4625

Merged
merged 4 commits into from
Nov 15, 2022

Conversation

jaimergp
Copy link
Contributor

@jaimergp jaimergp commented Nov 9, 2022

Description

After discussing #4604 in the community calls, we have come to the conclusion that the changes there contained might disrupt packaging workflows in conda-forge and other communities. Some reasons:

  • Isolated mode prevents PYTHON* variables from being used and we are unsure this behavior is propagated to subprocesses (like the ones running build.sh and bld.bat).
  • conda-forge-ci-setup sets PYTHONUNBUFFERED
  • Python-based activation scripts might rely on PYTHON* variables as well.
  • Since we are not sure and cannot test all the edge cases in time for the release, we are proposing a safer solution that better respects the default behavior in previous releases.
  • Since the affected recipes are only conda and its dependencies (a very small % of the whole packaging ecosystem), it makes more sense to not enable isolated unless requested explicitly via an environment variable.
  • This variable is CONDA_BUILD_ISOLATED_ACTIVATION. If set to a non-empty value, it will enable isolated mode for conda invocations.

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 Nov 9, 2022
@jezdez
Copy link
Member

jezdez commented Nov 9, 2022

pre-commit.ci autofix

@kenodegard
Copy link
Contributor

Can we just replace all python -m conda with the absolute path to the conda executable instead? Avoid the isolated mode issues and the new opt in condition.

@jaimergp
Copy link
Contributor Author

Maybe, but conda-build has always used a bare python, which might be different than what base's conda entry point has in its shebang (e.g. if you run conda-build from a non-base environment). At this point I am not too keen of placing new bets and offering more opportunities for breakage. Would you rather make this env var "private" as in _CONDA_BUILD_ISOLATED_ACTIVATION as a way of not establishing an API contract of sorts?

@jaimergp
Copy link
Contributor Author

================================== FAILURES ===================================
________________________________ test_convert _________________________________
[gw0] win32 -- Python 3.9.13 C:\Users\runneradmin\Miniconda3\python.exe
Traceback (most recent call last):
  File "D:\a\conda-build\conda-build\tests\test_cli.py", line 448, in test_convert
    main_convert.execute(args)
  File "D:\a\conda-build\conda-build\conda_build\cli\main_convert.py", line 117, in execute
    api.convert(f, **args.__dict__)
  File "D:\a\conda-build\conda-build\conda_build\api.py", line 292, in convert
    return conda_convert(package_file, output_dir=output_dir, show_imports=show_imports,
  File "D:\a\conda-build\conda-build\conda_build\convert.py", line 796, in conda_convert
    convert_from_windows_to_unix(file_path, output_dir, platform,
  File "D:\a\conda-build\conda-build\conda_build\convert.py", line 709, in convert_from_windows_to_unix
    update_lib_contents(directory, temp_dir, 'unix', file_path)
  File "D:\a\conda-build\conda-build\conda_build\convert.py", line 279, in update_lib_contents
    src_dir.rename(dst_dir)
  File "C:\Users\runneradmin\Miniconda3\lib\pathlib.py", line 1382, in rename
    self._accessor.rename(self, target)
PermissionError: [WinError 5] Access is denied: 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpb6ybnggj\\Lib' -> 'C:\\Users\\RUNNER~1\\AppData\\Local\\Temp\\tmpb6ybnggj\\lib'

Windows filesystem is usually case insensitive, so that's why it's failing. However, Windows might be mounting case-sensitive filesystems and in that case that would be a valid operation...

That said, I don't know why this is an error in this PR? I can't restart the failing jobs to see if it was a temporary issue.

@jezdez
Copy link
Member

jezdez commented Nov 10, 2022

@jaimergp restarted the build, this is related to #4619 which was passing and now it's not :-/

@jezdez jezdez added this to the 3.23.0 milestone Nov 10, 2022
@jaimergp jaimergp marked this pull request as ready for review November 14, 2022 09:07
@kenodegard
Copy link
Contributor

Maybe, but conda-build has always used a bare python, which might be different than what base's conda entry point has in its shebang (e.g. if you run conda-build from a non-base environment).

Hm, I'm not sure I understand. When conda-build is installed outside of the base environment, conda is also installed into this non-base environment. I'm suggesting we use the prefix for the environment that conda-build is installed into for this absolute path. If conda-build is installed in base it will point to the base conda, if it's installed elsewhere it'll point to that other conda install.

Would you rather make this env var "private" as in _CONDA_BUILD_ISOLATED_ACTIVATION as a way of not establishing an API contract of sorts?

Yes.

@jaimergp
Copy link
Contributor Author

@kenodegard Oh I see, what you mean now. I guess that would work as well. I am afraid we break something else though (shebangs, different paths across operating systems...)

Is it ok if for now we just do it with the (private) env var?

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.

This is an acceptable stopgap. I have opened a new issue to investigate/refactor this using context.conda_exe instead (#4628).

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.

Switching to private variable and updating snippet to indicate the current temporary nature of the current fix.

conda_build/build.py Outdated Show resolved Hide resolved
conda_build/build.py Outdated Show resolved Hide resolved
conda_build/build.py Outdated Show resolved Hide resolved
news/4604-run-conda-in-isolated-mode Outdated Show resolved Hide resolved
@jezdez jezdez added the in-progress issue is actively being worked on label Nov 15, 2022
@github-actions github-actions bot locked as resolved and limited conversation to collaborators Nov 16, 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.

5 participants