Skip to content

chore: Pinning submodules to commit SHA #1084

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 1 commit into
base: main
Choose a base branch
from

Conversation

ChaosEternal
Copy link

Fixes #1083

What does this PR do?

Pinning the submodules to their commit SHA

Type of Change

  • Bug fix (non-breaking change)
  • New feature (non-breaking change)
  • Breaking change (fix or feature with breaking changes)
  • Documentation update

Test Plan

  • Unit tests added/updated
  • Integration tests added/updated
  • Passed make installcheck
  • Passed make -C src/test installcheck-cbdb-parallel

Impact

Checklist

Additional Context

CI Skip Instructions


Copy link

@github-actions github-actions bot left a comment

Choose a reason for hiding this comment

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

Hi, @ChaosEternal welcome!🎊 Thanks for taking the effort to make our project better! 🙌 Keep making such awesome contributions!

@edespino edespino self-requested a review May 3, 2025 19:49
Copy link
Contributor

@edespino edespino left a comment

Choose a reason for hiding this comment

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

Thanks for outlining the plan and rationale — pinning submodules to a specific commit SHA is definitely a step forward for ensuring reproducibility.

That said, since several of the submodules appear to reference well-established upstream projects (e.g., googletest, benchmark, tabulate), it's worth considering tag-based pinning instead of raw commit SHAs where possible. Git submodules can reference a tag — essentially, you'd checkout the desired tag in the submodule and commit that state in the superproject. This approach has a few advantages:

  • Improved traceability: Tags often correspond to stable releases (e.g., v1.14.0), which makes it easier to understand what version is being used at a glance.
  • Community alignment: Staying in sync with upstream’s versioning helps when debugging or contributing upstream.
  • Reduced churn: Upstream tags are less likely to be force-pushed or removed compared to commits on active branches.

Here’s how you might do it:

cd submodule-dir
git fetch --tags
git checkout tags/v1.14.0
cd ../
git add submodule-dir
git commit -m "Update submodule to tag v1.14.0"

This doesn’t prevent you from switching to a subtree model later, which has its own merits, especially for long-term archival or if upstream becomes unstable or disappears. But in the meantime, where upstream uses semver-style tagging, tagging improves clarity and slightly reduces maintenance overhead compared to opaque SHAs.

Happy to help explore this option further if there’s interest.

 3137465194014d66a8402941e80d2bccc6346f51 contrib/pax_storage/src/cpp/contrib/cpp-stub (heads/master)
 0da57b85cf23e48d0e515f58c65a25425dbde012 contrib/pax_storage/src/cpp/contrib/googlebench (heads/main)
 52204f78f94d7512df1f0f3bea1d47437a2c3a58 contrib/pax_storage/src/cpp/contrib/googletest (heads/v1.15.x)
 3a58301067bbc03da89ae5a51b3e05b7da719d38 contrib/pax_storage/src/cpp/contrib/tabulate (heads/master)
 61c03f62b370b685b7994830b570a88d05ba15ab dependency/yyjson (heads/master)
 a09ea700d32bab83325aff9ff34d0582e50e3997 gpcontrib/gpcloud/test/googletest (heads/main)
@tuhaihe
Copy link
Member

tuhaihe commented May 26, 2025

Hi, is there any update on this PR? We're nearing the release, but there hasn't been any recent developments. We need to make progress.

If we can't reach a consensus at this time, we can postpone this PR from the 2.0.0 release.

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.

3 participants