pkg: Improve fetch error messages (#12490)

* Use `command` for referring to the curl binary

Signed-off-by: Marek Kubica <[email protected]>

* Make casing of error messages more consistent

Signed-off-by: Marek Kubica <[email protected]>

---------

Signed-off-by: Marek Kubica <[email protected]>

Author: Leonidas-from-XIV

src/dune_pkg/fetch.ml

@@ -7,12 +7,26 @@ module Curl = struct
       (match Bin.which ~path:(Env_path.path Env.initial) "curl" with
        | Some p -> p
        | None ->
+         let curl = User_message.command "curl" in
+         let sep = Pp.space in
          User_error.raise
-           ~hints:[ Pp.text "Install curl with your system package manager." ]
-           [ Pp.text
-               "The program \"curl\" does not appear to be installed. Dune uses curl to \
-                download packages. Dune requires that the \"curl\" executable be located \
-                in one of the directories listed in the PATH variable."
+           ~hints:
+             [ Pp.concat
+                 ~sep
+                 [ Pp.text "Install"; curl; Pp.text "with your system package manager." ]
+             ]
+           [ Pp.concat
+               ~sep
+               [ Pp.text "The program"
+               ; curl
+               ; Pp.text "does not appear to be installed. Dune uses"
+               ; curl
+               ; Pp.text "to download packages. Dune requires that the"
+               ; curl
+               ; Pp.text
+                   "executable be located in one of the directories listed in the PATH \
+                    variable."
+               ]
            ])
   ;;
 
@@ -89,14 +103,20 @@ module Curl = struct
           [ Pp.text s ]
         | exception s ->
           [ Pp.textf
-              "failed to read stderr form file %s"
+              "Failed to read stderr from file %s"
               (Path.to_string_maybe_quoted stderr)
           ; Exn.pp s
           ]
       in
       Error
         (User_message.make
-           ([ Pp.textf "curl returned an invalid error code %d" exit_code ] @ stderr)))
+           ([ Pp.concat
+                ~sep:Pp.space
+                [ User_message.command "curl"
+                ; Pp.textf "returned an invalid error code %d" exit_code
+                ]
+            ]
+            @ stderr)))
     else (
       Path.unlink_no_err stderr;
       match
@@ -108,12 +128,17 @@ module Curl = struct
       | None ->
         Error
           (User_message.make
-             [ Pp.textf "curl returned an HTTP code we don't understand: %S" http_code ])
+             [ Pp.concat
+                 ~sep:Pp.space
+                 [ User_message.command "curl"
+                 ; Pp.textf "returned an HTTP code we don't understand: %S" http_code
+                 ]
+             ])
       | Some http_code ->
         if http_code = 200
         then Ok ()
         else
-          Error (User_message.make [ Pp.textf "download failed with code %d" http_code ]))
+          Error (User_message.make [ Pp.textf "Download failed with code %d" http_code ]))
   ;;
 end
 
@@ -126,7 +151,7 @@ let label = "dune-fetch"
 let unpack_archive ~archive_driver ~target ~archive =
   Archive_driver.extract archive_driver ~archive ~target
   >>| Result.map_error ~f:(fun () ->
-    Pp.textf "unable to extract %S" (Path.to_string archive))
+    Pp.textf "Unable to extract %s" (Path.to_string_maybe_quoted archive))
 ;;
 
 let check_checksum checksum path =
@@ -180,9 +205,9 @@ let fetch_curl ~unpack:unpack_flag ~checksum ~target (url : OpamUrl.t) =
          let exn =
            User_message.make
              [ Pp.textf
-                 "failed to unpack archive downloaded from %s"
+                 "Failed to unpack archive downloaded from %s"
                  (OpamUrl.to_string url)
-             ; Pp.text "reason:"
+             ; Pp.text "Reason:"
              ; msg
              ]
          in

test/blackbox-tests/test-cases/pkg/compute-checksums-when-missing.t

@@ -51,7 +51,7 @@ Recreate the foo package with a fake port number to signal that the file will
   Package "foo" has source archive which lacks a checksum.
   The source archive will be downloaded from: http://0.0.0.0:9000
   Dune will compute its own checksum for this source archive.
-  Warning: download failed with code 404
+  Warning: Download failed with code 404
   Solution for dune.lock:
   - foo.0.0.1
   $ cat ${default_lock_dir}/foo.pkg

test/blackbox-tests/test-cases/pkg/curl-not-installed.t

@@ -19,8 +19,8 @@ Build the package in an environment without curl.
   File "dune.lock/foo.pkg", line 3, characters 7-28:
   3 |   (url "http://0.0.0.0:8000")))
              ^^^^^^^^^^^^^^^^^^^^^
-  Error: The program "curl" does not appear to be installed. Dune uses curl to
-  download packages. Dune requires that the "curl" executable be located in one
-  of the directories listed in the PATH variable.
-  Hint: Install curl with your system package manager.
+  Error: The program 'curl' does not appear to be installed. Dune uses 'curl'
+  to download packages. Dune requires that the 'curl' executable be located in
+  one of the directories listed in the PATH variable.
+  Hint: Install 'curl' with your system package manager.
   [1]

test/blackbox-tests/test-cases/pkg/fetch-cache.t

@@ -48,7 +48,7 @@ cache, it will fail, as the source is 404 now:
   File "dune.lock/test.pkg", line 4, characters 7-25:
   4 |   (url http://localhost:1)
              ^^^^^^^^^^^^^^^^^^
-  Error: download failed with code 404
+  Error: Download failed with code 404
          
   [1]