fix: allow internal dune actions to be sandboxed

Internal actions that that don't need to be sandboxed would prevent the entire rule from being sandboxed. Now we just sandbox them even if it's not necessary. Signed-off-by: Rudi Grinberg <[email protected]>
ocaml · Oct 30, 2023 · 8e6c4f4 · 8e6c4f4
1 parent f7f331b
commit 8e6c4f4
Show file tree

Hide file tree

Showing 3 changed files with 13 additions and 47 deletions.
diff --git a/src/dune_engine/build_system.ml b/src/dune_engine/build_system.ml
@@ -470,22 +470,10 @@ end = struct
       let* () = Targets.maybe_async (fun () -> Path.mkdir_p (Path.build dir)) in
       let is_action_dynamic = Action.is_dynamic action.action in
       let sandbox_mode =
-        match Action.is_useful_to_sandbox action.action with
-        | Clearly_not ->
-          if Sandbox_config.mem action.sandbox Sandbox_mode.none
-          then Sandbox_mode.none
-          else
-            User_error.raise
-              ~loc
-              [ Pp.text
-                  "Rule dependencies are configured to require sandboxing, but the rule \
-                   has no actions that could potentially require sandboxing."
-              ]
-        | Maybe ->
-          select_sandbox_mode
-            ~loc
-            action.sandbox
-            ~sandboxing_preference:config.sandboxing_preference
+        select_sandbox_mode
+          ~loc
+          action.sandbox
+          ~sandboxing_preference:config.sandboxing_preference
       in
       (* CR-someday amokhov: More [always_rerun] and [can_go_in_shared_cache]
          to [Rule_cache] too. *)

diff --git a/test/blackbox-tests/test-cases/corrections.t b/test/blackbox-tests/test-cases/corrections.t
@@ -103,28 +103,7 @@ One fix is to not look at it, and flag its existence as an error.
   differ.
   [1]
 
-Sandboxing doesn't necessarily help either because dune thinks
-[diff?] is not worth sandboxing.
+Sandboxing helps since dune will sandbox all actions:
 
   $ dune build text-file-corrected
   $ dune build @correction1 --sandbox copy
-  File "text-file", line 1, characters 0-0:
-  Error: Files _build/default/text-file and _build/default/text-file-corrected
-  differ.
-  [1]
-
-Sandboxing does help if the command producing the
-correction is non-trivial.
-
-  $ cat > dune <<EOF
-  > (rule (alias correction1)
-  >   (deps)
-  >   (action
-  >     (progn
-  >       (bash "true")
-  >       (diff? text-file text-file-corrected)))
-  > )
-  > EOF
-
-  $ dune build @correction1 --sandbox copy
-
diff --git a/test/blackbox-tests/test-cases/sandboxing.t b/test/blackbox-tests/test-cases/sandboxing.t
@@ -105,18 +105,17 @@ If rule fails to generate targets, we give a good error message, even with sandb
   - t
   [1]
 
-If rule is configured to require sandboxing, but clearly needs none,
-we give an error message:
+If rule is configured to require sandboxing, but clearly needs none, we sandbox
+anyway.
 
   $ true > dune
-  $ echo '(rule (target t) (deps (sandbox always)) (action (echo "")))' >> dune
+  $ cat >dune <<EOF
+  > (rule
+  >  (target t)
+  >  (deps (sandbox always))
+  >  (action (write-file t "")))
+  > EOF
   $ dune build t
-  File "dune", line 1, characters 0-60:
-  1 | (rule (target t) (deps (sandbox always)) (action (echo "")))
-      ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
-  Error: Rule dependencies are configured to require sandboxing, but the rule
-  has no actions that could potentially require sandboxing.
-  [1]
 
 If an action [chdir]s to a non-existing directory, it is created.