Add curl to the list of required cygwin packages to avoid issues with Windows' curl

[kit-ty-kate]

Fixes #6120
Backported in opam 2.2 by #6143

[dra27]

I think this is probably the way it'll have to go - two things which ought to be checked:

• Does opam init --reinit catch this (e.g. external Cygwin or MSYS2 which doesn't have curl installed; opam init with 2.2.0; does opam init --reinit prompt to add the curl package
• Is it still possible to use OPAMFETCH to force the use of either wget or the system's curl (it ought to be; I just haven't checked)

[kit-ty-kate]

• Does opam init --reinit catch this (e.g. external Cygwin or MSYS2 which doesn't have curl installed; opam init with 2.2.0; does opam init --reinit prompt to add the curl package

I've tried this and it's not pulled by opam init --reinit if curl is missing. I'll have a look at a solution

• Is it still possible to use OPAMFETCH to force the use of either wget or the system's curl (it ought to be; I just haven't checked)

OPAMCURL=C:\Windows\System32\curl.exe and OPAMFETCH=wget both worked as expected

[kit-ty-kate]

The last commit makes opam init --reinit re-run setup.exe and after trying it locally i can confirm this will reinstall curl if it is not installed yet.

[kit-ty-kate]

Note for reviewers: the diff is easier to read with the "Hide whitespace" option on.

[dra27]

Thanks, @kit-ty-kate! I think the additional code for opam init --reinit will be worth checking and integrating regardless, but I just found curl/curl#13845 ... which makes sense, since Windows 11 has cURL 8.8.0, but my Windows 10 box has 8.0.1.

[kit-ty-kate]

but I just found curl/curl#13845 ... which makes sense, since Windows 11 has cURL 8.8.0, but my Windows 10 box has 8.0.1.

ah interesting, so should we just wait for Microsoft to fix this? I'm not sure how frequent are their updates though

[dra27]

I don't know what the policy (or if it's published), but the current version installed is from May 2024, so it's at least happened once recently! The problem with "forcing" Cygwin's curl is the same as with git - it's the wrong SSL stack.

I just had a closer look at the --reinit part - it's not quite what I was thinking... I (think I!) was thinking that this part:

opam/src/client/opamClient.ml

Lines 1504 to 1516 in 6effccc

	let config, mechanism, cygwin_packages, git_location =
	let mechanism, cygwin_packages =
	match mechanism with
	| `Internal pkgs ->
	`Internal, pkgs
	| (`Root _ | `Path _) as mechanism ->
	let cygwin_packages =
	if cygwin_is_tweakable && not OpamStateConfig.(!r.no_depexts) then
	OpamInitDefaults.required_packages_for_cygwin
	else
	[]
	in
	mechanism, cygwin_packages

would want altering so that even with an internal installation it does the depext check at:

opam/src/client/opamClient.ml

Line 1626 in 6effccc

check_for_sys_packages config system_packages;

would automatically add the new packages - i.e. doing it in OpamClient rather than at OpamSysInteract. I'm not sure, but does the present commit mean that opam init --reinit now always re-runs Cygwin setup?

[kit-ty-kate]

I'm not sure, but does the present commit mean that opam init --reinit now always re-runs Cygwin setup?

It does but i personally view it as a feature. As far as i know we currently have no way to update cygwin itself except by running the setup.exe binary itself and give all the correct paths to it, or by installing a depext. I feel like having opam init --reinit do that would be fairly reasonable.
Although looking into it more it seems that it's not the case here either but it should be if --upgrade-also where given to the setup.exe incantation 🪄

[dra27]

Yes, indeed - I think re-running setup for the internal Cygwin installation is a good idea .. but, as you note, it needs --upgrade-also! However, the check higher up is also needed for the MSYS2/external-Cygwin case.

[kit-ty-kate]

Converted to draft as i think the original issue is better fixed by #6168. However i'm keeping this around as we might need want to think about the upgrade mechanism again at some point