Merge pull request ocaml#5984 from dra27/harden-curl-parsing

Harden curl output parsing

master_changes.md

@@ -69,6 +69,7 @@ users)
 
 ## Repository
   * Fix download URLs containing invalid characters on Windows (e.g. the ? character in `?full_index=1`) [#5921 @dra27]
+  * [BUG] Fix curl failures - the progress meter can become interleaved with the status code on Windows [#5984 @dra27 @kit-ty-kate @rjbou]
 
 ## Lock
 
@@ -170,6 +171,7 @@ users)
   * Extracted `OpamSolution.install_sys_packages` from `OpamSolution.install_depexts` [#5994 @dra27]
 
 ## opam-repository
+  * `OpamDownload.download_command`: separate output from stdout and stderr [#5984 @kit-ty-kate]
 
 ## opam-state
 
@@ -190,3 +192,4 @@ users)
   * `OpamConsole.menu` now supports up to 35 menu items [#5992 @dra27]
   * `OpamStd.Sys.resolve_command`: extracted the logic from `OpamSystem.resolve_command`, without the default environment handling from OpamProcess. [#5991 @dra27]
   * `OpamStd.Sys.resolve_in_path`: split the logic of `OpamStd.Sys.resolve_command` to allow searching for an arbitrary file in the search path [#5991 @dra27]
+  * `OpamProcess.run_background`: name the stderr output file have the .err extension when cmd_stdout is given [#5984 @kit-ty-kate]

src/core/opamProcess.ml

@@ -538,11 +538,17 @@ let run_background command =
       in
       Some (Filename.concat d (Printf.sprintf "%s.%s" n ext))
   in
-  let stdout_file =
-    OpamStd.Option.Op.(cmd_stdout >>+ fun () -> file "out")
-  in
-  let stderr_file =
-    if OpamCoreConfig.(!r.merged_output) then file "out" else file "err"
+  let stdout_file, stderr_file =
+    let err () = file "err" in
+    match cmd_stdout with
+    | None ->
+      let out = file "out" in
+      if OpamCoreConfig.(!r.merged_output) then
+        (out, out)
+      else
+        (out, err ())
+    | Some _ as out ->
+      (out, err ())
   in
   let env_file    = file "env" in
   let info_file   = file "info" in

src/repository/opamDownload.ml

@@ -22,15 +22,15 @@ let user_agent =
 
 let curl_args = [
   CString "--write-out", None;
-  CString "%%{http_code}\\n", None;
-  CString "--retry", None; CIdent "retry", None;
-  CString "--retry-delay", None; CString "2", None;
+  CString "%%{http_code}\\n", None; (* 6.5 13-Mar-2000 *)
+  CString "--retry", None; CIdent "retry", None; (* 7.12.3 20-Dec-2004 *)
+  CString "--retry-delay", None; CString "2", None; (* 7.12.3 20-Dec-2004 *)
   CString "--compressed",
-  Some (FIdent (OpamFilter.ident_of_string "compress"));
-  CString "--user-agent", None; user_agent, None;
-  CString "-L", None;
-  CString "-o", None; CIdent "out", None;
-  CString "--", None; (* End list of options *)
+  Some (FIdent (OpamFilter.ident_of_string "compress")); (* 7.10 1-Oct-2002 *)
+  CString "--user-agent", None; user_agent, None; (* 4.5.1 12-Jun-1998 *)
+  CString "-L", None; (* 4.9 7-Oct-1998 *)
+  CString "-o", None; CIdent "out", None; (* 2.3 21-Aug-1997 *)
+  CString "--", None; (* End list of options; 5.0 1-Dec-1998 *)
   CIdent "url", None;
 ]
 
@@ -132,7 +132,9 @@ let download_command ~compress ?checksum ~url ~dst () =
       OpamConsole.error_and_exit `Configuration_error
         "Empty custom download command"
   in
-  OpamSystem.make_command ~allow_stdin:false cmd args @@> tool_return url
+  let stdout = OpamSystem.temp_file ~auto_clean:false "dl" in
+  OpamProcess.Job.finally (fun () -> OpamSystem.remove_file stdout) @@ fun () ->
+  OpamSystem.make_command ~allow_stdin:false ~stdout cmd args @@> tool_return url
 
 let really_download
     ?(quiet=false) ~overwrite ?(compress=false) ?checksum ?(validate=true)