From 7d9540a53d62d9f8ad7569bdb93b0ca68145d40c Mon Sep 17 00:00:00 2001 From: Alpha Issiaga DIALLO Date: Tue, 24 Sep 2024 12:06:46 +0200 Subject: [PATCH] fix: enabled_if with `env` variable (#10936) * fix: enabled_if with `env` variable Signed-off-by: Alpha DIALLO --- doc/changes/10936.md | 1 + src/dune_rules/enabled_if.ml | 36 +++++++------ .../test-cases/enabled_if/eif-env-vars.t | 52 +++++++++++++++++++ .../enabled_if/eif-exec-forbidden_var.t/run.t | 8 +-- .../eif-install-forbidden_var.t/run.t | 4 +- 5 files changed, 78 insertions(+), 23 deletions(-) create mode 100644 doc/changes/10936.md create mode 100644 test/blackbox-tests/test-cases/enabled_if/eif-env-vars.t diff --git a/doc/changes/10936.md b/doc/changes/10936.md new file mode 100644 index 00000000000..7b7d047ef56 --- /dev/null +++ b/doc/changes/10936.md @@ -0,0 +1 @@ +- Fix `enabled_if` when it uses `env` variable. (#10936, fixes #10905, @moyodiallo) diff --git a/src/dune_rules/enabled_if.ml b/src/dune_rules/enabled_if.ml index 4e5fcacad37..1b372f64fef 100644 --- a/src/dune_rules/enabled_if.ml +++ b/src/dune_rules/enabled_if.ml @@ -19,6 +19,7 @@ let common_vars_list = ; "ocaml_version" ; "context_name" ; "arch_sixtyfour" + ; "env" ] ;; @@ -29,6 +30,7 @@ let common_vars ~since = match var with | "context_name" -> var, (2, 7) | "arch_sixtyfour" -> var, (3, 11) + | "env" -> var, (3, 15) | _ -> var, since) common_vars_list) ;; @@ -53,24 +55,24 @@ let decode_value ~allowed_vars ?(is_error = true) () = Blang.Ast.decode ~override_decode_bare_literal:None (String_with_vars.decode_manually (fun env var -> - match Dune_lang.Template.Pform.payload var with - | Some _ -> + let name = Dune_lang.Template.Pform.name var in + if List.exists ~f:(String.equal name) common_vars_list + then ( + match List.assoc allowed_vars name with + | None -> + emit_warning allowed_vars is_error var; + Pform.Env.parse env var + | Some min_ver -> + let current_ver = Pform.Env.syntax_version env in + if min_ver > current_ver + then ( + let loc = Dune_lang.Template.Pform.loc var in + let what = Dune_lang.Template.Pform.describe var in + Dune_lang.Syntax.Error.since loc Stanza.syntax min_ver ~what) + else Pform.Env.unsafe_parse_without_checking_version env var) + else ( emit_warning allowed_vars is_error var; - Pform.Env.parse env var - | None -> - let name = Dune_lang.Template.Pform.name var in - (match List.assoc allowed_vars name with - | None -> - emit_warning allowed_vars is_error var; - Pform.Env.parse env var - | Some min_ver -> - let current_ver = Pform.Env.syntax_version env in - if min_ver > current_ver - then ( - let loc = Dune_lang.Template.Pform.loc var in - let what = Dune_lang.Template.Pform.describe var in - Dune_lang.Syntax.Error.since loc Stanza.syntax min_ver ~what) - else Pform.Env.unsafe_parse_without_checking_version env var))) + Pform.Env.parse env var))) ;; let decode ~allowed_vars ?is_error ~since () = diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-env-vars.t b/test/blackbox-tests/test-cases/enabled_if/eif-env-vars.t new file mode 100644 index 00000000000..db4819e3a1c --- /dev/null +++ b/test/blackbox-tests/test-cases/enabled_if/eif-env-vars.t @@ -0,0 +1,52 @@ +Test enabled_if with 'env' variable. + + $ cat > dune-project < (lang dune 3.14) + > (name dune-test) + > (package + > (name dune-test)) + > EOF + + $ cat > dune < (executable + > (name main) + > (public_name dune_test) + > (modules main) + > (package dune-test) + > (modes exe)) + > (executable + > (name main_2) + > (public_name dune_test_2) + > (enabled_if (= enabled %{env:MYVAR=disabled})) + > (modules main_2) + > (package dune-test) + > (modes exe)) + > EOF + + $ MYVAR=disabled dune exec -- dune_test + File "dune", line 10, characters 24-45: + 10 | (enabled_if (= enabled %{env:MYVAR=disabled})) + ^^^^^^^^^^^^^^^^^^^^^ + Error: %{env:..} is only available since version 3.15 of the dune language. + Please update your dune-project file to have (lang dune 3.15). + [1] + + $ cat > dune-project < (lang dune 3.15) + > (name dune-test) + > (package + > (name dune-test)) + > EOF + + $ cat > main.ml < let () = print_string "Hello world" + > EOF + + $ MYVAR=disabled dune exec -- dune_test + Hello world + $ MYVAR=enabled dune exec -- dune_test + File "dune", line 11, characters 10-16: + 11 | (modules main_2) + ^^^^^^ + Error: Module Main_2 doesn't exist. + [1] diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-exec-forbidden_var.t/run.t b/test/blackbox-tests/test-cases/enabled_if/eif-exec-forbidden_var.t/run.t index 6146e02cdf1..140d01a5a16 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-exec-forbidden_var.t/run.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-exec-forbidden_var.t/run.t @@ -8,8 +8,8 @@ The next ones use forbidden variables For dune 2.3 -> 2.5 it is a warning 3 | (enabled_if (<> %{project_root} ""))) ^^^^^^^^^^^^^^^ Warning: Only architecture, system, model, os_type, ccomp_type, profile, - ocaml_version, context_name and arch_sixtyfour variables are allowed in this - 'enabled_if' field. Please upgrade your dune language to at least 3.15. + ocaml_version, context_name, arch_sixtyfour and env variables are allowed in + this 'enabled_if' field. Please upgrade your dune language to at least 3.15. bar For dune >= 2.6 it is an error @@ -21,6 +21,6 @@ For dune >= 2.6 it is an error 3 | (enabled_if (<> %{project_root} ""))) ^^^^^^^^^^^^^^^ Error: Only architecture, system, model, os_type, ccomp_type, profile, - ocaml_version, context_name and arch_sixtyfour variables are allowed in this - 'enabled_if' field. Please upgrade your dune language to at least 3.15. + ocaml_version, context_name, arch_sixtyfour and env variables are allowed in + this 'enabled_if' field. Please upgrade your dune language to at least 3.15. [1] diff --git a/test/blackbox-tests/test-cases/enabled_if/eif-install-forbidden_var.t/run.t b/test/blackbox-tests/test-cases/enabled_if/eif-install-forbidden_var.t/run.t index 424c3391fe9..4d32ee5ccda 100644 --- a/test/blackbox-tests/test-cases/enabled_if/eif-install-forbidden_var.t/run.t +++ b/test/blackbox-tests/test-cases/enabled_if/eif-install-forbidden_var.t/run.t @@ -4,6 +4,6 @@ Tests for enabled_if in install stanza using forbidden variable. 6 | (enabled_if (= %{project_root} "")) ^^^^^^^^^^^^^^^ Error: Only architecture, system, model, os_type, ccomp_type, profile, - ocaml_version, context_name and arch_sixtyfour variables are allowed in this - 'enabled_if' field. Please upgrade your dune language to at least 3.15. + ocaml_version, context_name, arch_sixtyfour and env variables are allowed in + this 'enabled_if' field. Please upgrade your dune language to at least 3.15. [1]