Quote command line arguments if necessary when printing them

src/core/opamProcess.ml

@@ -211,7 +211,23 @@ type command = {
   cmd_metadata: (string * string) list option;
 }
 
-let string_of_command c = String.concat " " (c.cmd::c.args)
+let string_of_cmd cmd args =
+  let is_not_shell_special_char = function
+    | '0'..'9' | 'a'..'z' | 'A'..'Z'
+    | '+' | '.' | '/' | '@' | '_' -> true
+    | '\\' -> Sys.win32
+    | '%' | ',' | '-' | '^' -> not Sys.win32
+    | _ -> false
+  in
+  let print x =
+    if String.for_all is_not_shell_special_char x then
+      x
+    else
+      Filename.quote x
+  in
+  print cmd ^ List.fold_left (fun acc x -> acc ^ " " ^ print x) "" args
+
+let string_of_command c = string_of_cmd c.cmd c.args
 let text_of_command c = c.cmd_text
 let default_verbose () = OpamCoreConfig.(!r.verbose_level) >= 2
 let is_verbose_command c =
@@ -225,7 +241,7 @@ let make_command_text ?(color=`green) str ?(args=[]) cmd =
           not (String.contains s '/') && not (String.contains s '='))
         args
     with
-    | hd::_ -> String.concat " " [cmd; hd]
+    | hd::_ -> string_of_cmd cmd [hd]
     | [] -> cmd
   in
   Printf.sprintf "[%s: %s]" (OpamConsole.colorise color str) summary
@@ -281,7 +297,7 @@ let make_info ?code ?signal
 
   List.iter (fun (k,v) -> print k v) metadata;
   print     "path"         cwd;
-  print     "command"      (String.concat " " (cmd :: args));
+  print     "command"      (string_of_cmd cmd args);
   print_opt "exit-code"    (OpamStd.Option.map string_of_int code);
   print_opt "signalled"    (OpamStd.Option.map string_of_int signal);
   print_opt "env-file"     env_file;

src/core/opamProcess.mli

@@ -11,6 +11,8 @@
 
 (** Process and job handling, with logs, termination status, etc. *)
 
+val string_of_cmd : string -> string list -> string
+
 (** The type of shell commands *)
 type command = private {
   cmd: string;

src/core/opamSystem.ml

@@ -570,7 +570,7 @@ let run_process
              ~env ~name ~verbose ?metadata ?allow_stdin ?stdout
              full_cmd args)
       in
-      let str = String.concat " " (cmd :: args) in
+      let str = OpamProcess.string_of_cmd cmd args in
       log ~level:2 "[%a] (in %.3fs) %s"
         (OpamConsole.slog Filename.basename) name
         (chrono ()) str;

src/state/opamSysInteract.ml

@@ -1117,7 +1117,7 @@ let sudo_run_command ?(env=OpamVariable.Map.empty) ?vars cmd args =
     | `AsAdmin cmd, Some ("linux" | "unix" | "freebsd" | "netbsd" | "dragonfly" | "macos") when not_root ->
       if OpamSystem.resolve_command "sudo" = None then
         "su",
-        ["root"; "-c"; Printf.sprintf "%S" (String.concat " " (cmd::args))]
+        ["root"; "-c"; Printf.sprintf "%S" (OpamProcess.string_of_cmd cmd args)]
       else
         "sudo", cmd::args
     | (`AsUser cmd | `AsAdmin cmd), _ -> cmd, args
@@ -1127,7 +1127,7 @@ let sudo_run_command ?(env=OpamVariable.Map.empty) ?vars cmd args =
   | code ->
     Printf.ksprintf failwith
       "failed with exit code %d at command:\n    %s"
-      code (String.concat " " (cmd::args))
+      code (OpamProcess.string_of_cmd cmd args)
 
 let install ?env config packages =
   if OpamSysPkg.Set.is_empty packages then

tests/reftests/assume-built.test

@@ -34,7 +34,7 @@ Processing  1/3: [ongoing.dev: git]
 -> retrieved ongoing.dev  (no changes)
 Processing  2/3: [ongoing: test ongoing.txt]
 + test "-f" "ongoing.txt" (CWD=${BASEDIR}/OPAM/assume-built/.opam-switch/build/ongoing.dev)
-Processing  2/3: [ongoing: sh cat ongoing.txt > out]
+Processing  2/3: [ongoing: sh 'cat ongoing.txt > out']
 + sh "-c" "cat ongoing.txt > out" (CWD=${BASEDIR}/OPAM/assume-built/.opam-switch/build/ongoing.dev)
 -> compiled  ongoing.dev
 Processing  3/3: [ongoing: test out]

tests/reftests/env.unix.test

@@ -70,7 +70,7 @@ The following actions will be performed:
   - install col-target 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [col-target: sh env | grep RCT_ENV | sort]
+Processing  2/3: [col-target: sh 'env | grep RCT_ENV | sort']
 + sh "-c" "env | grep RCT_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target.1)
 - RCT_ENVBUILD=/a/given/path
 - RCT_ENVBUILD_ADD=/a/given/path:a/path/to
@@ -137,7 +137,7 @@ The following actions will be performed:
   - install col-target-quoted 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [col-target-quoted: sh env | grep RCTQ_ENV | sort]
+Processing  2/3: [col-target-quoted: sh 'env | grep RCTQ_ENV | sort']
 + sh "-c" "env | grep RCTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-target-quoted.1)
 - RCTQ_ENVBUILD=/a/given/path
 - RCTQ_ENVBUILD_ADD=/a/given/path:a/path/to
@@ -204,7 +204,7 @@ The following actions will be performed:
   - install col-host 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [col-host: sh env | grep RCH_ENV | sort]
+Processing  2/3: [col-host: sh 'env | grep RCH_ENV | sort']
 + sh "-c" "env | grep RCH_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host.1)
 - RCH_ENVBUILD=/a/given/path
 - RCH_ENVBUILD_ADD=/a/given/path:a/path/to
@@ -271,7 +271,7 @@ The following actions will be performed:
   - install col-host-quoted 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [col-host-quoted: sh env | grep RCHQ_ENV | sort]
+Processing  2/3: [col-host-quoted: sh 'env | grep RCHQ_ENV | sort']
 + sh "-c" "env | grep RCHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/col-host-quoted.1)
 - RCHQ_ENVBUILD=/a/given/path
 - RCHQ_ENVBUILD_ADD=/a/given/path:a/path/to
@@ -335,7 +335,7 @@ The following actions will be performed:
   - install semicol-target 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [semicol-target: sh env | grep RST_ENV | sort]
+Processing  2/3: [semicol-target: sh 'env | grep RST_ENV | sort']
 + sh "-c" "env | grep RST_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target.1)
 - RST_ENVBUILD=/a/given/path
 - RST_ENVBUILD_ADD=/a/given/path;a/path/to
@@ -402,7 +402,7 @@ The following actions will be performed:
   - install semicol-target-quoted 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [semicol-target-quoted: sh env | grep RSTQ_ENV | sort]
+Processing  2/3: [semicol-target-quoted: sh 'env | grep RSTQ_ENV | sort']
 + sh "-c" "env | grep RSTQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-target-quoted.1)
 - RSTQ_ENVBUILD=/a/given/path
 - RSTQ_ENVBUILD_ADD=/a/given/path;a/path/to
@@ -469,7 +469,7 @@ The following actions will be performed:
   - install semicol-host 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [semicol-host: sh env | grep RSH_ENV | sort]
+Processing  2/3: [semicol-host: sh 'env | grep RSH_ENV | sort']
 + sh "-c" "env | grep RSH_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host.1)
 - RSH_ENVBUILD=/a/given/path
 - RSH_ENVBUILD_ADD=/a/given/path;a/path/to
@@ -536,7 +536,7 @@ The following actions will be performed:
   - install semicol-host-quoted 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [semicol-host-quoted: sh env | grep RSHQ_ENV | sort]
+Processing  2/3: [semicol-host-quoted: sh 'env | grep RSHQ_ENV | sort']
 + sh "-c" "env | grep RSHQ_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/semicol-host-quoted.1)
 - RSHQ_ENVBUILD=/a/given/path
 - RSHQ_ENVBUILD_ADD=/a/given/path;a/path/to
@@ -603,7 +603,7 @@ The following actions will be performed:
   - install false 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [false: sh env | grep RF_ENV | sort]
+Processing  2/3: [false: sh 'env | grep RF_ENV | sort']
 + sh "-c" "env | grep RF_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/false.1)
 - RF_ENVBUILD=/a/given/path
 - RF_ENVBUILD_ADD=/a/given/path:a/path/to
@@ -672,7 +672,7 @@ The following actions will be performed:
   - install rewrite 1
 
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
-Processing  2/3: [rewrite: sh env | grep RO_ENV | sort]
+Processing  2/3: [rewrite: sh 'env | grep RO_ENV | sort']
 + sh "-c" "env | grep RO_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/rewrite.1)
 - RO_ENVBUILD=/another/given/path
 - RO_ENVBUILD_COL=s:mething
@@ -745,7 +745,7 @@ The following actions will be performed:
   - install all-formulae 1
 [ERROR] Formula can't be completely resolved : RAF_ENVBUILD_UNRES ":" true | ";" true. Using default ':' 'target'.
 [ERROR] Formula can't be completely resolved : RAF_ENVBUILD_UNRES ("host" true | "target-quoted" true). Using default ':' 'target'.
-Processing  2/3: [all-formulae: sh env | grep RAF_ENV | sort]
+Processing  2/3: [all-formulae: sh 'env | grep RAF_ENV | sort']
 + sh "-c" "env | grep RAF_ENV | sort" (CWD=${BASEDIR}/OPAM/rewriting/.opam-switch/build/all-formulae.1)
 [ERROR] Formula can't be completely resolved : RAF_ENVSET_UNRES ("host" true | "target-quoted" true). Using default ':' 'target'.
 [ERROR] Formula can't be completely resolved : RAF_ENVSET_UNRES ":" true | ";" true. Using default ':' 'target'.

tests/reftests/run.ml

@@ -207,7 +207,7 @@ let command
     List.iter print_endline out;
   let out = String.concat "\n" out in
   if not (List.mem ret allowed_codes) then
-    raise (Command_failure (ret, String.concat " " (cmd :: args), out))
+    raise (Command_failure (ret, OpamProcess.string_of_cmd cmd args, out))
   else
     out
 

tests/reftests/swhid.unix.test

@@ -39,7 +39,7 @@ The following actions will be performed:
 <><> Processing actions <><><><><><><><><><><><><><><><><><><><><><><><><><><><>
 [ERROR] Failed to get sources of snappy-ko.2: curl failed
 
-OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
+OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out '%{http_code}\\n' --retry 3 --retry-delay 2 --user-agent 'opam/current' -L -o ${OPAMTMP}/url.tar.gz.part -- 'https://fake.exe/url.tar.gz'\" exited with code 6)")
 
 
 <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>
@@ -68,7 +68,7 @@ The following actions will be performed:
 Processing  1/3: [snappy-swhid-dir.2: http]
 [ERROR] Failed to get sources of snappy-swhid-dir.2: curl failed
 
-OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out %{http_code}\\n --retry 3 --retry-delay 2 --user-agent opam/current -L -o ${OPAMTMP}/url.tar.gz.part -- https://fake.exe/url.tar.gz\" exited with code 6)")
+OpamSolution.Fetch_fail("https://fake.exe/url.tar.gz (curl failed: \"curl --write-out '%{http_code}\\n' --retry 3 --retry-delay 2 --user-agent 'opam/current' -L -o ${OPAMTMP}/url.tar.gz.part -- 'https://fake.exe/url.tar.gz'\" exited with code 6)")
 
 
 <><> Error report <><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>