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

Run CI on Windows #156

Merged
merged 5 commits into from
Nov 17, 2024
Merged

Run CI on Windows #156

merged 5 commits into from
Nov 17, 2024

Conversation

aantron
Copy link
Contributor

@aantron aantron commented Sep 3, 2024

Right now, it's not clear how to install this package ssl on a GitHub Actions Windows runner. This should be addressed and maintained in this repo for all downstream users. At the moment, it's not clear which version of SSL is even being linked against by
the environment created by setup-ocaml in GHA. For example, windows-latest at the moment appears to be windows-2022, with software list here, which includes OpenSSL 1.1.1w. Yet trying to build ssl on it fails with the issue in #155, so I suspect some other SSL from the setup-ocaml (Cygwin?) environment is at play. We encountered this in various forms during aantron/dream#337, but it really should be solved upstream, perhaps by having this repo's GHA .yml file be an example.

If the CI build for this PR fails on Windows, what would it take to get it to work?

Long-term, this repo probably doesn't need a full (OS x OCaml) build matrix, but just to include: one row for testing on Windows on a certain OCaml version.

@@ -13,17 +13,19 @@ concurrency:

jobs:
build:
runs-on: ubuntu-latest
Copy link
Collaborator

Choose a reason for hiding this comment

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

I think you still need something like runs-on: ${{ matrix.setup.os }}

@aantron
Copy link
Contributor Author

aantron commented Sep 3, 2024

@anmonteiro Could you take over this? The Windows build on 4.05 failed, but I don't think that's important -- we'd basically like to see the 4.14 and 5.x builds working in some way, 4.05 might have too much bitrot and it would be fine from Dream's point of view to just include: those in the matrix.

The actual 4.14 and 5.x builds were canceled, presumably by 4.05, but I also see a nix_build failure, which, I think, is outside the scope of this PR.

@aantron
Copy link
Contributor Author

aantron commented Sep 3, 2024

I've also opened ocaml/setup-ocaml#856 so that the setup-ocaml maintainers might comment on the environment this is all running in.

@anmonteiro
Copy link
Collaborator

I can try but I’m not sure how far I’ll get, since I don’t have access to a windows machine (or enough interest to spend significant time on it)

@anmonteiro
Copy link
Collaborator

@aantron you can now see the windows failure in CI:

[ERROR] depext is not a known command or plugin (package opam-depext.1.2.3 does not have the 'plugin' flag set).

I have no idea what this means or how to solve it since I don't use opam

@anmonteiro
Copy link
Collaborator

The failure now seems to reproduce what you reported. It would be interesting to understand how to get a newer libssl on windows.

@aantron
Copy link
Contributor Author

aantron commented Sep 5, 2024

Great, thank you! I left a comment about that in setup-ocaml. I'd wait for a response to that, and once that's clarified, we can merge this PR in this repo so that it also keeps checking that ssl is still installable in GHA, which should help everybody downstream.

@aantron
Copy link
Contributor Author

aantron commented Sep 5, 2024

Overextending. I would suggest merging this PR in this repo :) Thanks again!

@anmonteiro
Copy link
Collaborator

I'd be happy to merge this PR once we can get the build to be green

@anmonteiro
Copy link
Collaborator

This seems to progress further after ocaml/setup-ocaml#892 but the github actions run seems to fail even though the test run is successful?

@aantron
Copy link
Contributor Author

aantron commented Nov 12, 2024

The log output doesn't seem to describe the failure. How about temporarily removing the @runtest on with-test and/or other additional flags to try to narrow down which part of the build is causing this?

@anmonteiro anmonteiro merged commit 48759c1 into savonet:master Nov 17, 2024
11 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants