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

Add windows build to Build & Test workflow. #359

Merged
merged 10 commits into from
Jan 27, 2025

Conversation

nirosys
Copy link
Contributor

@nirosys nirosys commented Jan 25, 2025

Issue #, if available: #358, #255

Description of changes:
This PR adds a windows target to our Build & Test workflow, and addresses a couple issues that got in the way of successful windows builds while testing for this PR.

Some more information about the issues that were fixed can be found in #358. The first issue was due to GCC/Clang allowing named initializers (which are now a C++20 feature, but supported in C99) in C++, while MSVC does not.

The second issue was a result of refactoring dependencies a while ago, which resulted in google test being imported into the build tree earlier than a key option (gtest_force_shared_crt) to not be set until after google test was imported, resulting in it not taking effect, and causing a linking error. I've moved that setting up to the top-level CMakeLists.txt.


By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

@nirosys nirosys marked this pull request as ready for review January 27, 2025 20:38
jobarr-amzn
jobarr-amzn previously approved these changes Jan 27, 2025
.github/workflows/main.yml Outdated Show resolved Hide resolved
Previously this was required due to an issue with the version API and how it constructed the version at build time.

Co-authored-by: Joshua Barr <[email protected]>
@nirosys nirosys merged commit 1bbcf42 into amazon-ion:master Jan 27, 2025
12 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