feat: ensure @pkg-install is not registered without lock file (#11153)

* chore: ensure @pkg-install is not registered without lock file Signed-off-by: Etienne Marais <[email protected]> * fix: improve error message Signed-off-by: Etienne Marais <[email protected]> * Use `Action_builder.fail` to register an action which fails Signed-off-by: Marek Kubica <[email protected]> --------- Signed-off-by: Etienne Marais <[email protected]> Signed-off-by: Marek Kubica <[email protected]> Co-authored-by: Marek Kubica <[email protected]>
ocaml · Dec 3, 2024 · 64be619 · 64be619
1 parent 0a12c60
commit 64be619
Show file tree

Hide file tree

Showing 4 changed files with 54 additions and 18 deletions.
diff --git a/doc/reference/aliases/pkg-install.rst b/doc/reference/aliases/pkg-install.rst
@@ -18,6 +18,6 @@ downloaded, ``dune build`` will **also** take care of getting and building them.
     change less regularly.
 
 If you are building the ``@pkg-install`` alias in a repository where package
-management is not activated, the command has no effect.
+management is not activated, the command will fail.
 
 .. seealso:: :doc:`/explanation/package-management`
diff --git a/src/dune_rules/pkg_rules.ml b/src/dune_rules/pkg_rules.ml
@@ -1958,6 +1958,25 @@ let gen_rules context_name (pkg : Pkg.t) =
 
 module Gen_rules = Build_config.Gen_rules
 
+let pkg_alias_disabled =
+  Action_builder.fail
+    { fail =
+        (fun () ->
+          let error =
+            [ Pp.text "The @pkg-install alias cannot be used without a lock dir" ]
+          in
+          let hints =
+            [ Pp.concat
+                ~sep:Pp.space
+                [ Pp.text "You might want to create the lock dir with"
+                ; User_message.command "dune pkg lock"
+                ]
+            ]
+          in
+          User_error.raise ~hints error)
+    }
+;;
+
 let setup_pkg_install_alias =
   let build_packages_of_context ctx_name =
     (* Fetching the package target implies that we will also fetch the extra
@@ -1981,14 +2000,15 @@ let setup_pkg_install_alias =
       with
       | false -> Memo.return Rules.empty
       | true ->
-        Lock_dir.lock_dir_active ctx_name
-        >>= (function
-         | false -> Memo.return Rules.empty
-         | true ->
-           Rules.collect_unit (fun () ->
-             let alias = Alias.make ~dir Alias0.pkg_install in
-             let deps = build_packages_of_context ctx_name in
-             Rules.Produce.Alias.add_deps alias deps))
+        let* active = Lock_dir.lock_dir_active ctx_name in
+        let alias = Alias.make ~dir Alias0.pkg_install in
+        Rules.collect_unit (fun () ->
+          let deps =
+            match active with
+            | true -> build_packages_of_context ctx_name
+            | false -> pkg_alias_disabled
+          in
+          Rules.Produce.Alias.add_deps alias deps)
     in
     Gen_rules.rules_for ~dir ~allowed_subdirs:Filename.Set.empty rule
     |> Gen_rules.rules_here

diff --git a/test/blackbox-tests/test-cases/describe/aliases.t/run.t b/test/blackbox-tests/test-cases/describe/aliases.t/run.t
@@ -9,6 +9,7 @@ In an empty dune project, the following aliases are available.
   default
   fmt
   ocaml-index
+  pkg-install
 
 User defined aliases can be added to a dune file. These should be picked up by
 the command.
@@ -24,6 +25,7 @@ the command.
   fmt
   foo
   ocaml-index
+  pkg-install
 
 Aliases in subdirectories should not be picked up.
 
@@ -39,6 +41,7 @@ Aliases in subdirectories should not be picked up.
   fmt
   foo
   ocaml-index
+  pkg-install
 
 But checking the subdirectory it should be available.
 
@@ -62,6 +65,7 @@ Adding an OCaml library will introduce OCaml specific aliases:
   doc-private
   fmt
   ocaml-index
+  pkg-install
 
 Adding a cram test will introduce an alias with the name of the test and also
 introduce the runtest alias:
@@ -75,6 +79,7 @@ bbb
   fmt
   mytest
   ocaml-index
+  pkg-install
   runtest
 
 We can also show aliases in multiple directories at once:
@@ -86,6 +91,7 @@ We can also show aliases in multiple directories at once:
   fmt
   mytest
   ocaml-index
+  pkg-install
   runtest
 
   subdir:
@@ -104,6 +110,7 @@ Including those in the _build/ directory:
   fmt
   mytest
   ocaml-index
+  pkg-install
   runtest
 
   _build/default:
@@ -112,6 +119,7 @@ Including those in the _build/ directory:
   fmt
   mytest
   ocaml-index
+  pkg-install
   runtest
 
 These are context sensitive:
@@ -132,4 +140,5 @@ These are context sensitive:
   fmt
   mytest
   ocaml-index
+  pkg-install
   runtest
diff --git a/test/blackbox-tests/test-cases/pkg/alias-pkg-install.t b/test/blackbox-tests/test-cases/pkg/alias-pkg-install.t
@@ -3,6 +3,22 @@ without building the project itself.
 
   $ . ./helpers.sh
 
+Create a project using the fake library as a dependency:
+  $ cat > dune-project << EOF
+  > (lang dune 3.16)
+  > (package
+  >  (name bar)
+  >  (allow_empty)
+  >  (depends foo))
+  > EOF
+
+Ensure the alias is not available outside of the package manamgent context:
+  $ dune build @pkg-install
+  Error: The @pkg-install alias cannot be used without a lock dir
+  -> required by alias pkg-install
+  Hint: You might want to create the lock dir with 'dune pkg lock'
+  [1]
+
 Create a fake package which echoes information to stdout when build:
   $ make_lockdir
   $ cat > dune.lock/foo.pkg << EOF
@@ -13,15 +29,6 @@ Create a fake package which echoes information to stdout when build:
   >  (run echo "Install package foo"))
   > EOF
 
-Create a project using the fake library as a dependency:
-  $ cat > dune-project << EOF
-  > (lang dune 3.16)
-  > (package
-  >  (name bar)
-  >  (allow_empty)
-  >  (depends foo))
-  > EOF
-
 Create a rule to show that this rule is not called with `@pkg-install` as `bar`
 is not build when calling the alias. If called, it would output the content of
 the `bar.ml` file: