From 09adaf2f2145682dd2c955b5355070f6e7910454 Mon Sep 17 00:00:00 2001 From: Ali Caglayan Date: Tue, 12 May 2026 15:07:33 +0100 Subject: [PATCH] fix: reject (package ...) inside named dependency bindings MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit `(deps (:name (package foo)))` is silently accepted by the parser but `%{name}` resolves to an empty path list — the package contributes no paths to the binding, only env vars (when (package ...) is allowed by the surrounding code). This is rarely what users intend. Reject the construct in [named_paths_builder] with a clear error pointing at the (package ...) location and a hint to move the entry out of the binding. Pre-existing behavior; not specific to the install-layout PR but found while reviewing. Signed-off-by: Ali Caglayan --- doc/changes/fixed/14499.md | 3 ++ src/dune_rules/dep_conf_eval.ml | 16 +++++++- .../named-binding-with-package-error.t | 39 +++++++++++++++++++ 3 files changed, 57 insertions(+), 1 deletion(-) create mode 100644 doc/changes/fixed/14499.md create mode 100644 test/blackbox-tests/test-cases/depend-on/named-binding-with-package-error.t diff --git a/doc/changes/fixed/14499.md b/doc/changes/fixed/14499.md new file mode 100644 index 00000000000..23a00976cb7 --- /dev/null +++ b/doc/changes/fixed/14499.md @@ -0,0 +1,3 @@ +- Reject `(package ...)` inside a named dependency binding + (`(deps (:name (package foo)))`). Previously this was silently accepted but + `%{name}` would resolve to an empty path list. (#14499, @Alizter) diff --git a/src/dune_rules/dep_conf_eval.ml b/src/dune_rules/dep_conf_eval.ml index 06c3b3fbbcd..8d3fa07d25d 100644 --- a/src/dune_rules/dep_conf_eval.ml +++ b/src/dune_rules/dep_conf_eval.ml @@ -341,7 +341,21 @@ and named_paths_builder ~expander l = in to_action_builder r :: builders, bindings, envs | Named (name, x) -> - let x = List.map x ~f:(dep expander) in + let x = + List.map x ~f:(function + | Dep_conf.Package p -> + User_error.raise + ~loc:(String_with_vars.loc p) + ~hints: + [ Pp.text "Place the (package ...) entry in the deps list directly." + ] + [ Pp.textf + "(package ...) is not supported inside a named dependency \ + binding (:%s)." + name + ] + | d -> dep expander d) + in let envs = List.fold_left x ~init:envs ~f:(fun envs r -> match include_bin_env r with diff --git a/test/blackbox-tests/test-cases/depend-on/named-binding-with-package-error.t b/test/blackbox-tests/test-cases/depend-on/named-binding-with-package-error.t new file mode 100644 index 00000000000..96f002fbfc7 --- /dev/null +++ b/test/blackbox-tests/test-cases/depend-on/named-binding-with-package-error.t @@ -0,0 +1,39 @@ +Writing `(package ...)` inside a named dependency binding like +`(:name (package foo))` is rejected: the binding would resolve to an +empty path list, which is rarely what the user intended. + + $ cat >dune-project < (lang dune 3.24) + > (package (name mypkg)) + > EOF + $ mkdir src + $ cat >src/dune < (library (public_name mypkg)) + > EOF + $ cat >src/mypkg.ml <<'EOF' + > let x = 1 + > EOF + + $ cat >dune <<'EOF' + > (rule + > (deps (:pkg (package mypkg))) + > (action (with-stdout-to out (echo %{pkg})))) + > EOF + + $ dune build out 2>&1 + File "dune", line 2, characters 22-27: + 2 | (deps (:pkg (package mypkg))) + ^^^^^ + Error: (package ...) is not supported inside a named dependency binding + (:pkg). + Hint: Place the (package ...) entry in the deps list directly. + [1] + +Putting the package outside the named binding works: + + $ cat >dune <<'EOF' + > (rule + > (deps (package mypkg)) + > (action (with-stdout-to out (echo done)))) + > EOF + $ dune build out