Return a status type instead of a plain bool

Signed-off-by: Etienne Millon <me@emillon.org>

Author: emillon

bin/install_uninstall.ml

@@ -696,9 +696,12 @@ let install_uninstall ~what =
                             match special_file with
                             | _ when not create_install_files ->
                               Fiber.return true
-                            | None ->
-                              Dune_rules.Artifact_substitution.test_file
-                                ~src:entry.src ()
+                            | None -> (
+                              let open Dune_rules.Artifact_substitution in
+                              let+ status = test_file ~src:entry.src () in
+                              match status with
+                              | Some_substitution -> true
+                              | No_substitution -> false)
                             | Some Special_file.META
                             | Some Special_file.Dune_package ->
                               Fiber.return true

src/dune_rules/artifact_substitution.ml

@@ -425,6 +425,10 @@ type mode =
       ; conf : conf
       }
 
+type status =
+  | Some_substitution
+  | No_substitution
+
 (** The copy algorithm works as follow:
 
     {v
@@ -465,8 +469,7 @@ output the replacement        |                                             |
     v} *)
 let parse ~input ~mode =
   let open Fiber.O in
-  let rec loop scanner_state ~beginning_of_data ~pos ~end_of_data ~has_seen_subs
-      =
+  let rec loop scanner_state ~beginning_of_data ~pos ~end_of_data ~status =
     let scanner_state = Scanner.run scanner_state ~buf ~pos ~end_of_data in
     let placeholder_start =
       match scanner_state with
@@ -492,7 +495,7 @@ let parse ~input ~mode =
       match decode placeholder with
       | Some t -> (
         match mode with
-        | Test -> Fiber.return true
+        | Test -> Fiber.return Some_substitution
         | Copy { output; input_file; conf } ->
           let* s = eval t ~conf in
           (if !Clflags.debug_artifact_substitution then
@@ -509,13 +512,13 @@ let parse ~input ~mode =
           output (Bytes.unsafe_of_string s) 0 len;
           let pos = placeholder_start + len in
           loop Scan0 ~beginning_of_data:pos ~pos ~end_of_data
-            ~has_seen_subs:true)
+            ~status:Some_substitution)
       | None ->
         (* Restart just after [prefix] since we know for sure that a placeholder
            cannot start before that. *)
         loop Scan0 ~beginning_of_data:placeholder_start
           ~pos:(placeholder_start + prefix_len)
-          ~end_of_data ~has_seen_subs)
+          ~end_of_data ~status)
     | scanner_state -> (
       (* We reached the end of the buffer: move the leftover data back to the
          beginning of [buf] and refill the buffer *)
@@ -538,23 +541,24 @@ let parse ~input ~mode =
           (* There might still be another placeholder after this invalid one
              with a length that is too long *)
           loop Scan0 ~beginning_of_data:0 ~pos:prefix_len ~end_of_data:leftover
-            ~has_seen_subs
+            ~status
         | _ -> (
           match mode with
-          | Test -> Fiber.return false
+          | Test -> Fiber.return No_substitution
           | Copy { output; _ } ->
             (* Nothing more to read; [leftover] is definitely not the beginning
                of a placeholder, send it and end the copy *)
             output buf 0 leftover;
-            Fiber.return has_seen_subs))
+            Fiber.return status))
       | n ->
         loop scanner_state ~beginning_of_data:0 ~pos:leftover
-          ~end_of_data:(leftover + n) ~has_seen_subs)
+          ~end_of_data:(leftover + n) ~status)
   in
   match input buf 0 buf_len with
-  | 0 -> Fiber.return false
+  | 0 -> Fiber.return No_substitution
   | n ->
-    loop Scan0 ~beginning_of_data:0 ~pos:0 ~end_of_data:n ~has_seen_subs:false
+    loop Scan0 ~beginning_of_data:0 ~pos:0 ~end_of_data:n
+      ~status:No_substitution
 
 let copy ~conf ~input_file ~input ~output =
   parse ~input ~mode:(Copy { conf; input_file; output })
@@ -570,8 +574,8 @@ let copy_file_non_atomic ~conf ?chmod ~src ~dst () =
 
 let run_sign_hook conf ~has_subst file =
   match has_subst with
-  | false -> Fiber.return ()
-  | true -> (
+  | No_substitution -> Fiber.return ()
+  | Some_substitution -> (
     match Lazy.force conf.sign_hook with
     | Some hook -> hook file
     | None -> Fiber.return ())

src/dune_rules/artifact_substitution.mli

@@ -67,6 +67,10 @@ val copy_file :
   -> unit
   -> unit Fiber.t
 
+type status =
+  | Some_substitution
+  | No_substitution
+
 (** Generic version of [copy_file]. Rather than filenames, it takes an input and
     output functions. Their semantic must match the ones of the [input] and
     [output] functions from the OCaml standard library.
@@ -80,10 +84,10 @@ val copy :
   -> input_file:Path.t
   -> input:(Bytes.t -> int -> int -> int)
   -> output:(Bytes.t -> int -> int -> unit)
-  -> bool Fiber.t
+  -> status Fiber.t
 
 (** Produce the string that would replace the placeholder with the given value .*)
 val encode_replacement : len:int -> repl:string -> string
 
 (** Test if a file contains a substitution placeholder. *)
-val test_file : src:Path.t -> unit -> bool Fiber.t
+val test_file : src:Path.t -> unit -> status Fiber.t

test/unit-tests/artifact_substitution/artifact_substitution.ml

@@ -145,7 +145,7 @@ let test input =
        to_copy
      in
      let output = Buffer.add_subbytes buf in
-     let+ (_ : bool) =
+     let+ (_ : Artifact_substitution.status) =
        Artifact_substitution.copy ~conf:Artifact_substitution.conf_dummy
          ~input_file:(Path.of_string "<memory>")
          ~input ~output