-
Notifications
You must be signed in to change notification settings - Fork 62
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
Add release build to github workflow #359
Conversation
It turns out the release build wasn't the only issue causing my build to fail. That being said, we should probably be testing that both the debug and release builds work since |
To replicate the errors I was seeing I updated ubuntu to use Ultimately, the -Werror flag should not be in the release build as discussed in #316 |
@jacobmerson This looks good. Thank you. Are you all set with this? |
The pipeline works and I'm set with that. Do you need me to fix the build errors (existing issues in PUMI) before merge? |
Either way (new PR or here) works for me. |
As I track this down I noticed that the compiler is not getting selected correctly. gcc is used for the clang build as well. |
8408519
to
2cd5506
Compare
@cwsmith we are nearing completion with this and you could probably merge as is (although I should cleanup the commit history). One interesting side effect is that the If you are happy to merge as is, I will rebase the commits to make them nice. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jacobmerson LGTM. Thank you.
Should we set fail-fast
to false
so that the other matrix jobs aren't cancelled when gnu-release fails?
https://docs.github.com/en/actions/using-workflows/workflow-syntax-for-github-actions#example-preventing-a-specific-failing-matrix-job-from-failing-a-workflow-run
Lets merge this and then resolve the field_io
error in a separate PR.
The use of CMAKE_INSTALL_PREFIX causes issues when setting the prefix at install time using cmake --install with --prefix
This commit cleans up the github workflow and adds a release build to the build matrix. In addition it fixes an error where the correct compiler was not being selected during build. Additionally, the operating system has been updated to ubuntu-latest and gcc-10 is used.
e5f2ea4
to
c6bfeef
Compare
@cwsmith I don't have merge permissions for core. Can you merge when you get a chance? I squashed the commits into something reasonable. |
Adding release build to GitHub actions
see issue #358 . The current release build fails, but the GitHub actions tests are passing. To make sure we notice these issues cropping up I have (attempted) to add a release build to the GitHub actions workflow.