From 211c85db635e00d87e885c9b4d2326abad02fb21 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Mon, 11 Oct 2021 13:34:44 +0200 Subject: [PATCH 1/3] reftests: add set env test --- tests/reftests/dune.inc | 17 +++++++++++++++++ tests/reftests/env.test | 25 +++++++++++++++++++++++++ 2 files changed, 42 insertions(+) create mode 100644 tests/reftests/env.test diff --git a/tests/reftests/dune.inc b/tests/reftests/dune.inc index 5d2c72bf3a3..132a5152f76 100644 --- a/tests/reftests/dune.inc +++ b/tests/reftests/dune.inc @@ -135,6 +135,23 @@ %{targets} (run ./run.exe %{bin:opam} %{dep:dot-install.test} %{read-lines:testing-env})))) +(rule + (alias reftest-env) + (action + (diff env.test env.out))) + +(alias + (name reftest) + (deps (alias reftest-env))) + +(rule + (targets env.out) + (deps root-N0REP0) + (action + (with-stdout-to + %{targets} + (run ./run.exe %{bin:opam} %{dep:env.test} %{read-lines:testing-env})))) + (rule (alias reftest-init) (action diff --git a/tests/reftests/env.test b/tests/reftests/env.test new file mode 100644 index 00000000000..196cd0b17f7 --- /dev/null +++ b/tests/reftests/env.test @@ -0,0 +1,25 @@ +N0REP0 +### +opam-version: "2.0" +setenv: [ NV_VARS += "%{_:doc}%:%{_:share}%" ] +flags: compiler +### opam update + +<><> Updating package repositories ><><><><><><><><><><><><><><><><><><><><><><> +[default] synchronised from file://${BASEDIR}/REPO +Now run 'opam upgrade' to apply any package updates. +### opam switch create setenv nv + +<><> Installing new switch packages <><><><><><><><><><><><><><><><><><><><><><> +Switch invariant: ["nv"] + +<><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><> +-> installed nv.1 +Done. +### opam env | grep "NV_VARS" | '[:;]' -> '-' | '[:;]' -> '-' | '[:;]' -> '-' +NV_VARS='${BASEDIR}/OPAM/setenv/doc/nv-${OPAMTMP}/OPAM/setenv/share/nv'- export NV_VARS- +### opam exec -- opam env --revert | grep "NV_VARS" +### NV_VARS=/another/path +### opam env | grep "NV_VARS" | '[:;]' -> '-' | '[:;]' -> '-' | '[:;]' -> '-' +NV_VARS='${BASEDIR}/OPAM/setenv/doc/nv-${OPAMTMP}/OPAM/setenv/share/nv-/another/path'- export NV_VARS- +### opam exec -- opam env --revert | grep "NV_VARS" From a19c26cb2c2e7b480dcbaa4057b67425faa706ba Mon Sep 17 00:00:00 2001 From: David Allsopp Date: Thu, 7 Oct 2021 20:07:05 +0100 Subject: [PATCH 2/3] Fix reverting additions to PATH-like variables For environment variables which can be split (e.g. `PATH` on `:`), OpamEnv.unzip_to attempts to find the point in the variable at which a value was added. This is fine if a _single_ value was added, but fails if the addition was multiple values (for example, if a setenv instruction added two directories to `PATH` in one `+=`). This is fixed by first splitting the value being searched according to the same rule as the environmen variable and ensuring they all match in order. --- master_changes.md | 1 + src/state/opamEnv.ml | 30 +++++++++++++++++++++++------- 2 files changed, 24 insertions(+), 7 deletions(-) diff --git a/master_changes.md b/master_changes.md index 82319b5db6b..793abbf9635 100644 --- a/master_changes.md +++ b/master_changes.md @@ -114,6 +114,7 @@ users) ## State * Handle empty environment variable updates - missed cherry-pick from 2.0 [#4840 @dra27] * Repository state: stop scanning directory once opam file is found [#4847 @rgrinberg] + * Fix reverting environment additions to PATH-like variables when several dirs added at once [#4861 @dra27] ## Opam file format * diff --git a/src/state/opamEnv.ml b/src/state/opamEnv.ml index 0c890810edb..9237e417078 100644 --- a/src/state/opamEnv.ml +++ b/src/state/opamEnv.ml @@ -29,14 +29,30 @@ let join_var l = (* To allow in-place updates, we store intermediate values of path-like as a pair of list [(rl1, l2)] such that the value is [List.rev_append rl1 l2] and the place where the new value should be inserted is in front of [l2] *) -let unzip_to elt = - let rec aux acc = function - | [] -> None - | x::r -> - if x = elt then Some (acc, r) - else aux (x::acc) r + + +let unzip_to elt current = + (* If [r = l @ rs] then [remove_prefix l r] is [Some rs], otherwise [None] *) + let rec remove_prefix l r = + match l, r with + | (l::ls, r::rs) when l = r -> + remove_prefix ls rs + | ([], rs) -> Some rs + | _ -> None in - aux [] + match split_var elt with + | [] -> invalid_arg "OpamEnv.unzip_to" + | hd::tl -> + let rec aux acc = function + | [] -> None + | x::r -> + if x = hd then + match remove_prefix tl r with + | Some r -> Some (acc, r) + | None -> aux (x::acc) r + else aux (x::acc) r + in + aux [] current let rezip ?insert (l1, l2) = List.rev_append l1 (match insert with None -> l2 | Some i -> i::l2) From 36c9ce39e270e5212d07883046408e94885ea08f Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Mon, 11 Oct 2021 13:39:15 +0200 Subject: [PATCH 3/3] reftests: update set env test --- tests/reftests/env.test | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/tests/reftests/env.test b/tests/reftests/env.test index 196cd0b17f7..08f8e014cfe 100644 --- a/tests/reftests/env.test +++ b/tests/reftests/env.test @@ -18,8 +18,10 @@ Switch invariant: ["nv"] Done. ### opam env | grep "NV_VARS" | '[:;]' -> '-' | '[:;]' -> '-' | '[:;]' -> '-' NV_VARS='${BASEDIR}/OPAM/setenv/doc/nv-${OPAMTMP}/OPAM/setenv/share/nv'- export NV_VARS- -### opam exec -- opam env --revert | grep "NV_VARS" +### opam exec -- opam env --revert | grep "NV_VARS" | '[:;]' -> '-' | '[:;]' -> '-' | '[:;]' -> '-' +NV_VARS=''- export NV_VARS- ### NV_VARS=/another/path ### opam env | grep "NV_VARS" | '[:;]' -> '-' | '[:;]' -> '-' | '[:;]' -> '-' NV_VARS='${BASEDIR}/OPAM/setenv/doc/nv-${OPAMTMP}/OPAM/setenv/share/nv-/another/path'- export NV_VARS- -### opam exec -- opam env --revert | grep "NV_VARS" +### opam exec -- opam env --revert | grep "NV_VARS" | '[:;]' -> '-' +NV_VARS='/another/path'- export NV_VARS-