env: disable carriage return for bash/zsh on windows

master_changes.md

@@ -48,6 +48,9 @@ users)
 
 ## Exec
 
+## Env
+  * Fix `opam env` containing carriage return on Cygwin [#5715 @rjbou - fix #5684]
+
 ## Source
 
 ## Lint
@@ -131,3 +134,4 @@ users)
 ## opam-core
   * `OpamSystem.apply_cygpath`: runs `cygpath` over the argument [#5723 @dra27 - function itself added in #3348]
   * Add `OpamConsole.disable_carriage_return` to create a new stdout in binary mode
+  * `OpamConsole.msg`: Add an optional `?stdout` argument to specify which channel to use

src/client/opamCommands.ml

@@ -3866,7 +3866,7 @@ let lint cli =
         OpamConsole.error_and_exit `Bad_arguments
           "--package and a file argument are incompatible"
     in
-    let msg = if normalise then OpamConsole.errmsg else OpamConsole.msg in
+    let msg fmt = if normalise then OpamConsole.errmsg fmt else OpamConsole.msg fmt in
     let json =
       match OpamClientConfig.(!r.json_out) with
       | None -> None

src/client/opamConfigCommand.ml

@@ -67,29 +67,37 @@ let possibly_unix_path_env_value k v =
     (Lazy.force OpamSystem.get_cygpath_path_transform) ~pathlist:true v
   else v
 
-let rec print_env = function
+let rec print_env_t ~stdout = function
   | [] -> ()
   | (k, v, comment) :: r ->
     if OpamConsole.verbose () then
-      OpamStd.Option.iter (OpamConsole.msg ": %s;\n") comment;
+      OpamStd.Option.iter (OpamConsole.msg ~stdout ": %s;\n") comment;
     if not (List.exists (fun (k1, _, _) -> k = k1) r) || OpamConsole.verbose ()
     then (
       let v' = possibly_unix_path_env_value k v in
-      OpamConsole.msg "%s='%s'; export %s;\n"
+      OpamConsole.msg ~stdout "%s='%s'; export %s;\n"
         k (OpamStd.Env.escape_single_quotes v') k);
-    print_env r
+    print_env_t ~stdout r
+
+let print_env env =
+  OpamConsole.disable_carriage_return @@ fun ~stdout ->
+  print_env_t ~stdout env
 
-let rec print_csh_env = function
+let rec print_csh_env_t ~stdout = function
   | [] -> ()
   | (k, v, comment) :: r ->
     if OpamConsole.verbose () then
-      OpamStd.Option.iter (OpamConsole.msg ": %s;\n") comment;
+      OpamStd.Option.iter (OpamConsole.msg ~stdout ": %s;\n") comment;
     if not (List.exists (fun (k1, _, _) -> k = k1) r) || OpamConsole.verbose ()
     then (
       let v' = possibly_unix_path_env_value k v in
-      OpamConsole.msg "setenv %s '%s';\n"
+      OpamConsole.msg ~stdout "setenv %s '%s';\n"
         k (OpamStd.Env.escape_single_quotes v'));
-    print_csh_env r
+    print_csh_env_t ~stdout r
+
+let print_csh_env env =
+  OpamConsole.disable_carriage_return @@ fun ~stdout ->
+  print_csh_env_t ~stdout env
 
 let rec print_pwsh_env = function
   | [] -> ()
@@ -135,10 +143,10 @@ let print_sexp_env env =
   aux env;
   OpamConsole.msg ")\n"
 
-let rec print_fish_env env =
+let rec print_fish_env_t ~stdout env =
   let set_arr_cmd ?(modf=fun x -> x) k v =
     let v = modf @@ OpamStd.String.split v ':' in
-    OpamConsole.msg "set -gx %s %s;\n" k
+    OpamConsole.msg ~stdout "set -gx %s %s;\n" k
       (OpamStd.List.concat_map " "
          (fun v ->
             Printf.sprintf "'%s'"
@@ -147,7 +155,7 @@ let rec print_fish_env env =
   in
   (* set manpath if and only if fish version >= 2.7 *)
   let manpath_cmd v =
-    OpamConsole.msg "%s" (
+    OpamConsole.msg ~stdout "%s" (
       (* test for existence of `argparse` builtin, introduced in fish 2.7 .
        * use `grep' instead of `builtin string match' so that old fish versions do not
        *     produce unwanted error messages on stderr.
@@ -171,9 +179,13 @@ let rec print_fish_env env =
        | "MANPATH" ->
          manpath_cmd v
        | _ ->
-         OpamConsole.msg "set -gx %s '%s';\n"
+         OpamConsole.msg ~stdout "set -gx %s '%s';\n"
            k (OpamStd.Env.escape_single_quotes ~using_backslashes:true v));
-    print_fish_env r
+    print_fish_env_t ~stdout r
+
+let print_fish_env env =
+  OpamConsole.disable_carriage_return @@ fun ~stdout ->
+  print_fish_env_t ~stdout env
 
 let print_eval_env ~csh ~sexp ~fish ~pwsh ~cmd env =
   let env = (env : OpamTypes.env :> (string * string * string option) list) in

src/core/opamConsole.ml

@@ -324,7 +324,7 @@ let get_win32_console_shim :
 let is_windows_10 =
   lazy (let (v, _, _, _) = OpamStubs.getWindowsVersion () in v >= 10)
 
-let win32_print_message ch msg =
+let win32_print_message ?(stdout=stdout) ch msg =
   let ocaml_ch =
     match ch with
     | `stdout -> stdout
@@ -521,13 +521,15 @@ let clear_status =
 
 let print_message =
   if Sys.win32 then
-    fun ch fmt ->
+    fun ?(stdout=stdout) ch fmt ->
       flush (if ch = `stdout then stderr else stdout);
+      (* clear_status also uses [stdout] but the rabbit hole went way too far so i gave up *)
+      (* TODO: clean this *)
       clear_status ();
       (* win32_print_message *always* flushes *)
-      Printf.ksprintf (win32_print_message ch) fmt
+      Printf.ksprintf (win32_print_message ~stdout ch) fmt
   else
-    fun ch fmt ->
+    fun ?(stdout=stdout) ch fmt ->
       let output_string =
         let output_string ch s =
           output_string ch s;
@@ -628,7 +630,7 @@ let error_and_exit reason fmt =
     OpamStd.Sys.exit_because reason
   ) fmt
 
-let msg fmt = print_message `stdout fmt
+let msg ?stdout fmt = print_message ?stdout `stdout fmt
 
 let formatted_msg ?indent fmt =
   Printf.ksprintf

src/core/opamConsole.mli

@@ -96,7 +96,7 @@ val errmsg : ('a, unit, string, unit) format4 -> 'a
 
 val error_and_exit :
   OpamStd.Sys.exit_reason -> ('a, unit, string, 'b) format4 -> 'a
-val msg : ('a, unit, string, unit) format4 -> 'a
+val msg : ?stdout:out_channel -> ('a, unit, string, unit) format4 -> 'a
 val formatted_msg : ?indent:int -> ('a, unit, string, unit) format4 -> 'a
 val header_msg : ('a, unit, string, unit) format4 -> 'a
 val header_error :