Skip to content

Add Submodule Version Check to cmake Configuration #750

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

Open
wants to merge 14 commits into
base: main
Choose a base branch
from

Conversation

sbSteveK
Copy link
Contributor

@sbSteveK sbSteveK commented Jul 7, 2025

Add a check during CMake's configuration phase that determines whether the local submodules are at least the version/commit expected by aws-crt-cpp or later.

This check is done by determining the submodule commits aws-crt-cpp expects and then comparing that commit to the one that is currently actually checked out by submodules in the crt/ folder.

It will fail cmake configuration if it detects a commit that is currently in a submodule folder is older than the one expected. This check can be turned off with -DENFORCE_SUBMODULE_VERSIONS=OFF

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

@sbSteveK sbSteveK marked this pull request as ready for review July 8, 2025 18:15
@sbSteveK sbSteveK changed the title Add Submodule Check Add Submodule Version Check to cmake Configuration Jul 8, 2025
Copy link
Contributor

@bretambrose bretambrose left a comment

Choose a reason for hiding this comment

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

I can't help but feel like there ought to be programmatic ways of getting the expected submodule versions that don't require us to have a tracking file committed.

Copy link
Contributor

@xiazhvera xiazhvera left a comment

Choose a reason for hiding this comment

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

It seems we'd need to update the workflow to always use the script for updating submodules with the changes. I guess the goal is applied the "latest_submodules.py" across all CRT/SDKs? If that's the case we’ll need clear guidance on the new updating submodules workflow.

Copy link
Contributor

@sfod sfod left a comment

Choose a reason for hiding this comment

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

fix&ship

" • or rerun CMake with commit checks disabled: -DENFORCE_SUBMODULE_VERSIONS=OFF")
endif()

function(check_submodule_commit name rel_path cmake_sym)
Copy link
Contributor

Choose a reason for hiding this comment

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

nit: cmake_sym is not used, can be removed

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.

4 participants