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

Change default rule for PKG_CONFIG_PATH and MANPATH to target #5972

Merged
merged 1 commit into from
Jun 6, 2024

Conversation

dra27
Copy link
Member

@dra27 dra27 commented May 24, 2024

Neither of these systems supports quoting (on Windows or Unix).

This should address the regression in #5923, but it's not the final fix for Windows, so that issue should stay open after this is merged.

cc @RyanGibb

@dra27 dra27 added this to the 2.2.0~beta3 milestone May 24, 2024
@dra27 dra27 added this to For beta3 in Opam 2.2.0 May 24, 2024
Copy link
Collaborator

@rjbou rjbou left a comment

Choose a reason for hiding this comment

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

Thanks!

### opam exec -- opam env --revert | grep MANPATH
MANPATH=''; export MANPATH;
Copy link
Member

Choose a reason for hiding this comment

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

is this expected?

Copy link
Member Author

Choose a reason for hiding this comment

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

No, that was clumsy of me. I think the Target above should be Host, but I need to test that (especially w.r.t. MSYS2)

Neither of these systems supports quoting (on Windows or Unix)
@dra27 dra27 marked this pull request as ready for review June 6, 2024 19:14
@dra27
Copy link
Member Author

dra27 commented Jun 6, 2024

The switch to Host restores the MANPATH revert and gets us to:

set MANPATH=:/cygdrive/c/Users/DRA/AppData/Local/opam/testing-mingw/man

on Cygwin, and

set MANPATH=:/c/Devel/Roots/msys2-testing/testing/man

on MSYS2, which should be correct. i.e. the directories are colon-separated, but are converted using cygpath.

@kit-ty-kate
Copy link
Member

Thanks!

@kit-ty-kate kit-ty-kate merged commit 3aa169e into ocaml:master Jun 6, 2024
29 checks passed
Opam 2.2.0 automation moved this from For beta3 to Done Jun 6, 2024
@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
No open projects
Opam 2.2.0
  
Done
Development

Successfully merging this pull request may close these issues.

None yet

3 participants