-
Notifications
You must be signed in to change notification settings - Fork 387
fix: Adapt root prefix' precedence for CONDA_ENVS_PATH #3852
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
Conversation
@@ -757,6 +758,7 @@ def test_env_dir_idempotence(tmp_path, tmp_home, tmp_root_prefix): | |||
assert Path(condarc_envs_dirs / env_name).exists() | |||
|
|||
|
|||
@pytest.mark.parametrize("conda_envs_x", ("CONDA_ENVS_DIRS", "CONDA_ENVS_PATH")) |
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.
We should also test having both of the environment variables set and check the precedence?
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.
Sure, I wasn't sure the behavior was defined, but I'll check what mamba 1 and conda do when both are set.
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.
Actually, setting both is not allowed in both conda and mamba 1, so the test is OK as written.
$ CONDA_ENVS_PATH=/tmp/envpath/envs CONDA_ENVS_DIRS=/tmp/envroot/envs mamba create --offline -n test1
Multiple aliased keys in file envvars:
- envs_path
- envs_dirs
Must declare only one. Prefer '_envs_dirs'
MultipleKeysError: Multiple aliased keys in file envvars:
- envs_path
- envs_dirs
Must declare only one. Prefer '_envs_dirs'
mamba 2 does allow both at the moment and CONDA_ENVS_PATH
takes precedence.
I can create a separate PR to prohibit having both environment variables set.
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.
Thank you for looking into it!
Hmm interesting, we should check indeed why these are not aliases in mamba 2 (but that would be in another PR).
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.
Makes sense. I started looking into it and realized there's another incompatibility in the syntax.
In conda and mamba1, both CONDA_ENVS_DIRS
and CONDA_ENVS_PATHS
are path-separator delimited (colon on Linux, semicolon on Windows).
In mamba2, CONDA_ENVS_DIRS
is comma-delimited instead.
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 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.
Thanks!
Fixes #3851.