From e0a5781e1eb92bd300869cba52404162db657932 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Thu, 23 Nov 2023 18:48:26 +0100 Subject: [PATCH 1/2] reftest: add OPAMSWITCH & OPAMROOT setting/unsetting test --- master_changes.md | 1 + tests/reftests/switch-set.test | 167 +++++++++++++++++++++++++++++++++ 2 files changed, 168 insertions(+) diff --git a/master_changes.md b/master_changes.md index cfa455f6b39..ccd753beef6 100644 --- a/master_changes.md +++ b/master_changes.md @@ -107,6 +107,7 @@ users) ## Reftests ### Tests * Add some additional test to tree, for `--dev` && `--no-switch` [#5687 @rjbou] + * switch-set: add test that checks unsetting `OPAMSWITCH` when it was set by `opam env --set-switch` on an already set `OPAMSWITCH` variable in environment [#5742 rjbou] ### Engine * Set `SHELL` to `/bin/sh` in Windows to ensure `opam env` commands are consistent [#5723 @dra27] diff --git a/tests/reftests/switch-set.test b/tests/reftests/switch-set.test index 5b674e54ee0..ed033e00ea9 100644 --- a/tests/reftests/switch-set.test +++ b/tests/reftests/switch-set.test @@ -75,3 +75,170 @@ Selecting opam switch -. Would select opam switch bar. ### opam switch show - +### : OPAMSWITCH & OPAMROOT specific handling on last env storage and reverts : +### opam sw create foo --empty +### +unset `env | grep OPAM | cut -f 1 -d = | grep -v OPAMROOT | grep -v OPAMNOENVNOTICE` +case $1 in + opam) + opam sw foo + ;; + env) + export OPAMSWITCH=foo + ;; +esac +echo "# current switch" +opam sw show +echo "# OPAMSWITCH current value" +echo $OPAMSWITCH +echo "# opam env" +echo "$(opam env)" | grep OPAMSWITCH +echo "# opam env with switch bar" +echo "$(opam env --set-switch --sw bar)" | grep OPAMSWITCH +eval $(opam env --set-switch --sw bar) +echo "# OPAMSWITCH current value" +echo $OPAMSWITCH +echo "# opam env" +echo "$(opam env)" | grep OPAMSWITCH +echo "# OPAMSWITCH current value" +echo $OPAMSWITCH +echo "# current switch" +opam sw show +echo "# evaluate opam env" +eval $(opam env) +echo "# OPAMSWITCH current value" +echo $OPAMSWITCH +echo "# current switch" +opam sw show +### sh set-switch.sh opam +# current switch +foo +# OPAMSWITCH current value + +# opam env +# opam env with switch bar +OPAMSWITCH='bar'; export OPAMSWITCH; +# OPAMSWITCH current value +bar +# opam env +OPAMSWITCH=''; export OPAMSWITCH; +# OPAMSWITCH current value +bar +# current switch +bar +# evaluate opam env +# OPAMSWITCH current value + +# current switch +foo +### sh set-switch.sh env +# current switch +foo +# OPAMSWITCH current value +foo +# opam env +# opam env with switch bar +OPAMSWITCH='bar'; export OPAMSWITCH; +# OPAMSWITCH current value +bar +# opam env +OPAMSWITCH=''; export OPAMSWITCH; +# OPAMSWITCH current value +bar +# current switch +bar +# evaluate opam env +# OPAMSWITCH current value + +# current switch +foo +### opam init --bare --bypass-checks --root ./foo ./REPO -n | grep -v Cygwin +No configuration file found, using built-in defaults. + +<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><> +[default] Initialised +### opam switch create sw-foo --root ./foo --empty +### opam init --bare --bypass-checks --root ./bar ./REPO -n | grep -v Cygwin +No configuration file found, using built-in defaults. + +<><> Fetching repository information ><><><><><><><><><><><><><><><><><><><><><> +[default] Initialised +### opam switch create sw-bar --root ./bar --empty +### +unset `env | grep OPAM | cut -f 1 -d = | grep -v OPAMNOENVNOTICE` +case $1 in + opam) + ARGF="--root foo" + ;; + env) + export OPAMROOT=${BASEDIR}/foo + ;; +esac +echo "# current root" +opam var root $ARGF +echo "# OPAMROOT current value" +echo $OPAMROOT +echo "# opam env" +echo "$(opam env $ARGF)" | grep OPAMROOT +echo "# opam env with root bar" +echo "$(opam env --set-root --root ./bar)" | grep OPAMROOT +eval $(opam env --set-root --root ./bar) +echo "# OPAMROOT current value" +echo $OPAMROOT +echo "# opam env" +echo "$(opam env)" | grep OPAMROOT +echo "# OPAMROOT current value" +echo $OPAMROOT +echo "# current root" +opam var root +echo "evaluate opam env" +eval $(opam env) +echo "# OPAMROOT current value" +echo $OPAMROOT +echo "# current root" +opam var root +### sh set-root.sh opam +# current root +${BASEDIR}/foo +# OPAMROOT current value + +# opam env +[NOTE] To make opam select ${BASEDIR}/foo as its root in the current shell, add --set-root or set OPAMROOT +# opam env with root bar +OPAMROOT='${BASEDIR}/bar'; export OPAMROOT; +# OPAMROOT current value +${BASEDIR}/bar +# opam env +OPAMROOT=''; export OPAMROOT; +# OPAMROOT current value +${BASEDIR}/bar +# current root +${BASEDIR}/bar +evaluate opam env +# OPAMROOT current value + +# current root +[ERROR] ${BASEDIR} exists, but does not appear to be a valid opam root. Please remove it and use `opam init', or specify a different `--root' argument +# Return code 50 # +### sh set-root.sh env +# current root +${BASEDIR}/foo +# OPAMROOT current value +${BASEDIR}/foo +# opam env +# opam env with root bar +OPAMROOT='${BASEDIR}/bar'; export OPAMROOT; +# OPAMROOT current value +${BASEDIR}/bar +# opam env +OPAMROOT=''; export OPAMROOT; +# OPAMROOT current value +${BASEDIR}/bar +# current root +${BASEDIR}/bar +evaluate opam env +# OPAMROOT current value + +# current root +[ERROR] ${BASEDIR} exists, but does not appear to be a valid opam root. Please remove it and use `opam init', or specify a different `--root' argument +# Return code 50 # From 7f5f0556f1f575ab7d2d7a0c9b628fb0f25fa7b0 Mon Sep 17 00:00:00 2001 From: Raja Boujbel Date: Mon, 11 Dec 2023 18:12:31 +0100 Subject: [PATCH 2/2] env: don't store 'OPAMSWITCH' and 'OPAMROOT' in last env, they should not be reverted They are set by '--set-switch' and '--set-root' with the idea to have them defined for the session --- src/client/opamConfigCommand.ml | 10 +++++++++- tests/reftests/switch-set.test | 22 ++++++++-------------- 2 files changed, 17 insertions(+), 15 deletions(-) diff --git a/src/client/opamConfigCommand.ml b/src/client/opamConfigCommand.ml index 83bcca450f7..ccae3369d26 100644 --- a/src/client/opamConfigCommand.ml +++ b/src/client/opamConfigCommand.ml @@ -297,7 +297,15 @@ let ensure_env_aux ?(base=[]) ?(set_opamroot=false) ?(set_opamswitch=false) not (String.equal upd.envu_var "OPAM_LAST_ENV")) updates in - let last_env_file = write_last_env_file gt switch updates in + let last_env_file = + write_last_env_file gt switch + (* We remove OPAMSWITCH & OPAMROOT as they are not supposed + to be reverted *) + (List.filter (fun upd -> + not ((String.equal upd.envu_var "OPAMSWITCH") + || (String.equal upd.envu_var "OPAMROOT"))) + updates) + in let updates = OpamStd.Option.map_default (fun target -> (env_update_resolved "OPAM_LAST_ENV" Eq diff --git a/tests/reftests/switch-set.test b/tests/reftests/switch-set.test index ed033e00ea9..190d27eda42 100644 --- a/tests/reftests/switch-set.test +++ b/tests/reftests/switch-set.test @@ -121,16 +121,15 @@ OPAMSWITCH='bar'; export OPAMSWITCH; # OPAMSWITCH current value bar # opam env -OPAMSWITCH=''; export OPAMSWITCH; # OPAMSWITCH current value bar # current switch bar # evaluate opam env # OPAMSWITCH current value - +bar # current switch -foo +bar ### sh set-switch.sh env # current switch foo @@ -142,16 +141,15 @@ OPAMSWITCH='bar'; export OPAMSWITCH; # OPAMSWITCH current value bar # opam env -OPAMSWITCH=''; export OPAMSWITCH; # OPAMSWITCH current value bar # current switch bar # evaluate opam env # OPAMSWITCH current value - +bar # current switch -foo +bar ### opam init --bare --bypass-checks --root ./foo ./REPO -n | grep -v Cygwin No configuration file found, using built-in defaults. @@ -209,17 +207,15 @@ OPAMROOT='${BASEDIR}/bar'; export OPAMROOT; # OPAMROOT current value ${BASEDIR}/bar # opam env -OPAMROOT=''; export OPAMROOT; # OPAMROOT current value ${BASEDIR}/bar # current root ${BASEDIR}/bar evaluate opam env # OPAMROOT current value - +${BASEDIR}/bar # current root -[ERROR] ${BASEDIR} exists, but does not appear to be a valid opam root. Please remove it and use `opam init', or specify a different `--root' argument -# Return code 50 # +${BASEDIR}/bar ### sh set-root.sh env # current root ${BASEDIR}/foo @@ -231,14 +227,12 @@ OPAMROOT='${BASEDIR}/bar'; export OPAMROOT; # OPAMROOT current value ${BASEDIR}/bar # opam env -OPAMROOT=''; export OPAMROOT; # OPAMROOT current value ${BASEDIR}/bar # current root ${BASEDIR}/bar evaluate opam env # OPAMROOT current value - +${BASEDIR}/bar # current root -[ERROR] ${BASEDIR} exists, but does not appear to be a valid opam root. Please remove it and use `opam init', or specify a different `--root' argument -# Return code 50 # +${BASEDIR}/bar