Fallback to copying on symlink-less MSYS2

[jonahbeckford]

Splitting up #4813:

For 92397ac ("Allow MSYS2 to prefer copying over symlinking") I think the code duplication can be reduced if is_cygwin_variant and is_msys2_variant are merged into a single function get_windows_executable_variant : string -> [ Native | Cygwin of [Cygwin | Msys2] | Tainted of [Cygwin | Msys2] ]. The old is_cygwin_variant and the new is_msys2_variant can then be defined in terms of that function. While it's being added, it would be good to thread the tainting support through to OpamSystem.install (the point of CygLinked is to warn for the situation where you've accidentally linked with a Cygwin or MSYS2 compiled DLL which usually implies bindings are wrong somewhere). I was initially confused by the title of the commit, as we must definitely prefer symlinking over copying (if it's available and works), vs requiring symlinking (I completely agree with you that requiring developer mode is an unacceptable barrier), but I think that the actual rsync change works correctly if native symlinking is enabled. The longer-term plan for this (I say longer, it's possible this'll happen in 2.2 or 2.3) is to use ocaml-tar and implement the cp command directly. Cygwin/MSYS2' cp and tar commands are somewhat belligerently staying portable where symlinks are concerned - Windows symlinks do not require the file to exist, what they do require is for the type of the target to be known, which cp does (trivially) and tar can (slightly less-trivially). The only thing which is very hard to copy reliably is a dangling/broken symlink. However, I think your rsync solution is a very good stop-gap!

For continuity this initial PR is as-is from the original PR, except the new title and a change in the commit description:

Default MSYS2 installations "support" symlinking by copying. This
commit adds a two-phase rsync (copy first without symlinks, then
copy symlinks) so the MSYS2 copying behavior works.

[jonahbeckford]

1. Switched to get_windows_executable_variant.
2. I kept is_cygwin_variant and marked it deprecated. It is not used in any current ocaml/opam code but perhaps packages outside of Opam rely on it.

[dra27]

Thanks for the update to this - sorry for the number of comments, though most of them are in fact reverts!

Consider Ubuntu and Debian: most of the time we can just treat as Debian in terms of handling and then very occasionally we care that it's specifically Ubuntu. I think the Msys2/Cygwin API should be the same - most of the time we simply need to know that it's Cygwin-like and we can call that Cygwin. Then only the specific place where you want some handling that's just for Msys2 do we actually match on that. It makes the diff smaller too 🙂

I haven't fleshed out the change to OpamSystem.default_install_warning - I think there should be Msys2-specific messages for that.

[jonahbeckford]

The suggested changes were made.

[dra27]

Apologies for the delay approving this (and thanks to @rjbou for rebasing it!). Thanks for this - the rsync support will either get extended to the Cygwin-side at some point or the whole thing will get replaced with Windows-aware OCaml versions but, in the meantime, this is a great stability improvement!

[dra27]

(it still needs to run through CI, which is presently suffering from another GitHub outage...)

[kit-ty-kate]

Thanks a lot!