-
Notifications
You must be signed in to change notification settings - Fork 463
ci: verify submodule pins are reachable from tracked branch #11063
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
jkiviluoto-nv
wants to merge
13
commits into
shader-slang:master
Choose a base branch
from
jkiviluoto-nv:submodule-pin-ci-check
base: master
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
+366
−0
Open
Changes from 3 commits
Commits
Show all changes
13 commits
Select commit
Hold shift + click to select a range
dfa19a5
ci: verify submodule pins are reachable from tracked branch
jkiviluoto-nv dca3810
ci: scope check-submodules workflow to read-only token permissions
jkiviluoto-nv f0ae06f
ci: prevent set -e exit on missing path/url in check-submodule-commit…
jkiviluoto-nv 22a5917
ci: exempt spirv-tools submodule pin from branch-reachability check
jkiviluoto-nv 2499dd3
Merge remote-tracking branch 'upstream/master' into submodule-pin-ci-…
jkiviluoto-nv 79640b2
ci: clarify why the lua submodule needs an explicit branch override
jkiviluoto-nv 7a5488f
Merge branch 'master' into submodule-pin-ci-check
jkiviluoto-nv 24afbeb
Merge remote-tracking branch 'origin/master' into submodule-pin-ci-check
jkiviluoto-nv 9daf3e3
ci: bound check-submodules job with a 20-minute timeout
jkiviluoto-nv b860e88
ci: accept tag-tracked submodule pins in reachability check
jkiviluoto-nv ce3d015
ci: stop skipping imgui in submodule pin check
jkiviluoto-nv cada121
ci: tailor pin-check failure advice for opt-out submodules
jkiviluoto-nv d8f8865
ci: run submodule pin check only when submodules change
jkiviluoto-nv File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,26 @@ | ||
| name: Check Submodule Pointers | ||
|
|
||
| on: | ||
| pull_request: | ||
| branches: [master] | ||
| types: [opened, synchronize, reopened, ready_for_review] | ||
| merge_group: | ||
| types: [checks_requested] | ||
|
|
||
| # No top-level paths: filter — GitHub treats path-skipped jobs as "not run", | ||
| # which breaks required-check gating. The script's --diff-base flag handles | ||
| # the short-circuit when no submodule pointer changed. | ||
| jobs: | ||
| check-submodules: | ||
| if: github.event_name != 'pull_request' || github.event.pull_request.draft != true | ||
| runs-on: ubuntu-latest | ||
| permissions: | ||
| contents: read | ||
| steps: | ||
| - uses: actions/checkout@v4 | ||
| with: | ||
| fetch-depth: 0 | ||
| - name: Verify submodule pins | ||
| env: | ||
| BASE_SHA: ${{ github.event.pull_request.base.sha || github.event.merge_group.base_sha }} | ||
| run: ./extras/check-submodule-commits.sh --diff-base "$BASE_SHA" | ||
|
coderabbitai[bot] marked this conversation as resolved.
Outdated
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,275 @@ | ||
| #!/usr/bin/env bash | ||
| # Verify that every submodule pin in this superproject points at a commit that | ||
| # is reachable from the upstream submodule's tracked branch. | ||
| # | ||
| # Motivated by issue #9335: a PR landed with external/slang-rhi pointing at a | ||
| # developer branch instead of main. Reviewers see only the pointer diff, so | ||
| # this check enforces the invariant in CI. | ||
| # | ||
| # Opt-out: setting `submodule.<name>.slang-skip-pin-check = true` in | ||
| # .gitmodules disables the branch-reachability check for that submodule. The | ||
| # script still verifies that the pinned SHA is fetchable from the URL (so | ||
| # typos and rewritten history are still caught) — it just doesn't insist on | ||
| # branch membership. Use sparingly: this is intended for vendored/forked | ||
| # submodules whose pinned commit deliberately isn't on the upstream's | ||
| # branches (e.g. external/imgui, which carries a slang-local patch). | ||
| # | ||
| # See issue #9336. | ||
|
|
||
| set -euo pipefail | ||
|
|
||
| usage() { | ||
| cat <<EOF | ||
| Usage: $0 [--diff-base <ref>] [--help] | ||
|
|
||
| Checks that every submodule's pinned commit (as recorded in HEAD) is reachable | ||
| from the upstream branch tracked in .gitmodules (the 'branch =' override if | ||
| set, otherwise the remote's default branch). | ||
|
|
||
| Options: | ||
| --diff-base <ref> Only check submodules whose pinned SHA differs between | ||
| <ref> and HEAD. Useful for limiting CI work to the set | ||
| of submodules a PR actually touched. When unset, every | ||
| submodule is checked (the default for ad-hoc local use). | ||
| --help Show this message. | ||
| EOF | ||
| } | ||
|
|
||
| DIFF_BASE="" | ||
| while [[ $# -gt 0 ]]; do | ||
| case "$1" in | ||
| --diff-base) | ||
| if [[ $# -lt 2 ]]; then | ||
| echo "ERROR: --diff-base requires an argument" >&2 | ||
| exit 2 | ||
| fi | ||
| DIFF_BASE="$2" | ||
| shift 2 | ||
| ;; | ||
| --help | -h) | ||
| usage | ||
| exit 0 | ||
| ;; | ||
| *) | ||
| echo "ERROR: unknown argument: $1" >&2 | ||
| usage >&2 | ||
| exit 2 | ||
| ;; | ||
| esac | ||
| done | ||
|
|
||
| if [[ ! -f .gitmodules ]]; then | ||
| echo "INFO: no .gitmodules at $(pwd); nothing to check." | ||
| exit 0 | ||
| fi | ||
|
|
||
| WORK_DIR="$(mktemp -d)" | ||
| # Drop the temp dir on any exit. The bare repos inside can be hundreds of MB | ||
| # after a full unshallow, so leaving them around would accumulate fast. | ||
| trap 'rm -rf "$WORK_DIR"' EXIT | ||
|
|
||
| resolve_default_branch() { | ||
| # Parse the symref line emitted by `git ls-remote --symref <url> HEAD`: | ||
| # ref: refs/heads/main HEAD | ||
| # We want just "main". Use awk for the field split + sub to keep this | ||
| # portable across BSD/GNU `sed` differences. | ||
| local url="$1" | ||
| git ls-remote --symref "$url" HEAD 2>/dev/null | | ||
| awk '$1 == "ref:" { sub("refs/heads/", "", $2); print $2; exit }' | ||
| } | ||
|
|
||
| ensure_bare_repo() { | ||
| # Each submodule gets its own bare repo keyed by URL hash so re-fetches | ||
| # against the same URL share the object store across iterations of the | ||
| # depth-escalation loop. | ||
| local url="$1" | ||
| local hash | ||
| hash="$(printf '%s' "$url" | git hash-object --stdin)" | ||
| local repo="$WORK_DIR/$hash" | ||
| if [[ ! -d "$repo" ]]; then | ||
| git init --bare --quiet "$repo" | ||
| fi | ||
| printf '%s\n' "$repo" | ||
| } | ||
|
|
||
| is_ancestor() { | ||
| local repo="$1" | ||
| local sha="$2" | ||
| local ref="$3" | ||
| git -C "$repo" merge-base --is-ancestor "$sha" "$ref" 2>/dev/null | ||
| } | ||
|
|
||
| # Fetch the pinned SHA directly (no branch context) to confirm it exists at | ||
| # the URL. Used for opt-out submodules where we don't require branch | ||
| # membership but still want to catch typos and rewritten history. | ||
| verify_sha_exists() { | ||
| local repo="$1" | ||
| local url="$2" | ||
| local sha="$3" | ||
|
|
||
| if git -C "$repo" cat-file -e "$sha" 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
|
|
||
| # Not all servers allow fetching by SHA (uploadpack.allowReachableSHA1InWant | ||
| # / allowAnySHA1InWant). GitHub does for public repos, which covers our | ||
| # current submodule set. | ||
| if git -C "$repo" fetch --quiet --filter=blob:none --depth=1 \ | ||
| "$url" "$sha" 2>/dev/null; then | ||
| return 0 | ||
| fi | ||
|
|
||
| return 1 | ||
| } | ||
|
|
||
| # Try fetching with progressively deeper history until the pinned commit is | ||
| # reachable, or until we've done a full unshallow and confirmed it isn't. | ||
| # Returns 0 on success, 1 on definitive failure. | ||
| verify_reachable() { | ||
| local repo="$1" | ||
| local url="$2" | ||
| local branch="$3" | ||
| local sha="$4" | ||
| local refspec="refs/heads/$branch:refs/remotes/origin/$branch" | ||
|
|
||
| local depth | ||
| for depth in 50 500; do | ||
| if git -C "$repo" fetch --quiet --filter=blob:none --depth="$depth" \ | ||
| "$url" "$refspec" 2>/dev/null; then | ||
| if is_ancestor "$repo" "$sha" "refs/remotes/origin/$branch"; then | ||
| return 0 | ||
| fi | ||
| fi | ||
| done | ||
|
|
||
| # Final attempt: full history. --unshallow only works on an existing shallow | ||
| # repo, so on the first try we just fetch without --depth. | ||
| if git -C "$repo" fetch --quiet --filter=blob:none --unshallow \ | ||
| "$url" "$refspec" 2>/dev/null || | ||
| git -C "$repo" fetch --quiet --filter=blob:none \ | ||
| "$url" "$refspec" 2>/dev/null; then | ||
| if is_ancestor "$repo" "$sha" "refs/remotes/origin/$branch"; then | ||
| return 0 | ||
| fi | ||
| fi | ||
|
|
||
| return 1 | ||
| } | ||
|
|
||
| # Build the list of submodule names from .gitmodules. The output of | ||
| # get-regexp is "submodule.<name>.path <value>", one per line. | ||
| mapfile -t SUBMODULE_NAMES < <( | ||
| git config -f .gitmodules --get-regexp '^submodule\..*\.path$' | | ||
| awk '{print $1}' | | ||
| sed -E 's/^submodule\.(.*)\.path$/\1/' | ||
| ) | ||
|
|
||
| if [[ ${#SUBMODULE_NAMES[@]} -eq 0 ]]; then | ||
| echo "INFO: .gitmodules has no submodule entries; nothing to check." | ||
| exit 0 | ||
| fi | ||
|
|
||
| declare -a FAILURES=() | ||
| declare -i CHECKED=0 | ||
| declare -i SKIPPED=0 | ||
|
|
||
| for name in "${SUBMODULE_NAMES[@]}"; do | ||
| path="$(git config -f .gitmodules --default '' "submodule.${name}.path")" | ||
| url="$(git config -f .gitmodules --default '' "submodule.${name}.url")" | ||
| branch_override="$(git config -f .gitmodules --default '' "submodule.${name}.branch")" | ||
| skip_pin_check="$(git config -f .gitmodules --default '' "submodule.${name}.slang-skip-pin-check")" | ||
|
|
||
| if [[ -z "$path" || -z "$url" ]]; then | ||
| echo "WARNING: submodule '$name' is missing path or url in .gitmodules; skipping." >&2 | ||
| SKIPPED+=1 | ||
| continue | ||
| fi | ||
|
|
||
|
jkwak-work marked this conversation as resolved.
|
||
| # Resolve the pinned SHA from HEAD's tree. ls-tree avoids needing the | ||
| # submodule contents checked out and works even on a fresh clone. | ||
| ls_tree_line="$(git ls-tree HEAD -- "$path" || true)" | ||
| if [[ -z "$ls_tree_line" ]]; then | ||
| echo "WARNING: submodule '$name' (path=$path) has no entry at HEAD; skipping." >&2 | ||
| SKIPPED+=1 | ||
| continue | ||
| fi | ||
| mode="$(printf '%s\n' "$ls_tree_line" | awk '{print $1}')" | ||
| pinned_sha="$(printf '%s\n' "$ls_tree_line" | awk '{print $3}')" | ||
| if [[ "$mode" != "160000" ]]; then | ||
| echo "WARNING: '$path' is not a gitlink (mode=$mode); skipping." >&2 | ||
| SKIPPED+=1 | ||
| continue | ||
| fi | ||
|
|
||
| if [[ -n "$DIFF_BASE" ]]; then | ||
| base_line="$(git ls-tree "$DIFF_BASE" -- "$path" 2>/dev/null || true)" | ||
| base_sha="$(printf '%s\n' "$base_line" | awk '{print $3}')" | ||
| if [[ -n "$base_sha" && "$base_sha" == "$pinned_sha" ]]; then | ||
| SKIPPED+=1 | ||
| continue | ||
| fi | ||
| fi | ||
|
|
||
| repo="$(ensure_bare_repo "$url")" | ||
|
|
||
| if [[ "$skip_pin_check" == "true" ]]; then | ||
| echo "INFO: '$name' (path=$path) skipping branch check (opted out via submodule.${name}.slang-skip-pin-check); verifying SHA $pinned_sha is fetchable." | ||
| if verify_sha_exists "$repo" "$url" "$pinned_sha"; then | ||
| echo " PASS: $pinned_sha is fetchable from $url." | ||
| else | ||
| FAILURES+=("$name|$path|$url|<opted out>|$pinned_sha|pinned commit not fetchable from URL (typo or rewritten history?)") | ||
| fi | ||
| CHECKED+=1 | ||
| continue | ||
| fi | ||
|
|
||
| if [[ -n "$branch_override" ]]; then | ||
| branch="$branch_override" | ||
| branch_source="branch override in .gitmodules" | ||
| else | ||
| branch="$(resolve_default_branch "$url" || true)" | ||
| branch_source="remote default branch" | ||
| if [[ -z "$branch" ]]; then | ||
| FAILURES+=("$name|$path|$url|<unknown>|$pinned_sha|could not resolve remote default branch") | ||
| CHECKED+=1 | ||
| continue | ||
| fi | ||
| fi | ||
|
|
||
| echo "INFO: checking '$name' (path=$path) pinned $pinned_sha against $branch ($branch_source)" | ||
|
|
||
| if verify_reachable "$repo" "$url" "$branch" "$pinned_sha"; then | ||
| echo " PASS: $pinned_sha is reachable from $branch." | ||
| else | ||
| FAILURES+=("$name|$path|$url|$branch|$pinned_sha|pinned commit not reachable from branch") | ||
| fi | ||
| CHECKED+=1 | ||
| done | ||
|
|
||
| echo | ||
| echo "Submodules checked: $CHECKED skipped: $SKIPPED failed: ${#FAILURES[@]}" | ||
|
|
||
| if [[ ${#FAILURES[@]} -gt 0 ]]; then | ||
| echo | ||
| echo "ERROR: one or more submodule pins are not reachable from their tracked branch." | ||
| echo | ||
| for entry in "${FAILURES[@]}"; do | ||
| IFS='|' read -r name path url branch sha reason <<<"$entry" | ||
| echo " Submodule: $name" | ||
| echo " path: $path" | ||
| echo " url: $url" | ||
| echo " branch: $branch" | ||
| echo " pinned: $sha" | ||
| echo " reason: $reason" | ||
| echo " fix: the pinned commit is not reachable from $branch; either land" | ||
| echo " the commit on $branch or re-point the submodule to a commit" | ||
| echo " that is on $branch. If you intended to pin a tag, note that" | ||
| echo " tags are not branches: this check verifies branch reachability," | ||
| echo " so the tagged commit must also exist on $branch." | ||
| echo | ||
| done | ||
| exit 1 | ||
| fi | ||
|
|
||
| echo "All submodule pins are reachable from their tracked branches." | ||
| exit 0 | ||
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Uh oh!
There was an error while loading. Please reload this page.