search

ocaml / opam

opam is a source-based package manager. It supports multiple simultaneous compiler installations, flexible package constraints, and a Git-friendly development workflow.
https://opam.ocaml.org
Other
1.21k stars 348 forks source link

Harden curl output parsing #5984

Closed dra27 closed 4 weeks ago

dra27 commented 1 month ago

Issue originally reported on Discuss.

The OP was using a native Windows curl.exe from Git-for-Windows git-bash (MSYS2 has curl both for its "Cygwin" main install, but also compiled natively - git-bash installs the mingw64 native-compiled version). This yields an exception during opam init while attempting to setup the internal Cygwin installation:

Fatal error:
OpamDownload.Download_fail(_, "curl: code  while downloading https://cygwin.com/sha512.sum")

Note that there is no code. Running the command manually, everything appears to be OK, but if the output is piped to a file, the issue becomes apparent (the various \r effects removed):

  % Total    % Received % Xferd  Average Speed   Time    Time     Time  Current
                                 Dload  Upload   Total   Spent    Left  Speed
100   291  100   291    0     0    635      0 --:--:-- --:--:-- --:--:--   638200

There is a blank line at the end of the file and the 200 is at the end of the penultimate line. There appears to be an issue with interleaving stderr and stdout when the output is being piped. At this point I could wave my hands and spout Posix, line-buffering and all sorts of explanations, but this PR is a somewhat simpler approach - the status code now has a newline printed before and after it and the parser attempts to convert the first non-blank line from the end of the output to an integer.

I was tempted to add --no-progress-meter, but it turns out that's too recent an option. I'm wondering why --silent has never been added (same for the others), but perhaps that's because if something goes wrong, the display might be useful. We could add --silent --show-error which would add errors only to the log files (--silent is there since the dawn of time, and --show-error was added in 5.9 on 22-May-1999).

kit-ty-kate commented 4 weeks ago

Thanks!