fix: Better error for unzip failure (#12683)

Signed-off-by: Sudha Parimala <[email protected]>
Signed-off-by: Shon Feder <[email protected]>
Co-authored-by: Shon Feder <[email protected]>

Author: Sudha247

src/dune_pkg/archive_driver.ml

@@ -10,7 +10,7 @@ module Command = struct
 end
 
 type t =
-  { command : Command.t Lazy.t
+  { command : Command.t Fiber.Lazy.t
   ; suffixes : string list
   }
 
@@ -26,38 +26,56 @@ let make_zip_args ~archive ~target_in_temp =
 
 let tar =
   let command =
-    lazy
-      (match
-         (* Test for tar before bsdtar as tar is more likely to be installed
+    Fiber.Lazy.create (fun () ->
+      match
+        (* Test for tar before bsdtar as tar is more likely to be installed
             and both work equally well for tarballs. *)
-         List.find_map [ "tar"; "bsdtar" ] ~f:which
-       with
-       | Some bin -> { Command.bin; make_args = make_tar_args }
-       | None -> Dune_engine.Utils.program_not_found "tar" ~loc:None)
+        List.find_map [ "tar"; "bsdtar" ] ~f:which
+      with
+      | Some bin -> Fiber.return { Command.bin; make_args = make_tar_args }
+      | None ->
+        Fiber.return
+        @@ User_error.raise
+             [ Pp.text "No program found to extract tar file. Tried:"
+             ; Pp.enumerate [ "tar"; "bsdtar" ] ~f:Pp.verbatim
+             ])
   in
   { command; suffixes = [ ".tar"; ".tar.gz"; ".tgz"; ".tar.bz2"; ".tbz" ] }
 ;;
 
+let which_bsdtar (bin_name : string) =
+  match which bin_name with
+  | None -> Fiber.return None
+  | Some bin ->
+    let+ output, _error = Process.run_capture ~display:Quiet Return bin [ "--version" ] in
+    let re = Re.compile (Re.str "bsdtar") in
+    if Re.execp re output then Some bin else None
+;;
+
 let zip =
   let command =
-    lazy
-      (match which "unzip" with
-       | Some bin -> { Command.bin; make_args = make_zip_args }
-       | None ->
-         (* Fall back to using tar to extract zip archives, which is possible in some cases. *)
-         (match
-            (* Test for bsdtar before tar, as if bsdtar is installed then it's
-               likely that the tar binary is GNU tar which can't extract zip
-               archives, whereas bsdtar can. If bsdtar is absent, try using the
-               tar command anyway, as on MacOS, Windows, and some BSD systems,
-               the tar command can extract zip archives. *)
-            List.find_map [ "bsdtar"; "tar" ] ~f:which
-          with
-          | Some bin -> { Command.bin; make_args = make_tar_args }
-          | None ->
-            (* Still reference unzip in the error message, as installing it
-               is the simplest way to fix the problem. *)
-            Dune_engine.Utils.program_not_found "unzip" ~loc:None))
+    Fiber.Lazy.create (fun () ->
+      match which "unzip" with
+      | Some bin -> Fiber.return { Command.bin; make_args = make_zip_args }
+      | None ->
+        let rec find_tar programs =
+          match programs with
+          | [] -> Fiber.return None
+          | x :: xs ->
+            let* res = which_bsdtar x in
+            (match res with
+             | Some _ -> Fiber.return res
+             | None -> find_tar xs)
+        in
+        let* program = find_tar [ "bsdtar"; "tar" ] in
+        (match program with
+         | Some bin -> Fiber.return { Command.bin; make_args = make_tar_args }
+         | None ->
+           Fiber.return
+           @@ User_error.raise
+                [ Pp.text "No program found to extract zip file. Tried:"
+                ; Pp.enumerate [ "unzip"; "bsdtar"; "tar" ] ~f:Pp.verbatim
+                ]))
   in
   { command; suffixes = [ ".zip" ] }
 ;;
@@ -73,7 +91,7 @@ let choose_for_filename_default_to_tar filename =
 
 let extract t ~archive ~target =
   let* () = Fiber.return () in
-  let command = Lazy.force t.command in
+  let* command = Fiber.Lazy.force t.command in
   let prefix = Path.basename target in
   let target_in_temp =
     let suffix = Path.basename archive in

test/blackbox-tests/test-cases/pkg/zip-extract-fail.t

@@ -0,0 +1,98 @@
+Test the error message when unzip is needed but not installed.
+
+  $ . ./helpers.sh
+
+  $ make_lockdir
+
+  $ mkdir -p .fakebin
+  $ ln -s $(which dune) .fakebin/dune
+  $ ln -s $(which sh) .fakebin/sh
+  $ ln -s $(which cp) .fakebin/cp
+
+  $ echo "random" >> test.txt.zip
+
+  $ makepkg() {
+  > make_lockpkg $1 <<EOF
+  > (source
+  >  (fetch
+  >   (url "file://$(pwd)/test.txt.zip")))
+  > (version dev)
+  > EOF
+  > }
+
+  $ makepkg foo
+
+Build the package in an environment without unzip, or tar, or bsdtar.
+  $ PATH=.fakebin build_pkg foo 2>&1 | grep '^Error:' -A 3
+  Error: No program found to extract zip file. Tried:
+  - unzip
+  - bsdtar
+  - tar
+
+Build with only tar that doesn't work, not bsdtar or unzip, it should still fail to build
+
+  $ cat > .fakebin/tar << 'EOF'
+  > #!/usr/bin/env sh
+  > case $1 in
+  >  --version)
+  >    echo "tar"
+  >  ;;
+  >  *)
+  >    cp "$2" "$4/$(basename "${2%.zip}")"
+  > esac
+  > EOF
+  $ chmod +x .fakebin/tar
+  $ PATH=.fakebin build_pkg foo 2>&1 | grep '^Error:' -A 3
+  Error: No program found to extract zip file. Tried:
+  - unzip
+  - bsdtar
+  - tar
+
+Build with only tar that works, not bsdtar or unzip, it should work
+
+(NOTE: We wrap `(PATH=.fakebin foo)` in parens, otherwise the value of the PATH
+variable can escape to subseqent shell invocations on MacOS.)
+
+  $ cat > .fakebin/tar << 'EOF'
+  > #!/usr/bin/env sh
+  > case $1 in
+  >  --version)
+  >    echo "bsdtar"
+  >  ;;
+  >  *)
+  >    cp "$2" "$4/$(basename "${2%.zip}")"
+  > esac
+  > EOF
+  $ chmod +x .fakebin/tar
+  $ (PATH=.fakebin build_pkg foo)
+
+Build the package with bsdtar and tar. Now our fake bsdtar will get picked up
+and built.
+
+  $ cat > .fakebin/bsdtar << 'EOF'
+  > #!/usr/bin/env sh
+  > case $1 in
+  >  --version)
+  >    echo "bsdtar"
+  >  ;;
+  >  *)
+  >    cp "$2" "$4/$(basename "${2%.zip}")"
+  > esac
+  > EOF
+  $ chmod +x .fakebin/bsdtar
+  $ (PATH=.fakebin build_pkg foo)
+
+Build with unzip
+
+  $ cat > .fakebin/unzip << 'EOF'
+  > #!/usr/bin/env sh
+  > case $1 in
+  >  --version)
+  >    echo "unzip"
+  >  ;;
+  >  *)
+  >    cp "$2" "$4/$(basename "${2%.zip}")"
+  > esac
+  > EOF
+  $ chmod +x .fakebin/unzip
+  $ (PATH=.fakebin build_pkg foo)