Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Opam 2.2 init with MSYS2 #5843

Merged
merged 7 commits into from
Mar 4, 2024
Merged

Opam 2.2 init with MSYS2 #5843

merged 7 commits into from
Mar 4, 2024

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented Feb 15, 2024

Opam 2.2 init on Windows is focused on resolving a pre-existent Cygwin install, or install opam internal one. Functions that detect if a Cygwin install is present are strictly checking a Cygwin one. This behaviour broke opam usage under MSYS2. As MSYS2 Cygwin is a Cygwin variant, it is not detected, and no specific handling of paths, syscall, etc. are done.

MSYS2 is not officially supported (not testing it in a regular basis), we should ensure that we can keep MSYS2 users to have a way to use opam.

This PR fixes that behaviour by relaxing Cygwin install checks to accept variants. As for Cygwin setup for opam init, a check of the install is done, and to keep the information, the config file is appended with

global-variables: [os-distribution "msys2" "Set by opam init"]

to be sure that further invocations of opam we don't loose that we are in an MSYS2 context (related to #5348).

ftm in the PR, there is no sys-package-manager-cmd setting, user can set it by hand using opam option. It is an open question.

fixes #5683

/cc @jonahbeckford

@kit-ty-kate
Copy link
Member

I have trouble testing this PR.

How are users of MSYS2 expected to use opam?

@jonahbeckford
Copy link
Contributor

Thanks!

I am going to set expectations that I won't be able to look at this for at least a week. Most likely two weeks. But I will get to it!

@jonahbeckford
Copy link
Contributor

Yep, same issues that @kit-ty-kate reported.

WDAGUtilityAccount@37bfe102-8339-4958-b1e5-11ac3588b96d CLANG64 /c/Users/WDAGUtilityAccount/Tools/opam
# ./opam init --yes --no-setup --bare --disable-sandboxing '--git-location=/c/Program Files/Git/cmd' --cygwin-location=/c/msys64 default https://opam.ocaml.org
[WARNING] Flag --cygwin-location is experimental, there is no guarantee that it will be kept; avoid using it in scripts.
[WARNING] Flag --git-location is experimental, there is no guarantee that it will be kept; avoid using it in scripts.
No configuration file found, using built-in defaults.

<><> Git ><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><><>  🐫
Using Git from C:\Program Files\Git\cmdYou can change that later with 'opam option "git-location=C:\A\Path\bin"'
<><> Unix support infrastructure ><><><><><><><><><><><><><><><><><><><><><>  🐫
[ERROR] Error while checking Cygwin install (C:\cygwin64): bin\cygcheck.exe not found in C:\msys64

It clearly though is using cygpath to translate paths which is the main thing we wanted.

Test Apparatus (for me)
.\install-vcruntime.cmd
.\install-msys2.cmd
.\install-winget.cmd
.\install-git.cmd
cd opam # Unpack the .zip from GitHub Actions
C:\msys64\msys2_shell.cmd -here -clang64

@rjbou
Copy link
Collaborator Author

rjbou commented Feb 27, 2024

From dev meeting, there is 2 todos in this PR:

  • lookup for cygcheck in <path>/usr/bin too (ftm is is only in <path> and <path>/bin)
  • add a lookup mechanism for determining pacman location, and confirm with user.

…tall is a Cygwin variant install (e.g.MSYS2)
…s-distribution=msys2` in `global-variables` config file field
…pacman path and store it as system package manager in config file
If MSYS2 is detected via 'os-distribution' in the config file, resolve
'cygcheck' from the environment and store resulting value as Cygwin
binary path in 'OpamCoreConfig.cygbin'
@rjbou
Copy link
Collaborator Author

rjbou commented Feb 29, 2024

Updated PR, previous 2 points resolved.

Copy link
Member

@kit-ty-kate kit-ty-kate left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks a lot! It works really well as far as i tested! (being: opam init --bare, opam switch create --empty, opam install conf-autoconf)

@kit-ty-kate
Copy link
Member

Thanks!

@kit-ty-kate kit-ty-kate merged commit 314500f into ocaml:master Mar 4, 2024
39 checks passed
@s5bug
Copy link

s5bug commented Apr 7, 2024

When this makes it to a build, what will the procedure be for ripping out the OPAM Cygwin and configuring it to use my MSYS2?

@kit-ty-kate
Copy link
Member

@s5bug something like:

opam init '--cygwin-location=C:\msys64'

and without the option (a simple opam init) you'll be asked if you want to install the default Cygwin (recommended) or provide a custom location.

@dra27 dra27 mentioned this pull request Jun 13, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

opam init with MSYS2
4 participants