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.

Sign up for GitHub

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

Jump to bottom

Better cygwin support in core #5543

Closed
wants to merge 11 commits into from
Closed

Better cygwin support in core #5543

wants to merge 11 commits into from

Conversation

rjbou
Copy link
Collaborator

@rjbou rjbou commented May 10, 2023 •
edited
Loading

  • At opam_init and global state loading, update internal configuration to retrieve cygwin binary path and store it in configuration (OpamCoreConfig)
  • When defined, add cygwin binary path to build environment

TODO

@rjbou rjbou added this to the 2.2.0~alpha milestone May 10, 2023
This was referenced May 10, 2023
Merged
Merged
Draft
@rjbou rjbou added the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label May 11, 2023
@rjbou rjbou removed the PR: QUEUED Pending pull request, waiting for other work to be merged or closed label Jun 7, 2023
rjbou
rjbou commented Jun 12, 2023
View reviewed changes
src/client/opamArg.ml Outdated Show resolved Hide resolved
AltGr
AltGr approved these changes Jun 12, 2023
View reviewed changes
Copy link
Member

@AltGr AltGr left a comment

Choose a reason for hiding this comment

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

There are some tricky bits here that could maybe have some more explanations, but LGTM

src/client/opamArg.ml Outdated Show resolved Hide resolved
src/client/opamArg.ml Outdated Show resolved Hide resolved
src/core/opamSystem.ml Show resolved Hide resolved
rjbou and others added 11 commits June 13, 2023 19:11
@rjbou
core config: Add cygbin, cygwin binary path
737c186
@rjbou
sysinteract: export Cygwin.cygbin_opt
b69aca5
@rjbou
global state: add update of core config cygbin path on state loading
d827629
@rjbou
arg: add update of core config cygbin path at opam init
d1ae400
@rjbou
Std: Add cyg_env function, that gives environment with PATH containin…
8f4ef44 
…g cygwin binary path
@rjbou
compat: add Filename.quote_command
e2b80a6
@dra27 @rjbou
std: Sys functions get_windows_executablea takes a new argument, the …
1f64f68 
…cygwin binary location, and add a function to test directly if given cygwin installation path is a cygwin installation

std: Sys.get_windows_executable_variant, use cygenv to get environment & quote cygcheck
std: Update Sys.get_windows_executable_variant naming

Co-authored-by: Raja Boujbel <[email protected]>
@dra27 @rjbou
process-system: Use default_env that contain cygbin binary into path,…
a3e67d4 
… when defined

Co-authored-by: Raja Boujbel <[email protected]>
@rjbou
action: add cygwin binary path to path for build env
58e4399
@dra27 @rjbou
process: remove assert false in cygwin_create_process_env
2c5e591
@dra27 @rjbou
system: apply_cygpath fix empty output
e4e2976
@rjbou
Copy link
Collaborator Author

rjbou commented Jun 13, 2023

This PR is kept open to simplify review, as CI can't pass, it won't be merged. All commits are in PR #5544.

@rjbou
Copy link
Collaborator Author

rjbou commented Jun 19, 2023

This PR is merged as a subpart of #5544 ; it was split to ease review

@rjbou rjbou closed this Jun 19, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@AltGr AltGr AltGr approved these changes

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
2.2.0~alpha
Development

Successfully merging this pull request may close these issues.
3 participants
@rjbou @AltGr @dra27