Skip to content

Fix Windows CI build #738

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

Merged
merged 2 commits into from
May 13, 2025
Merged

Fix Windows CI build #738

merged 2 commits into from
May 13, 2025

Conversation

elopez
Copy link
Collaborator

@elopez elopez commented May 10, 2025

Description

Recent MSYS2 updates broke the build again. It turns out libff was being built by referencing the MSYS2-installed headers instead of the GHC-bundled ones. This adjusts the build process to have libff built with the same headers as the rest of hevm.

Checklist

  • tested locally
  • added automated tests
  • updated the docs
  • updated the changelog

Recent MSYS2 updates broke the build again. It turns out libff was
being built by referencing the MSYS2-installed headers instead of
the GHC-bundled ones. This adjusts the build process to have libff
built with the same headers as the rest of hevm.
@elopez elopez marked this pull request as ready for review May 11, 2025 16:37
Copy link
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

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

Looks OK, but isn't there a way to use relative path instead of this absolute path D:/a/_temp/msys64/clang64/include? Can we at least extract it to a variable instead of repeating it 4 times?

Copy link
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

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

OK!

@blishko
Copy link
Collaborator

blishko commented May 13, 2025

Looks like extracting to a variable actually broke the build again?

@elopez
Copy link
Collaborator Author

elopez commented May 13, 2025

yeah, I defined the variable too late in the process 😅 I went ahead and updated it to do it earlier and also reference the path from the action, hopefully it works this time 🤞

Copy link
Collaborator

@blishko blishko left a comment

Choose a reason for hiding this comment

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

Nice!
This is much better, no hardcoded path :)

@blishko blishko merged commit f7ca987 into main May 13, 2025
10 checks passed
@blishko blishko deleted the fix-windows-ci branch May 13, 2025 10:05
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