Skip to content

Only download Cygwin's setup.exe when the command is actually going to be displayed or used#6467

Merged
kit-ty-kate merged 1 commit intoocaml:masterfrom
kit-ty-kate:reftests-no-cygwin-download
Apr 9, 2025
Merged

Only download Cygwin's setup.exe when the command is actually going to be displayed or used#6467
kit-ty-kate merged 1 commit intoocaml:masterfrom
kit-ty-kate:reftests-no-cygwin-download

Conversation

@kit-ty-kate
Copy link
Copy Markdown
Member

opam only look at os-distribution to know if it should download Cygwin's setup.exe or not:

if OpamSysPoll.os_distribution env = Some "cygwin" then

if OpamSysPoll.os_distribution env = Some "cygwin" then

The tests have been failing quite frequently over the past two days due to some network issues with cygwin.com. Setting os-distribution to not be cygwin should alleviate this.

@kit-ty-kate kit-ty-kate added this to the 2.4.0~alpha1 milestone Apr 8, 2025
@kit-ty-kate kit-ty-kate requested a review from rjbou April 8, 2025 21:51
@rjbou
Copy link
Copy Markdown
Collaborator

rjbou commented Apr 9, 2025

I think its better to move those check directly in OpamSysInteract.install, like that it is checked (and downloaded) only when really needed.

| Cygwin ->
(* We use setup_x86_64 to install package instead of `cygcheck` that is
stored in `sys-pkg-manager-cmd` field *)
let is_internal = Cygwin.is_internal config in
[`AsUser (OpamFilename.to_string (Cygwin.cygsetup ())),

@kit-ty-kate
Copy link
Copy Markdown
Member Author

This is orthogonal to this PR though. Could we first deal with the CI failures and improve internals later? If you're fine with that please feel free to open a ticket so we can remember to do it

@kit-ty-kate kit-ty-kate force-pushed the reftests-no-cygwin-download branch from 4c1ad9d to 91f83fa Compare April 9, 2025 15:05
@kit-ty-kate kit-ty-kate changed the title Make the reftests more reliable by not downloading Cygwin's setup.exe on Windows Only download Cygwin's setup.exe when the command is actually going to be displayed or used Apr 9, 2025
@kit-ty-kate kit-ty-kate force-pushed the reftests-no-cygwin-download branch from 91f83fa to 562aab3 Compare April 9, 2025 15:06
Copy link
Copy Markdown
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!

@kit-ty-kate kit-ty-kate force-pushed the reftests-no-cygwin-download branch from 562aab3 to 8a44523 Compare April 9, 2025 16:53
@kit-ty-kate kit-ty-kate merged commit e1f60b9 into ocaml:master Apr 9, 2025
54 checks passed
@kit-ty-kate kit-ty-kate deleted the reftests-no-cygwin-download branch April 9, 2025 17:43
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.

2 participants