handling setenv field at package removal

[jhwoodyatt]

In 2.0.0~beta6, it seems that removing a package with a setenv field should entail use of eval $(opam env) to remove the environment updates that were previously applied by the install. I'm not seeing this in my experimentation.

[dbuenzli]

I recently stumbled on this aswell, in the following context: q does a setenv: and depends on p which has to compile without the setenv of q.

If you reinstall p (q will thus uninstall) then you expect the install of p to be done without the setenv of q. I didn't check but I suspect that this is the case.

However, the problem is that if you did an eval $(opam env) after the install of q then your user environment will have the effects of the setenv: of q. Since currently opam always adds the user environment to the compilation of packages, the reinstall of p will be done with the setenv of q.

It is not really possible for opam to undo the eval $(opam env) in your shell: you can only do this yourself.

In this this message @AltGr mentioned that in the future opam commands may run in cleaned up environments. With the advent of setenv: and the kind of problems highlighted above I think this would be a very sensitive thing to do eventually (and if only to improve build reproducibility and thus a potential caching layer).

This would mean to explicitly manage environment variables per opam switch and add opam env [current|set|get] --switch SWITCH to control them. opam would then only invoke commands in this tightly controlled environment.

However it could become painful to setup new switches (e.g. how to setup PATH, etc.) for this I would propose the following:

1. Only variables defined through the setenv: of installed packages are considered for inclusion in the environment of command run in a given opam switch. This would mean in a given repo e.g. having a base env package with a setenv: for PATH.
2. An opam environment variable has an initial value which is altered by the set of installed packages that are setenv:ing it in an arbitrary but deterministic an install-independent order (e.g. alphabetical).
3. The initial value of a variable is either empty or snapshoted from the user environment when a
   package installs with a setenv: for that variable with a new <update-op> called e.g. user.
4. If a package p setenv:s with user and a previous one already installed and setenv:ed the variable then p has no effect on the variable: either the initial value is empty or it was snapshoted by the previous package via user.
5. An opam var set VAR VAL always simply sets the initial value of VAR in the switch (VAR may not have been setenv:'d by a package yet, in this case the first package to setenv: will consider VAL to be the initial value).
6. An opam var snapshot command allows to resnapshot the initial value of all opam variables from the user environment (i.e. sh -- opam var set VAR $(VAR) for VAR in the set of variables known to the switch).

[jhwoodyatt]

@dbuenzli, That's another excellent problem, and I'd suggest it would make a good issue to report separate from your comment here.

In my case, I just noticed that removing a package with a setenv: field doesn't remove the environment update it applies with eval $(opam env). Even after removing the package, you still get its environment update until the next time you use opam-init/init.sh.

What am I doing? I have a package, called omake-env, which updates the OMAKEFLAGS environment variable with -j%{jobs}% --force-dotomake --dotomake %{prefix}%/var/spool/omake, which is handy for keeping OMake from littering your source tree with .omc files. I was sort of expecting opam remove omake-env to tell me that I need to use eval $(opam env) to update my environment. I was sort of expecting that my environment would be updated. Was surprised, and I filed an issue. It's not that much of a hardship to endure, but I can see how the setenv: field is a bit perilous to use as a general purpose communication medium.

[dbuenzli]

Even after removing the package, you still get its environment update until the next time

Are you sure you don't simply see the inability to undo environment updates once you eval $(opam env)'d with an install of omake-env ?

For example:

> opam --version
2.0.0~beta4
> opam env | grep BLA
> opam add pin setenv https://gist.github.com/11901650a3eba800c20ed586b616b639.git --yes
[...]
> opam env | grep BLA
BLA='BLA'; export BLA;
> opam remove setenv
> opam env | grep BLA
>

Now of course if I eval $(opam env) after the pin install I will have BLA defined in my environment until I unset it myself, no eval $(opam env) could reliably unset things.

[AltGr]

It is not really possible for opam to undo the eval $(opam env) in your shell: you can only do this yourself.

Not entirely true: opam will attempt to revert the changes, as far as possible ; same as it does when you switch over to another switch and run opam env. It will even do it across different opam roots (using the OPAM_SWITCH_PREFIX variable). You can try it with opam env --revert. However:

• some changes can't be undone (e.g. overwrite) without saving the original state, and since we can't assume that the environment isn't changed outside of opam, even that's not an option. So reverting changes can be approximate or destroy initial values.
• there is a limitation where opam can't unset variables and will instead set them to the empty string. This can be annoying sometimes and should be fixed (not done yet, because the code to reliably set variables across shells was long and hard enough to get by¹, and I didn't want to add other primitives just yet).

