Fully revertible environment updates

[dra27]

This PR builds on the solution to #3411 in #3413. There are two bug-fixes in this PR, but as they operate on the same code, I've submitted them as one.

The first - which is the original problem in #3411 - is what happens when you rename a local switch. At present, this behaviour is seen:

mkdir /tmp/foo
cd /tmp/foo
opam switch create . --empty
opam env | grep ^PATH=

PATH should begin /tmp/foo/_opam/bin:. If rename the switch:

mv /tmp/foo /tmp/bar
cd /tmp/bar
opam env | grep ^PATH=

PATH still begins /tmp/foo/_opam/bin: instead of /tmp/bar/_opam/bin. The issue here is two-fold:

1. The switch environment file needs regenerating
2. The shell env_hooks invoke opam env --readonly

The fix in #3413 ensured that OPAM_SWITCH_PREFIX is always correct, as this allows the environment updates to be correctly reverted. At that time, further fixes were somewhat academic, as the moving a local switch of course breaks OCaml. With - hopefully - relocatable OCaml around the corner, moving local switches is a less academic problem, and so the first commits in this PR address this issue. The first commit is an alteration so that the environment instead of being written with a lock can be written atomically without a lock.

The next commit removes the --readonly flag from the env_hooks. The problem at present is that regenerating the environment file can take a noticeable amount of time, which can cause delays returning the prompt. It seems to be much better to trust that the only change opam env can ever make is to the environment file, and allow the hooks to update it if necessary.

Armed with those changes, the third commit then causes the environment file to be regenerated if it has been moved. This is trivially detected by ensuring that OPAM_SWITCH_PREFIX in the environment file is correct.

Finally, opam exec had inconsistent behaviour w.r.t. opam env, and the next commit unifies the behaviour of opam env and opam exec when the environment file is missing or needs regenerating.

The next commit addresses a shortcoming of setenv that so far has been brushed away with lint warning 56. If one creates a package foo.1 with setenv: FOO = "bar" and then:

opam install foo.1
eval $(opam env)
echo $FOO

then bar will be displayed (and opam install will save FOO=bar to the environment file). If one now issues:

opam remove foo
eval $(opam env)
echo $FOO

then opam will still display FOO=bar. The reason for this is that opam remove dutifully updates the environment file and removes the FOO=bar update from it. opam env must first revert existing environment changes, which it does by locating the environment file via OPAM_SWITCH_PREFIX. This of course now says nothing about FOO and so FOO is - incorrectly - left alone.

For this reason, warning 56 has discouraged the use of setenv outside packages flagged compiler. This issue doesn't exist as long as the base environment variables defined by compiler packages are the same.

The forthcoming Windows packages need to be put Visual Studio configuration into the switch settings via environment variables, which is where this arose from. If the Visual Studio configuration package is uninstalled, then it leaves behind a considerable number of updates to the environment which should instead be reverted.

The same issue can arise without Windows if a switch is removed with other terminals open which had run eval $(opam env) for them - in this case, OPAM_SWITCH_PREFIX points to a non-existent directory, which means that the environment cannot be reverted.

The fix I've implemented here is to add an additional environment variable to the environment OPAM_LAST_ENV and have this point to a file in /tmp which contains the actual environment variables. If for whatever this can't be written, then OPAM_LAST_ENV is not set, and the existing mechanisms are used. We rely on the fact /tmp is cleared out reboot to clean-up old files and use hashing both to allow sharing of the environment file caches and also co-existence of terminals with different switches set. Windows doesn't ever clear out its TEMP directory automatically, so some garbage collection code is added to clean up any files which are older than the system's uptime.

Note that this paves the way for accurate reverting of FOO = "bar", but I have not implemented that (yet). This could also be used to differentiate variables which should be unset vs empty.

• Test for foo.1 package with a setenv - reverts correctly on opam env

• Writing to a directory in the opam root instead of /tmp (with hoovering done in opam clean instead)

• queued on 5476

[AltGr]

Note that this paves the way for accurate reverting of FOO = "bar", but I have not implemented that (yet). This could also be used to differentiate variables which should be unset vs empty.

Yes !! Finally \o/

This is a long-time need and looks awesome. Two remarks:

• I struggle making certain that we get the correct behaviour when manually changing OPAM_SWITCH_PREFIX: if the LAST_ENV variable is left unchanged, won't opam mistake that for a renamed switch ?
• I am eager to see the next PR that writes the original version of the variables in addition to their changes, to enable more clever reverting 😁 . This should also be part of the hash computation (which might help with the point above, if there is really an issue there ?)

[rjbou]

Thanks! All good on the idea.
Just some code comments, and a more general one: it's better to use OpamFilneame; and OpamSystem instead of Filename and Unix directly from the client.

[kit-ty-kate]

These two lines don't seem to be needed anymore (their use was reverted in the last commit)

[kit-ty-kate]

The tests on Windows fail with:

diff --git a/_build/default/tests/reftests/env.test b/_build/default/tests/reftests/env.out
index 56132e0..c461795 100644
--- a/_build/default/tests/reftests/env.test
+++ b/_build/default/tests/reftests/env.out
@@ -120,8 +120,18 @@ The following actions will be performed:
 -> installed foo.1
 Done.
 ### opam exec -- sh -c "eval $(opam env); echo $FOO"
+sh: $'\r': command not found
+sh: $'\r': command not found
+sh: $'\r': command not found
+sh: $'\r': command not found
+sh: $'\r': command not found

[rjbou]

I reworked it, so i can't review anymore

[kit-ty-kate]

Thanks!