fix: Create directories from envs_dirs if they do not exist

[holzman]

Fix #3790.
Fix #3836.

[holzman]

Ah, my bad. I'll revise the failing tests and update this PR.

[holzman]

Actually, I think this exposed an issue with #3692:

In 2.0.5, -r took precedence over MAMBA_ROOT_PREFIX:

$ export MAMBA_ROOT_PREFIX=/tmp/envroot
$ mamba create  --debug -n foo --print-config-only | yq '. | .envs_dirs'
[
  "/tmp/envroot/envs"
]

$ mamba create -r /tmp/cliroot --debug -n foo --print-config-only | yq '. | .envs_dirs'
[
  "/tmp/cliroot/envs"
]

but in 2.0.6:

$ mamba create -r /tmp/cliroot --debug -n foo --print-config-only | yq '. | .envs_dirs'
[
  "/tmp/envroot/envs",
  "/tmp/cliroot/envs"
]

[jjerphan]

Merging main into this branch to get the changes of #3813 in.

[holzman]

I assume the Windows test is failing because it succesfully creates Path("/noperm") and can write to it (unlike in Linux) - I can check into it as soon as I get back to a Windows machine.

[jjerphan]

Thank you!

For your information, looking at the test logs:

    def test_create_envs_dirs(tmp_root_prefix: Path, tmp_path: Path):
        """Create an environment when the first env dir is not writable."""
        os.environ["CONDA_ENVS_DIRS"] = f"{Path('/noperm')},{tmp_path}"
        env_name = "myenv"
        helpers.create("-n", env_name, "--offline", "--no-rc", no_dry_run=True)
>       assert (tmp_path / env_name / "conda-meta" / "history").exists()
E       AssertionError: assert False
E        +  where False = exists()
E        +    where exists = (((WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_create_envs_dirs_False_0') / 'myenv') / 'conda-meta') / 'history').exists

/noperm is not used but C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_create_envs_dirs_False_0 (which should be the value of tmp_root_prefix) is.

[holzman]

    def test_create_envs_dirs(tmp_root_prefix: Path, tmp_path: Path):
        """Create an environment when the first env dir is not writable."""
        os.environ["CONDA_ENVS_DIRS"] = f"{Path('/noperm')},{tmp_path}"
        env_name = "myenv"
        helpers.create("-n", env_name, "--offline", "--no-rc", no_dry_run=True)
>       assert (tmp_path / env_name / "conda-meta" / "history").exists()
E       AssertionError: assert False
E        +  where False = exists()
E        +    where exists = (((WindowsPath('C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_create_envs_dirs_False_0') / 'myenv') / 'conda-meta') / 'history').exists

/noperm is not used but C:/Users/runneradmin/AppData/Local/Temp/pytest-of-runneradmin/pytest-0/test_create_envs_dirs_False_0 (which should be the value of tmp_root_prefix) is.

So this test was previously successful on both Windows and Linux because /noperm did not exist, so it then goes to the next env_dir and creates an env in tmp_path. My guess is that this test is now failing because this PR tries to os.mkdir("/noperm") first. That should fail on Linux for permissions reasons, but if it succeeds on Windows, it will create the env there rather than in tmp_path.

[jjerphan]

Ah yes, my bad, I misread the assertion.

[jjerphan]

In the meantime, #3836 has been reported.

Could we see if this PR fixes it by adding a non-regression test for it?

[holzman]

Ok, I’m a little stumped on this one; I rolled out a Windows dev environment and the test (as revised) passed…

[jjerphan]

@holzman: I tried several alternatives, but I think it would be worth having someone with a Windows machine reproduce and understand the error locally.

Feel free to remove / revert my commits.

[holzman]

Thanks @jjerphan - I did test in a Windows machine, unfortunately. Something must be subtly different about the environment in the actual Windows runner. I'll see if I can figure that out in a separate project; if it turns out it's impossible to make a read-only directory for the test, we should probably just skip the test for Windows.

[jjerphan]

LGTM given 🟢 CI.

I am waiting for a review from someone else in the team before merging.

[jjerphan]

Nitpick: slightly stricter assertions.

Suggested change

	for _ in range(2):
	cmd = ["-n", env_name, "--rc-file", tmp_home / ".condarc"]
	helpers.create(*cmd, no_rc=False)
	assert not Path(mamba_root_prefix_envs / env_name).exists()
	assert Path(condarc_envs_dirs / env_name).exists()
	for _ in range(2):
	assert not Path(mamba_root_prefix_envs / env_name).exists()
	cmd = ["-n", env_name, "--rc-file", tmp_home / ".condarc"]
	helpers.create(*cmd, no_rc=False)
	assert Path(condarc_envs_dirs / env_name).exists()

[Klaim]

create_directories shouldn't report errors when attempting to create an already existing directory, so you can remove that if and call create_directories every time (keep the error handling for the other potential errors).

[Klaim]

Do we really want to continue when an error occurs here?
Did you consider not catching the error so that that we stop immediately and let the caller handle the issue?