[bug] venv modified vars do not take into account other venvs

[mjvankampen]

Environment Details (include every applicable attribute)

• Operating System+version: Windows 10
• Compiler+version: MSVC2019
• Conan version: 1.33.1
• Python version: 3.9.1

If you use multiple venv (such as activate & activate_run) at the same time errors might occur. When generating the venv files conan checks which env vars exists and treats those differently than new variables it will create when activating a venv.

When you activate two venv this assumption does not necessarily hold true, as a var might be new for activate, but when doing activate_run it is modified.

We could move the detection of new vs existing to the venv script itself. This way it does not matter when the venv scripts are generated and when they are run.

[ytimenkov]

First, which version do you use? venvs were changed several times and such behavior was (or at least tried) to be fixed.

Another thing, activating multiple envs is still prone to errors if you don't deactivate them strictly in reverse order.

That said, deactivating environment is anyways a big issue (especially when there are multiple ones) so the best way is to activate in another shell...

[mjvankampen]

Updated my original post!

The issue is that the venvs might introduce the same env var, such as Env:\DYLD_LIBRARY_PATH. These will now conflict as both venvs assume that this variable is new instead of being modified.

I think this can be fixed by moving this detection to time of activation of the venv instead of generation.

I'm willing to give it a go to fix this, but I want to see if someone sees something off with this as I don't want to put time in a PR that will strand :-). A (hacky) easy fix would be treating everything as modified instead of new.

[ytimenkov]

The issue is that the venvs might introduce the same env var, such as Env:\DYLD_LIBRARY_PATH. These will now conflict as both venvs assume that this variable is new instead of being modified.

Now I see...

I think this can be fixed by moving this detection to time of activation of the venv instead of generation.

This is actually how it is. As I understand the issue is that all generators (re)use the same name for "old" variable, so when you activate nested environment the "old" variable gets overwritten.

The workaround would be to "scope" "old" variables with generator name, i.e. use not CONAN_OLD_{{it}} but rather CONAN_OLD_{{venv_name | upper}}{{it}} in conans/client/envvars/environment.py.

At least you'll get better deactivation if you deactivate them in the reverse order.

However there are discussions to introduce a conan command to run a command inside environment (like docker exec) or merge runenv and virtualenv, but it breaks compatibility.

[memsharded]

Have you checked this? #8426

I am working on it, the idea is that the "activate" script is the one capturing the state to be restored, dynamically. I think this is the behavior that is expected, isn't it? The plan is to make the new environment the main model for the new toolchains approach.

[mjvankampen]

@ytimenkov looking at the code it seems like modified/new is detected at generation time not run? (for example

conan/conans/client/envvars/environment.py

Line 171 in 3694324

modified_vars = [name for name, _, existing in ret if existing]

) so if an env var does not exist yet all scripts will see it as new. Once you fix that you also need to scope the old one I agree. But I think memsharded approach is even nicer as you don't create new env vars

@memsharded ah yes exactly, looks good. Please don't forget powershell :). Any clue when this might go in? If it is in months a less nice fix in the current setup could also help.

[ytimenkov]

Ah, right, looking at evolution of virtualenvs code I assumed that it just stores all variables as in my initial attempt to fix it (#4248). Got confused by reading from .sh.env file and the template at the top of the file does save variables at the moment of activation. I think I naively assumed that it saves all variables it's going to set later on...

[mjvankampen]

@memsharded #8534 is in, but does not fix this issue. Say that activate and activate_run both modify LD_LIBRARY_PATH deactivating both of them will give an error as both the deactivate scripts will try to remove LD_LIBRARY_PATH from your environment.

[mjvankampen]

Excuse me I need to use the new generators.