So @jhwoodyatt is actually right. However, the issue is difficult to solve: what opam does is, at time t, check the expected changes in the environment from the switch with the current environment. However, once a package has been removed, there is nothing anymore in the switch that can make opam remember that some changes had been its own doing and should be reverted. Since we can't update the environment in the same opam run that does the removal, we would need to remember that info, and somehow tie it to shells initialised while the package was installed... This seems tedious. A good approximation could be, since the current environment updates are stored in <switch>/.opam-switch/environment, to store the previous environment in environment.bak, and to add checks that if environment.bak is current, then revert that rather than environment before applying the new changes.

This could advise the proper eval $(opam env) and have it work if you do it right away. However, if you do another removal in the meantime (or if the removals were done in a different shell), we're back to square one. So unless we add unique identifiers for given switch states, which really seems too much, that doesn't seem worth it.

Another solution would be to store the changes done by opam to the environment, in the environment itself, in some format. That would allow accurate selection of what to revert, if we can find a proper encoding, at the cost of a maybe very large environment variable (at the moment, OPAM_SWITCH_PREFIX is used to locate the proper environment file, that is then reverted). That may be worth it.

@dbuenzli: yes, I'm definitely planning some "sanitized environment" feature for an upcoming version (not yet in 2.0.0). I'll keep your comment in mind when I design that. For the reference, stacked setenv: are currently handled here, with little concern for multiple updates of the same variable (deterministic, but unspecified (ie reverse alphabetic), and without taking care of dependencies). Easy to improve though.

¹I'm looking at you, fish

[AltGr]

After some thought, I think that encoding the environment changes within the environment itself is the only way to keep some kind of consistency, so maybe it's the way we should go before making the rest more complicated than necessary. This would make opam env --revert revert the actual changes that were applied, instead of the expected changes assuming that opam env was current, and seems to be the only sane way to solve this.

So, to be more explicit, the idea would be that the environment file, which looks like this:

% cat ~/.opam/4.06.0/.opam-switch/environment 
OPAM_SWITCH_PREFIX      =       /home/lg/.opam/4.06.0   Prefix\ of\ the\ current\ opam\ switch
CAML_LD_LIBRARY_PATH    =       /home/lg/.opam/4.06.0/lib/stublibs      Updated\ by\ package\ ocaml-base-compiler
CAML_LD_LIBRARY_PATH    =       /home/lg/.opam/4.06.0/lib/ocaml/stublibs:/home/lg/.opam/4.06.0/lib/ocaml        Updated\ by\ package\ ocaml
CAML_LD_LIBRARY_PATH    +=      /home/lg/.opam/4.06.0/lib/stublibs      Updated\ by\ package\ ocaml
OCAML_TOPLEVEL_PATH     =       /home/lg/.opam/4.06.0/lib/toplevel      Updated\ by\ package\ ocaml
MANPATH =:      /home/lg/.opam/4.06.0/man       Current\ opam\ switch\ man\ dir
PATH    =+=     /home/lg/.opam/4.06.0/bin       Binary\ dir\ for\ opam\ switch\ 4.06.0

be encoded into a specific variable, that we read and revert before applying any new changes. What we currently do instead is read the appropriate environment file to revert it, but it has the issue that the file could have been modified in the meantime. Which is what happens here.

[dbuenzli]

Personally I would rather call it a day on opam env --revert and focus on the "sanitized environment" feature (somehow I prefer when someone tells me something won't work rather than it will work 98% of the time but fail in a very obscure and potentially time consuming way in 2% of the cases).

Isolating builds from the user environment (modulo snapshooting) seems a better strategy in the long term, especially for adding a potential build cache to opam for local switches.

[jhwoodyatt]

Are you sure you don't simply see the inability to undo environment updates once you eval $(opam env)'d with an install of omake-env?

@dbuenzli, yep... if I remove my omake-env package, the environment persists in the shell even after eval $(opam env). A new shell, however, doesn't get the environment update, of course.

[dra27]

This should be fully addressed in opam 2.2 by #5417 - please do re-open if that turns out to be incorrect.