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

Fix the limitation of recent-cmake in the setup-common GitHub action #2363

Closed
wantehchang opened this issue Aug 1, 2024 · 2 comments · Fixed by #2413
Closed

Fix the limitation of recent-cmake in the setup-common GitHub action #2363

wantehchang opened this issue Aug 1, 2024 · 2 comments · Fixed by #2413
Assignees

Comments

@wantehchang
Copy link
Collaborator

The setup-common GitHub action installs the minimum required version of cmake (currently 3.13) by default. If the recent-cmake option is set to 'true', it installs a recent version of cmake (currently 3.18). If a new use case needs a newer cmake version, we need to bump the recent-cmake version, e.g., in #2362. This is inconvenient.

There are two possible improvements:

  1. Replace the recent-cmake option with a cmake-version option, which defaults to the minimum required version of cmake for libavif.
  2. Replace the recent-cmake option with a latest-cmake option, which installs the latest version of cmake.
@wantehchang
Copy link
Collaborator Author

Between the two, I think a cmake-version option is better because it is more flexible. But perhaps latest-cmake would suffice. In any case, it would be good to document what cmake version we need and why in each case.

@vrabaud
Copy link
Collaborator

vrabaud commented Aug 27, 2024

For 1, you would still need to figure out a version that works with all the features you need for your test (e.g. C23, fuzztest ...) and this is not trivial. I believe having it done once and for all is easier.

For 2, we could end up with different CMake versions depending on the platform, and we might face CMake bugs in the latest releases.

Actually, we use recent-cmake: true everywhere where it matters except in ci-android-emulator-tests.yml ...

According to https://github.com/actions/runner-images?tab=readme-ov-file, CMake 3.30 is used on all images: we should probably use the system one by default, and have a test for the old CMake 3.13 that we support (though it should now be 3.16 if we follow https://github.com/google/oss-policies-info/blob/main/foundational-cxx-support-matrix.md). Let me make a PR for that.

vrabaud added a commit to vrabaud/libavif that referenced this issue Aug 27, 2024
All test actually use a recent CMake.
A new test in ci-unix-shared-installed.yml is added to test for the
oldest supported CMake (3.13).

This fixes AOMediaCodec#2363
vrabaud added a commit to vrabaud/libavif that referenced this issue Aug 27, 2024
All tests actually use a recent CMake.
A new test in ci-unix-shared-installed.yml is added to test for the
oldest supported CMake (3.13).
This fixes AOMediaCodec#2363
vrabaud added a commit that referenced this issue Aug 28, 2024
All tests actually use a recent CMake.
A new test in ci-unix-shared-installed.yml is added to test for the
oldest supported CMake (3.13).
This fixes #2363
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 a pull request may close this issue.

2 participants