Skip to content

Commit

Permalink
fix(pkg): mark more variables as unsupported (#8797)
Browse files Browse the repository at this point in the history
Also split the handling of global and package variables

Signed-off-by: Rudi Grinberg <me@rgrinberg.com>
  • Loading branch information
rgrinberg authored Sep 29, 2023
1 parent d5bc7ea commit 5d6a2bd
Show file tree
Hide file tree
Showing 3 changed files with 55 additions and 67 deletions.
42 changes: 27 additions & 15 deletions src/dune_pkg/package_variable.ml
Original file line number Diff line number Diff line change
Expand Up @@ -63,30 +63,42 @@ let to_macro_invocation { name; scope } =

let to_pform t = Pform.Macro (to_macro_invocation t)

let package_variable package variable =
match package, variable with
| _, ("root" | "hash" | "build-id" | "misc" | "opam-version") ->
`Unsupported_variable variable
| "_", variable -> `Package_variable (self_scoped (Name.of_string variable))
| package, variable ->
`Package_variable
(package_scoped (Name.of_string variable) (Package_name.of_string package))
let global_variable variable =
match variable with
| "root" -> Error (`Unsupported_variable variable)
| s -> Ok (Pform.Var.of_opam_global_variable_name s)
;;

let package_variable variable =
match variable with
| "hash" | "build-id" | "misc" | "opam-version" | "depends" | "build" | "opamfile" ->
Error (`Unsupported_variable variable)
| s -> Ok (Name.of_string s)
;;

let of_opam_ident ident =
match String.lsplit2 ident ~on:':' with
| Some (package, variable) -> package_variable package variable
| Some (package, variable) ->
let variable = package_variable variable in
Result.map variable ~f:(fun var ->
`Package_variable
(if package = "_"
then self_scoped var
else package_scoped var (Package_name.of_string package)))
| None ->
(match Pform.Var.of_opam_global_variable_name ident with
| Some var -> `Global_variable var
| None -> package_variable "_" ident)
global_variable ident
|> Result.bind ~f:(function
| Some var -> Ok (`Global_variable var)
| None ->
let variable = package_variable ident in
Result.map variable ~f:(fun var -> `Package_variable (self_scoped var)))
;;

let pform_of_opam_ident ~package_name ident =
match of_opam_ident ident with
| `Package_variable t -> to_pform t
| `Global_variable var -> Pform.Var var
| `Unsupported_variable name ->
| Ok (`Package_variable t) -> to_pform t
| Ok (`Global_variable var) -> Pform.Var var
| Error (`Unsupported_variable name) ->
User_error.raise
[ Pp.textf
"Variable %S occuring in opam package %S is not supported."
Expand Down
68 changes: 16 additions & 52 deletions test/blackbox-tests/test-cases/pkg/opam-var/opam-var-pkg.t
Original file line number Diff line number Diff line change
Expand Up @@ -14,8 +14,6 @@ We echo each package variable.
> [ "echo" "4" version ]
> [ "echo" "5" _:version ]
> [ "echo" "6" foo:version ]
> [ "echo" "7" _:depends ]
> [ "echo" "8" foo:depends ]
> [ "echo" "9" _:installed ]
> [ "echo" "10" foo:installed ]
> [ "echo" "11" _:enable ]
Expand All @@ -36,12 +34,8 @@ We echo each package variable.
> [ "echo" "26" foo:share ]
> [ "echo" "27" _:etc ]
> [ "echo" "28" foo:etc ]
> [ "echo" "29" _:build ]
> [ "echo" "30" foo:build ]
> [ "echo" "31" _:dev ]
> [ "echo" "32" foo:dev ]
> [ "echo" "33" _:opamfile ]
> [ "echo" "34" foo:opamfile ]
> [ "echo" "35" with-test ]
> [ "echo" "36" _:with-test ]
> [ "echo" "37" foo:with-test ]
Expand Down Expand Up @@ -73,8 +67,6 @@ corresponding Dune version.
(run echo 4 %{pkg-self:version})
(run echo 5 %{pkg-self:version})
(run echo 6 %{pkg:foo:version})
(run echo 7 %{pkg-self:depends})
(run echo 8 %{pkg:foo:depends})
(run echo 9 %{pkg-self:installed})
(run echo 10 %{pkg:foo:installed})
(run echo 11 %{pkg-self:enable})
Expand All @@ -95,12 +87,8 @@ corresponding Dune version.
(run echo 26 %{pkg:foo:share})
(run echo 27 %{pkg-self:etc})
(run echo 28 %{pkg:foo:etc})
(run echo 29 %{pkg-self:build})
(run echo 30 %{pkg:foo:build})
(run echo 31 %{pkg-self:dev})
(run echo 32 %{pkg:foo:dev})
(run echo 33 %{pkg-self:opamfile})
(run echo 34 %{pkg:foo:opamfile})
(run echo 35 %{pkg-self:with-test})
(run echo 36 %{pkg-self:with-test})
(run echo 37 %{pkg:foo:with-test})
Expand All @@ -115,60 +103,36 @@ corresponding Dune version.
The values here are not important, but Dune should be able to interpret the variables.

$ build_pkg testpkg
File "dune.lock/testpkg.pkg", line 11, characters 14-33:
11 | (run echo 7 %{pkg-self:depends})
^^^^^^^^^^^^^^^^^^^
Error: invalid section "depends"
File "dune.lock/testpkg.pkg", line 12, characters 14-32:
12 | (run echo 8 %{pkg:foo:depends})
^^^^^^^^^^^^^^^^^^
Error: invalid section "depends"
File "dune.lock/testpkg.pkg", line 33, characters 15-32:
33 | (run echo 29 %{pkg-self:build})
^^^^^^^^^^^^^^^^^
Error: invalid section "build"
File "dune.lock/testpkg.pkg", line 34, characters 15-31:
34 | (run echo 30 %{pkg:foo:build})
^^^^^^^^^^^^^^^^
Error: invalid section "build"
File "dune.lock/testpkg.pkg", line 37, characters 15-35:
37 | (run echo 33 %{pkg-self:opamfile})
^^^^^^^^^^^^^^^^^^^^
Error: invalid section "opamfile"
File "dune.lock/testpkg.pkg", line 38, characters 15-34:
38 | (run echo 34 %{pkg:foo:opamfile})
^^^^^^^^^^^^^^^^^^^
Error: invalid section "opamfile"
File "dune.lock/testpkg.pkg", line 39, characters 15-36:
39 | (run echo 35 %{pkg-self:with-test})
File "dune.lock/testpkg.pkg", line 33, characters 15-36:
33 | (run echo 35 %{pkg-self:with-test})
^^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-test"
File "dune.lock/testpkg.pkg", line 40, characters 15-36:
40 | (run echo 36 %{pkg-self:with-test})
File "dune.lock/testpkg.pkg", line 34, characters 15-36:
34 | (run echo 36 %{pkg-self:with-test})
^^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-test"
File "dune.lock/testpkg.pkg", line 41, characters 15-35:
41 | (run echo 37 %{pkg:foo:with-test})
File "dune.lock/testpkg.pkg", line 35, characters 15-35:
35 | (run echo 37 %{pkg:foo:with-test})
^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-test"
File "dune.lock/testpkg.pkg", line 42, characters 15-35:
42 | (run echo 38 %{pkg-self:with-doc})
File "dune.lock/testpkg.pkg", line 36, characters 15-35:
36 | (run echo 38 %{pkg-self:with-doc})
^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-doc"
File "dune.lock/testpkg.pkg", line 43, characters 15-35:
43 | (run echo 39 %{pkg-self:with-doc})
File "dune.lock/testpkg.pkg", line 37, characters 15-35:
37 | (run echo 39 %{pkg-self:with-doc})
^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-doc"
File "dune.lock/testpkg.pkg", line 44, characters 15-34:
44 | (run echo 40 %{pkg:foo:with-doc})
File "dune.lock/testpkg.pkg", line 38, characters 15-34:
38 | (run echo 40 %{pkg:foo:with-doc})
^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-doc"
File "dune.lock/testpkg.pkg", line 45, characters 15-41:
45 | (run echo 41 %{pkg-self:with-dev-setup})
File "dune.lock/testpkg.pkg", line 39, characters 15-41:
39 | (run echo 41 %{pkg-self:with-dev-setup})
^^^^^^^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-dev-setup"
File "dune.lock/testpkg.pkg", line 46, characters 15-40:
46 | (run echo 42 %{pkg:foo:with-dev-setup})))
File "dune.lock/testpkg.pkg", line 40, characters 15-40:
40 | (run echo 42 %{pkg:foo:with-dev-setup})))
^^^^^^^^^^^^^^^^^^^^^^^^^
Error: invalid section "with-dev-setup"
[1]
12 changes: 12 additions & 0 deletions test/blackbox-tests/test-cases/pkg/opam-var/opam-var-unsupported.t
Original file line number Diff line number Diff line change
Expand Up @@ -41,3 +41,15 @@ These should all have nice error messages explaining that they are not supported
$ fail_solve _:misc
Error: Variable "misc" occuring in opam package "testpkg.0.0.1" is not
supported.
# _:depends
$ fail_solve _:depends
Error: Variable "depends" occuring in opam package "testpkg.0.0.1" is not
supported.
# _:build
$ fail_solve _:build
Error: Variable "build" occuring in opam package "testpkg.0.0.1" is not
supported.
# _:opamfile
$ fail_solve _:opamfile
Error: Variable "opamfile" occuring in opam package "testpkg.0.0.1" is not
supported.

0 comments on commit 5d6a2bd

Please sign in to comment.