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

hot-fix: Use cmake 3.29.2 with jwlawson/actions-setup-cmake@v2 #1496

Merged
merged 2 commits into from
Apr 15, 2024

Conversation

jjerphan
Copy link
Collaborator

Reference Issues/PRs

See: actions/runner-images#9680 (comment)

What does this implement or fix?

Any other comments?

Checklist

Checklist for code changes...
  • Have you updated the relevant docstrings, documentation and copyright notice?
  • Is this contribution tested against all ArcticDB's features?
  • Do all exceptions introduced raise appropriate error messages?
  • Are API changes highlighted in the PR description?
  • Is the PR labelled as enhancement or bug so it appears in autogenerated release notes?

@jjerphan jjerphan marked this pull request as ready for review April 12, 2024 17:00
@jjerphan jjerphan changed the title ci: Try using cmake 3.29.2 from jwlawson/actions-setup-cmake@v2 hot-fix: Use cmake 3.29.2 with jwlawson/actions-setup-cmake@v2 Apr 12, 2024
jjerphan and others added 2 commits April 12, 2024 19:48
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>

Co-authored-by: Muhammad Hamza Sajjad <muhammadhamzasajjad@gmail.com>
@@ -73,6 +73,13 @@ jobs:
with:
submodules: recursive # Just in case a dep has its own third-party deps

# See: https://github.com/actions/runner-images/issues/9680#issuecomment-2051917949
- name: HOTFIX Setup CMake 3.29.2
Copy link
Collaborator

Choose a reason for hiding this comment

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

We don't really need to say HOTFIX do we? Choosing our own CMake seems sensible and not a hack.

Why are we only doing this on Windows? Using the CMake across all platforms might help to simplify the CI setup a bit.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

The next image should come with this newer version of CMake, and I think this pin should rather be temporary as it duplicates installations of CMake.

I pinned for Windows because it was the only OS affected by the update of the image.

Nevertheless, the name of this step definitely can be changed.

Feel free to author changes on top of mine if you want to.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

What do you think?

Suggested change
- name: HOTFIX Setup CMake 3.29.2
- name: Setup CMake 3.29.2

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Merging as is.

@jjerphan jjerphan merged commit 74f9932 into master Apr 15, 2024
114 checks passed
@jjerphan jjerphan deleted the ci/workaround-cmake-3.29.1-regressions branch April 15, 2024 08:10
muhammadhamzasajjad added a commit that referenced this pull request Apr 15, 2024
Signed-off-by: Julien Jerphanion <git@jjerphan.xyz>
Co-authored-by: Muhammad Hamza Sajjad <muhammadhamzasajjad@gmail.com>
jjerphan added a commit that referenced this pull request Apr 15, 2024
jjerphan added a commit that referenced this pull request Apr 16, 2024
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.

None yet

2 participants