Merge pull request #4904 from rjbou/var-shadow

Warn when setting a variable that shadow an option

Author: rjbou

master_changes.md

@@ -62,6 +62,7 @@ users)
 
 ## Var/Option
   * Fix the value of the 'arch' variable when the current OS is 32bit on a 64bit machine [#5950 @kit-ty-kate - fix #5949]
+  * Warn when setting a variable if an option is shadowed [#4904 @rjbou - fix #4730]
 
 ## Update / Upgrade
 

src/client/opamConfigCommand.ml

@@ -936,6 +936,19 @@ type ('var,'config) var_confset =
   }
 
 let set_var var value conf =
+  let svar = OpamVariable.Full.to_string var in
+  let global_option, switch_option =
+    let exists f = List.exists (fun (name, _) -> String.equal svar name) f in
+    (exists OpamFile.Config.fields),
+    (exists OpamFile.Switch_config.fields)
+  in
+  if global_option || switch_option then
+    (let scope = if global_option then "global" else "switch" in
+     OpamConsole.warning
+       "You are setting a variable that has the same name as a %s option.\n\
+        Did you mean to use `opam option` instead? You can revert this variable \
+        using: 'opam var %s= --%s'\n"
+       scope svar scope);
   let conf = conf (OpamVariable.Full.variable var) in
   let global_vars = conf.stv_vars in
   let rest = List.filter (fun v -> not (conf.stv_find v)) global_vars in

tests/reftests/var-option.test

@@ -17,12 +17,8 @@ Removed variable group in switch var-option
 ### opam option sys-pkg-manager-cmd= --global | grep -v configuration
 ### opam var arch=x86_64 --global
 Added '[arch "x86_64" "Set through 'opam var'"]' to field global-variables in global configuration
-### opam var jobs=7 --global
-Added '[jobs "7" "Set through 'opam var'"]' to field global-variables in global configuration
 ### opam var make=make --global
 Added '[make "make" "Set through 'opam var'"]' to field global-variables in global configuration
-### opam var opam-version=68.79 --global
-Added '[opam-version "68.79" "Set through 'opam var'"]' to field global-variables in global configuration
 ### opam var os=linux --global
 Added '[os "linux" "Set through 'opam var'"]' to field global-variables in global configuration
 ### opam var os-distribution=lorem --global
@@ -45,6 +41,8 @@ Set to '[]' the field wrap-build-commands in global configuration
 Set to '[]' the field wrap-install-commands in global configuration
 ### opam option wrap-remove-commands=[] --global
 Set to '[]' the field wrap-remove-commands in global configuration
+### opam option jobs=7 --global
+Set to '7' the field jobs in global configuration
 ### opam install i-got-the-variables --yes
 The following actions will be performed:
 === install 2 packages
@@ -55,14 +53,13 @@ The following actions will be performed:
 -> installed base.2
 -> installed i-got-the-variables.2.4.6
 Done.
-### opam var | "   *" -> " " | "[.]exe " -> ""
+### opam var | "   *" -> " " | "[.]exe " -> "" | "opam-version.*" -> '\c'
 
 <><> Global opam variables ><><><><><><><><><><><><><><><><><><><><><><><><><><>
 arch x86_64 # Set through 'opam var'
 exe # Suffix needed for executable filenames (Windows)
-jobs 7 # Set through 'opam var'
+jobs 7 # The number of parallel jobs set up in opam configuration
 make make # Set through 'opam var'
-opam-version 68.79 # Set through 'opam var'
 os linux # Set through 'opam var'
 os-distribution lorem # Set through 'opam var'
 os-family ipsum # Set through 'opam var'
@@ -113,12 +110,11 @@ etc      ${BASEDIR}/OPAM/var-option/etc
 man      ${BASEDIR}/OPAM/var-option/man
 toplevel ${BASEDIR}/OPAM/var-option/lib/toplevel
 stublibs ${BASEDIR}/OPAM/var-option/lib/stublibs
-### opam var --global | "   *" -> " " | "[.]exe " -> ""
+### opam var --global | "   *" -> " " | "[.]exe " -> "" | "opam-version.*" -> '\c'
 arch x86_64 # Set through 'opam var'
 exe # Suffix needed for executable filenames (Windows)
-jobs 7 # Set through 'opam var'
+jobs 7 # The number of parallel jobs set up in opam configuration
 make make # Set through 'opam var'
-opam-version 68.79 # Set through 'opam var'
 os linux # Set through 'opam var'
 os-distribution lorem # Set through 'opam var'
 os-family ipsum # Set through 'opam var'
@@ -231,7 +227,7 @@ depext-run-installs            true
 download-command               {}
 download-jobs                  3
 git-location                   {}
-jobs                           {}
+jobs                           7
 post-build-commands            {}
 post-install-commands          {}
 post-remove-commands           {}
@@ -275,7 +271,9 @@ wrap-remove-commands   {}
 [ERROR] Field or section download-jobs not found
 # Return code 5 #
 ### opam option jobs
+7
 ### opam option jobs --global
+7
 ### opam option jobs --switch var-option
 [ERROR] Field or section jobs not found
 # Return code 5 #
@@ -488,7 +486,7 @@ Reverted field archive-mirrors in global configuration
 ### opam option archive-mirrors
 ### # Check that eval-variable is removed when a global variable is removed
 ### opam option global-variables
-[[foo "global-foo" "Set through 'opam var'"] [bin "global-bin" "Set through 'opam var'"] [os-version "dolor" "Set through 'opam var'"] [os-family "ipsum" "Set through 'opam var'"] [os-distribution "lorem" "Set through 'opam var'"] [os "linux" "Set through 'opam var'"] [opam-version "68.79" "Set through 'opam var'"] [make "make" "Set through 'opam var'"] [jobs "7" "Set through 'opam var'"] [arch "x86_64" "Set through 'opam var'"] [sys-ocaml-version "4.08.0" "Set through 'opam var'"]]
+[[foo "global-foo" "Set through 'opam var'"] [bin "global-bin" "Set through 'opam var'"] [os-version "dolor" "Set through 'opam var'"] [os-family "ipsum" "Set through 'opam var'"] [os-distribution "lorem" "Set through 'opam var'"] [os "linux" "Set through 'opam var'"] [make "make" "Set through 'opam var'"] [arch "x86_64" "Set through 'opam var'"] [sys-ocaml-version "4.08.0" "Set through 'opam var'"]]
 ### opam option 'eval-variables=[dolore ["mania"] "alica"]'
 Set to '[dolore ["mania"] "alica"]' the field eval-variables in global configuration
 ### opam option eval-variables
@@ -500,8 +498,8 @@ Removed variable dolore in global configuration
 ### opam option eval-variables
 []
 ### opam option global-variables
-[[foo "global-foo" "Set through 'opam var'"] [bin "global-bin" "Set through 'opam var'"] [os-version "dolor" "Set through 'opam var'"] [os-family "ipsum" "Set through 'opam var'"] [os-distribution "lorem" "Set through 'opam var'"] [os "linux" "Set through 'opam var'"] [opam-version "68.79" "Set through 'opam var'"] [make "make" "Set through 'opam var'"] [jobs "7" "Set through 'opam var'"] [arch "x86_64" "Set through 'opam var'"] [sys-ocaml-version "4.08.0" "Set through 'opam var'"]]
-### # Check uniable operations
+[[foo "global-foo" "Set through 'opam var'"] [bin "global-bin" "Set through 'opam var'"] [os-version "dolor" "Set through 'opam var'"] [os-family "ipsum" "Set through 'opam var'"] [os-distribution "lorem" "Set through 'opam var'"] [os "linux" "Set through 'opam var'"] [make "make" "Set through 'opam var'"] [arch "x86_64" "Set through 'opam var'"] [sys-ocaml-version "4.08.0" "Set through 'opam var'"]]
+### # Check disallowed operations
 ### opam option 'variables+={esse: "cillum"}'
 [ERROR] Field variables can't be directly appended to, use `opam var` instead
 # Return code 2 #
@@ -534,10 +532,66 @@ jobs download-command download-jobs archive-mirrors solver-criteria solver-upgra
 [ERROR] There is no option named 'bar'. The allowed options are:
 synopsis setenv depext-bypass pre-build-commands pre-install-commands pre-remove-commands pre-session-commands wrap-build-commands wrap-install-commands wrap-remove-commands post-build-commands post-install-commands post-remove-commands post-session-commands variables
 # Return code 2 #
+### # Check option shadowing
+### opam option download-jobs
+1
+### opam var download-jobs
+[ERROR] Variable download-jobs not found in the global configuration
+# Return code 5 #
+### opam var download-jobs=42 --global
+[WARNING] You are setting a variable that has the same name as a global option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var download-jobs= --global'
+
+Added '[download-jobs "42" "Set through 'opam var'"]' to field global-variables in global configuration
+### opam option download-jobs --global
+1
+### opam var download-jobs --global
+42
+### opam var download-jobs=44 --switch var-option
+[WARNING] You are setting a variable that has the same name as a global option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var download-jobs= --global'
+
+Added 'download-jobs: "44"' to field variables in switch var-option
+### opam var download-jobs --switch var-option
+44
+### opam var synopsis --global
+[ERROR] Variable synopsis not found in the global configuration
+# Return code 5 #
+### opam var 'synopsis="lorem ipsum"' --global
+[WARNING] You are setting a variable that has the same name as a switch option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var synopsis= --switch'
+
+Added '[synopsis "\"lorem ipsum\"" "Set through 'opam var'"]' to field global-variables in global configuration
+### opam var synopsis --global
+"lorem ipsum"
+### opam option synopsis --switch var-option
+""
+### opam var synopsis --switch var-option
+[ERROR] Variable synopsis not found in switch var-option
+# Return code 5 #
+### opam var 'synopsis="ipsum"' --switch var-option
+[WARNING] You are setting a variable that has the same name as a switch option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var synopsis= --switch'
+
+Added 'synopsis: "\"ipsum\""' to field variables in switch var-option
+### opam option synopsis --switch var-option
+""
+### opam var synopsis --switch var-option
+"ipsum"
+### opam var synopsis= --global -y
+[WARNING] You are setting a variable that has the same name as a switch option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var synopsis= --switch'
+
+Removed variable synopsis in global configuration
+### opam var download-jobs= --global -y
+[WARNING] You are setting a variable that has the same name as a global option.
+          Did you mean to use `opam option` instead? You can revert this variable using: 'opam var download-jobs= --global'
+
+Removed variable download-jobs in global configuration
 ### : when no switch is present
 ### opam switch remove var-option -y
 Switch var-option and all its packages will be wiped. Are you sure? [y/n] y
-### opam var | ' +#.*' -> '' | " +[.]exe" -> ""
+### opam var | ' +#.*' -> '' | " +[.]exe" -> "" | "opam-version.*" -> '\c'
 
 <><> Global opam variables ><><><><><><><><><><><><><><><><><><><><><><><><><><>
 arch              x86_64
@@ -546,7 +600,6 @@ exe
 foo               global-foo
 jobs              7
 make              make
-opam-version      68.79
 os                linux
 os-distribution   lorem
 os-family         ipsum
@@ -577,14 +630,13 @@ PKG:hash
 PKG:dev
 PKG:build-id
 PKG:opamfile
-### opam var --global | ' +#.*' -> '' | " +[.]exe" -> ""
+### opam var --global | ' +#.*' -> '' | " +[.]exe" -> "" | "opam-version.*" -> '\c'
 arch              x86_64
 bin               global-bin
 exe
 foo               global-foo
 jobs              7
 make              make
-opam-version      68.79
 os                linux
 os-distribution   lorem
 os-family         ipsum
@@ -615,7 +667,7 @@ depext-run-installs            true
 download-command               {}
 download-jobs                  1
 git-location                   {}
-jobs                           {}
+jobs                           7
 post-build-commands            {}
 post-install-commands          {}
 post-remove-commands           {}
@@ -647,7 +699,7 @@ depext-run-installs            true
 download-command               {}
 download-jobs                  1
 git-location                   {}
-jobs                           {}
+jobs                           7
 post-build-commands            {}
 post-install-commands          {}
 post-remove-commands           {}