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

Install Sail from binary in CI #532

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

Timmmm
Copy link
Collaborator

@Timmmm Timmmm commented Aug 29, 2024

This should be a lot faster. It also means the version is pinned properly which wasn't the case before.

I also recommended users to do the same in the README do they don't have to deal with OPAM.

@Timmmm Timmmm force-pushed the user/timh/binary_release branch 3 times, most recently from 5732cb9 to c941569 Compare August 29, 2024 16:24
@Alasdair
Copy link
Collaborator

A trick I use in the Sail CI to speed it up is to cache the ~/.opam directory: https://github.com/rems-project/sail/blob/sail2/.github/workflows/build.yml

@Timmmm Timmmm force-pushed the user/timh/binary_release branch 2 times, most recently from 3340230 to 3535688 Compare August 29, 2024 16:39
@Timmmm
Copy link
Collaborator Author

Timmmm commented Aug 29, 2024

I'm hoping we can get away without OPAM as @jordancarlin suggested.

If this attempt doesn't work I'm writing a Dockerfile so I can try it locally! The edit-build-test cycle for Github CI is ridiculous.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Aug 29, 2024

Well, the C side seemed to work, but the OCaml build fails because it can't find ocamlfind, and we use -use-ocamlfind.

Maybe I'll just finish #509 first.

Copy link

github-actions bot commented Aug 29, 2024

Test Results

396 tests  ±0   396 ✅ ±0   0s ⏱️ ±0s
  4 suites ±0     0 💤 ±0 
  1 files   ±0     0 ❌ ±0 

Results for commit abfb8f5. ± Comparison against base commit bdf95b0.

♻️ This comment has been updated with latest results.

@jordancarlin
Copy link
Contributor

jordancarlin commented Aug 29, 2024

Well, the C side seemed to work, but the OCaml build fails because it can't find ocamlfind, and we use -use-ocamlfind.

Maybe I'll just finish #509 first.

That probably makes sense because otherwise it’s just unnecessary work that’ll be quickly obsolete. Or we just leave opam for now (assuming it has all the needed dependencies).

This should be a lot faster. It also means the version is pinned properly which wasn't the case before.

I also recommended users to do the same in the README do they don't have to deal with OPAM.
Copy link
Contributor

@jordancarlin jordancarlin left a comment

Choose a reason for hiding this comment

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

Looks great. Should make it much easier for new users to get going without needing to worry about Opam. And seems like it speeds up CI by another few minutes too.

@Timmmm
Copy link
Collaborator Author

Timmmm commented Sep 19, 2024

I think this should be ready to go, the only thing I'm not sure about is the Coq stuff. It was last touched 4 years ago by @bacam - Brian do you remember this?

@Timmmm Timmmm marked this pull request as ready for review September 19, 2024 09:42
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.

3 participants