Harden curl output parsing

[dra27]

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).

[rjbou]

Suggested change

	let num = try int_of_string code with Failure _ -> 999 in
	if num >= 400 then
	fail (Some ("curl error code " ^ code),
	Printf.sprintf "curl: code %s while downloading %s"
	code (OpamUrl.to_string url))
	else Done ()
	try
	if int_of_string code >= 400 then
	fail (Some ("curl error code " ^ code),
	Printf.sprintf "curl: code %s while downloading %s"
	code (OpamUrl.to_string url))
	else Done ()
	with Failure _ ->
	fail (Some "No error code",
	Printf.sprintf "curl: no error code while downloading %s"
	(OpamUrl.to_string url))

Like that we give the 'no code' information instead of an unknown 999 code

[dra27]

Hah - I debated whether the 999 thing should remain as well, so the fact you also thought it suggests it really ought to go!

I pushed an extra commit, but switching it to have fewer try blocks (not necessarily elegantly, though). It also removes the separation of "no output" from "no code parsed", which I think is also fine, as we don't really care about the distinction.

[rjbou]

Thanks!

[kit-ty-kate]

Thanks!