Fix case-preserving environment updates on Windows #5356
Merged
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There's existing code (of mine) which tries to deal with the fact that environment variable names are case insensitive on Windows. In
OpamStd.Env
, this is dealt with largely correctly inget
, but the handling inOpamEnv
is not. When dealing with environment updates, it's necessary to transform the names of any updates - i.e.PATH += "foo"
needs to be transformed toPath += "foo"
to use the same case as the base environment.The problem is manifesting itself principally with
PATH
, which by default is actuallyPath
on Windows. When reverting the updates inOpamEnv.get_pure
(for example, to evaluateeval_variables
), the switch'sbin
directory is correctly ignored when resolving the command, but when the command is executed, it re-emerges, because of a duplicate instruction in the env block for bothPath
(with the switch directory, as that comes from the calling environment) andPATH
(as determined inOpamEnv.get_pure
, which reverts the updates previously made).In particular, with this branch,
opam config list
stops reporting the switch's compiler settings insys-ocaml-arch
,sys-ocaml-cc
, andsys-ocaml-libc
, but I still need to mull some more whether this is strictly the correct approach (and the environment update code is already nightmarishly complicated even on Unix...)