diff --git a/README.md b/README.md
index f87d91a8..b0d3a957 100644
--- a/README.md
+++ b/README.md
@@ -676,7 +676,8 @@ fetch still failed — naming which source it used, meaning that credential
cannot read one of the LEG repositories, never the root, which no longer
depends on it — checked via job-level `env: SHAPE_LEGS_APP_SET` /
`SHAPE_LEGS_TOKEN_SET`, not `secrets` in the step `if:` (disallowed there;
-MedxEHR PR #8, MedxGlass PR #1: zero-job push runs).
+MedxEHR PR #8, MedxGlass PR #1: zero-job push runs). A `.gitmodules` that git
+refuses fails the run too, whatever the credential, naming the key or line.
### A descendant form is a claim; a claim needs a referent
diff --git a/docs/handbook.html b/docs/handbook.html
index 76b5ad83..8df78127 100644
--- a/docs/handbook.html
+++ b/docs/handbook.html
@@ -474,7 +474,7 @@
-
+
@@ -1126,7 +1126,7 @@ If it fails part-way
-
+
Setup
Reading private legs in CI
@@ -1179,7 +1179,8 @@ Reading private legs in CI
naming and manifest checks and skips validate-pins.py with a warning explaining
why. It fails outright only if a credential is configured and the fetch
still failed — meaning that credential cannot read one of the LEG repositories, never
- the root.
+ the root. A .gitmodules that git refuses fails the run too, whatever the
+ credential, naming the key or line.
Full text — private legs in CI
@@ -1188,7 +1189,7 @@ Reading private legs in CI
-
+
Reference
The three legs, and naming
diff --git a/templates/assembly-root/.github/workflows/validate.yml b/templates/assembly-root/.github/workflows/validate.yml
index 8ad2ee05..731464c0 100644
--- a/templates/assembly-root/.github/workflows/validate.yml
+++ b/templates/assembly-root/.github/workflows/validate.yml
@@ -67,6 +67,21 @@ jobs:
# in all three: it could print a workflow command of its own, and git
# cannot clone it. And each repository is named once, however many
# sections or paths give it.
+ #
+ # And only what git reads (#786), by the rules the doctor and `siblings.py`
+ # read the same file by (#733): only a `submodule..url` key counts;
+ # a section whose name is empty or holds a `..` component is ignored, as is
+ # one with no `path`; a value that begins with `-` is ignored, that value
+ # alone, so an earlier one stands; nothing is read while `.gitmodules` is
+ # unmerged; and a file git refuses whole (one it cannot parse, a `path`,
+ # `url`, `ignore`, `update` or `branch` with no value, an `update` other
+ # than none, checkout, rebase or merge, a `shallow` that is no boolean, a
+ # `fetchRecurseSubmodules` that is neither a boolean nor on-demand) fails
+ # the step with its cause and the key or line, since git reads none of it
+ # and a repository named from it would be one git never fetches. The two
+ # steps below that print the urls read by the same key rules; the fetch
+ # step refuses a file git refuses, and the failing step is never reached
+ # with one.
- name: leg repository names from .gitmodules
id: leg-names
if: env.SHAPE_LEGS_APP_SET == 'true'
@@ -75,9 +90,49 @@ jobs:
run: |
set -e
legs=""
- if [ -f .gitmodules ]; then
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then
+ # git reads none of a file it refuses, and neither does this (#786). Whatever
+ # of the file is printed is workflow-command data: % and CR are escaped.
+ esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; }
+ if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then
+ err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc)
+ printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at."
+ exit 1
+ fi
+ cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; }
+ us=$(printf '\001')
+ refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do
+ case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac
+ key=${rec%%"$us"*}
+ section=${key%.*}
+ case $section in submodule.?*) ;; *) continue ;; esac
+ case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac
+ field=${key##*.}
+ case $field in
+ path|url|ignore|update|branch)
+ [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;;
+ esac
+ case $field in
+ update)
+ case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;;
+ shallow)
+ git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;;
+ fetchrecursesubmodules)
+ [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;;
+ esac
+ done || true)
+ if [ -n "$refusal" ]; then
+ printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at."
+ exit 1
+ fi
while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
# A url holding a line break could print a workflow command of its own.
case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
@@ -151,7 +206,9 @@ jobs:
# carry: `https://github.com/...` and SSH `git@github.com:...`. With no
# credential configured this falls back to a plain, unauthenticated
# `git submodule update --init --recursive` — the same command that
- # already ran here, unchanged, for a project whose legs are public.
+ # already ran here, unchanged, for a project whose legs are public. A fetch
+ # that fails over a `.gitmodules` git refuses is no such degrade: no
+ # credential cures it, so the step names the cause and fails (#786).
- name: fetch the legs (submodules)
id: submodules
env:
@@ -185,14 +242,60 @@ jobs:
echo "legs_available=true" >> "$GITHUB_OUTPUT"
else
echo "legs_available=false" >> "$GITHUB_OUTPUT"
+ readable=
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then readable=y; fi
+ if [ -n "$readable" ]; then
+ # git reads none of a file it refuses, and neither does this (#786). Whatever
+ # of the file is printed is workflow-command data: % and CR are escaped.
+ esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; }
+ if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then
+ err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc)
+ printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at."
+ exit 1
+ fi
+ cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; }
+ us=$(printf '\001')
+ refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do
+ case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac
+ key=${rec%%"$us"*}
+ section=${key%.*}
+ case $section in submodule.?*) ;; *) continue ;; esac
+ case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac
+ field=${key##*.}
+ case $field in
+ path|url|ignore|update|branch)
+ [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;;
+ esac
+ case $field in
+ update)
+ case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;;
+ shallow)
+ git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;;
+ fetchrecursesubmodules)
+ [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;;
+ esac
+ done || true)
+ if [ -n "$refusal" ]; then
+ printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at."
+ exit 1
+ fi
+ fi
echo "::warning::submodule checkout failed (git exit $status) using credential source '$legs_credential'. If the legs are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the legs, as a repository or organisation secret. Leg URLs:"
- git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
- # A url holding a line break could print a workflow command of its own.
- case $url in *'
+ if [ -n "$readable" ]; then
+ git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
+ # A url holding a line break could print a workflow command of its own.
+ case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
- echo "::warning:: $url"
- done
+ echo "::warning:: $url"
+ done
+ fi
fi
- uses: actions/setup-python@v5
@@ -229,19 +332,30 @@ jobs:
# a credential that IS configured (App or PAT) that still could not
# check out the legs (wrong scope, missing contents:read grant, an
# incomplete App installation, a leg URL problem) is a finding a human
- # must act on, so this fails the job — the only case that does. The
- # root checkout above never depended on either credential, so a
- # failure here can only mean the credential named below cannot read one
- # of the LEG repositories.
+ # must act on, so this fails the job — the only case that does, but for a
+ # `.gitmodules` git refuses, which the fetch step above fails whatever the
+ # credential, since none cures it. The root checkout above never depended
+ # on either credential, so a failure here can only mean the credential
+ # named below cannot read one of the LEG repositories.
- name: fail — a leg credential is configured but the legs still could not be fetched
if: steps.submodules.outputs.legs_available != 'true' && (env.SHAPE_LEGS_TOKEN_SET == 'true' || env.SHAPE_LEGS_APP_SET == 'true')
run: |
echo "::error::A leg credential is configured (source: ${{ steps.submodules.outputs.legs_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.legs_credential }}' credential cannot read one or more of the following LEG repositories (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a leg URL problem):"
- git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
- # A url holding a line break could print a workflow command of its own.
- case $url in *'
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then
+ # No refusal here: the fetch step above exits 1 on a file git refuses, so
+ # this step is never reached with one.
+ git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
+ # A url holding a line break could print a workflow command of its own.
+ case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
- echo "::error:: $url"
- done
+ echo "::error:: $url"
+ done
+ fi
exit 1
diff --git a/templates/assembly-root/README.md b/templates/assembly-root/README.md
index 1ae66d50..c7d3f2e3 100644
--- a/templates/assembly-root/README.md
+++ b/templates/assembly-root/README.md
@@ -110,7 +110,9 @@ fetch still failed — naming which source it used. That presence check reads
job-level `env: SHAPE_LEGS_APP_SET` / `SHAPE_LEGS_TOKEN_SET` booleans rather
than the `secrets` context directly in the step's `if:` — the `secrets`
context is not allowed in a step-level `if:` expression, and using it there
-makes GitHub reject the whole workflow file instead of just that step.
+makes GitHub reject the whole workflow file instead of just that step. A
+`.gitmodules` that git refuses fails the run too, whatever the credential,
+naming the key or line.
## The lockstep invariant
diff --git a/templates/family-root/.github/workflows/validate.yml b/templates/family-root/.github/workflows/validate.yml
index 56c5dbef..5da6fc2c 100644
--- a/templates/family-root/.github/workflows/validate.yml
+++ b/templates/family-root/.github/workflows/validate.yml
@@ -63,6 +63,21 @@ jobs:
# skipped in all three: it could print a workflow command of its own,
# and git cannot clone it. And each member is named once, however many
# sections or paths give it.
+ #
+ # And only what git reads (#786), by the rules the doctor and `siblings.py`
+ # read the same file by (#733): only a `submodule..url` key counts;
+ # a section whose name is empty or holds a `..` component is ignored, as is
+ # one with no `path`; a value that begins with `-` is ignored, that value
+ # alone, so an earlier one stands; nothing is read while `.gitmodules` is
+ # unmerged; and a file git refuses whole (one it cannot parse, a `path`,
+ # `url`, `ignore`, `update` or `branch` with no value, an `update` other
+ # than none, checkout, rebase or merge, a `shallow` that is no boolean, a
+ # `fetchRecurseSubmodules` that is neither a boolean nor on-demand) fails
+ # the step with its cause and the key or line, since git reads none of it
+ # and a repository named from it would be one git never fetches. The two
+ # steps below that print the urls read by the same key rules; the fetch
+ # step refuses a file git refuses, and the failing step is never reached
+ # with one.
- name: member repository names from .gitmodules
id: member-names
if: env.SHAPE_LEGS_APP_SET == 'true'
@@ -71,9 +86,49 @@ jobs:
run: |
set -e
members=""
- if [ -f .gitmodules ]; then
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then
+ # git reads none of a file it refuses, and neither does this (#786). Whatever
+ # of the file is printed is workflow-command data: % and CR are escaped.
+ esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; }
+ if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then
+ err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc)
+ printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at."
+ exit 1
+ fi
+ cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; }
+ us=$(printf '\001')
+ refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do
+ case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac
+ key=${rec%%"$us"*}
+ section=${key%.*}
+ case $section in submodule.?*) ;; *) continue ;; esac
+ case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac
+ field=${key##*.}
+ case $field in
+ path|url|ignore|update|branch)
+ [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;;
+ esac
+ case $field in
+ update)
+ case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;;
+ shallow)
+ git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;;
+ fetchrecursesubmodules)
+ [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;;
+ esac
+ done || true)
+ if [ -n "$refusal" ]; then
+ printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at."
+ exit 1
+ fi
while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
# A url holding a line break could print a workflow command of its own.
case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
@@ -152,7 +207,9 @@ jobs:
# members AND their legs, which is why the App token above is scoped to
# both. Whichever credential resolves is scoped to THIS STEP alone via
# `env:` and used only through a `git -c url.<...>.insteadOf=<...>`
- # rewrite, never persisted and never printed.
+ # rewrite, never persisted and never printed. A fetch that fails over a
+ # `.gitmodules` git refuses is no such degrade: no credential cures it, so
+ # the step names the cause and fails (#786).
- name: fetch the members (submodules)
id: submodules
env:
@@ -186,14 +243,60 @@ jobs:
echo "members_available=true" >> "$GITHUB_OUTPUT"
else
echo "members_available=false" >> "$GITHUB_OUTPUT"
+ readable=
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then readable=y; fi
+ if [ -n "$readable" ]; then
+ # git reads none of a file it refuses, and neither does this (#786). Whatever
+ # of the file is printed is workflow-command data: % and CR are escaped.
+ esc() { sed -e 's/%/%25/g' -e "s/$(printf '\r')/%0D/g"; }
+ if ! err=$(git config --file .gitmodules --list 2>&1 >/dev/null); then
+ err=$(printf '%s\n' "$err" | sed -n '$s/^fatal: //p' | esc)
+ printf '%s\n' "::error::.gitmodules holds a line git cannot parse${err:+ ($err)}, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the line git stops at."
+ exit 1
+ fi
+ cause() { printf '%s (%s)\n' "$1" "$(printf '%s' "$key" | esc)"; }
+ us=$(printf '\001')
+ refusal=$(git config -z --file .gitmodules --list | tr '\000\n' '\n\001' | while IFS= read -r rec; do
+ case $rec in *"$us"*) value=${rec#*"$us"}; bare= ;; *) value=; bare=y ;; esac
+ key=${rec%%"$us"*}
+ section=${key%.*}
+ case $section in submodule.?*) ;; *) continue ;; esac
+ case /${section#submodule.}/ in *[/\\]..[/\\]*) continue ;; esac
+ field=${key##*.}
+ case $field in
+ path|url|ignore|update|branch)
+ [ -z "$bare" ] || { case $field in ignore|update) a=an ;; *) a=a ;; esac; cause "$a $field key with no value"; break; } ;;
+ esac
+ case $field in
+ update)
+ case $value in none|checkout|rebase|merge) ;; *) cause "an update that is not none, checkout, rebase or merge"; break ;; esac ;;
+ shallow)
+ git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a shallow that is not a boolean"; break; } ;;
+ fetchrecursesubmodules)
+ [ "$value" = on-demand ] || git -c "x.y=$value" config --type=bool --get x.y >/dev/null 2>&1 || { cause "a fetchRecurseSubmodules that is neither a boolean nor on-demand"; break; } ;;
+ esac
+ done || true)
+ if [ -n "$refusal" ]; then
+ printf '%s\n' "::error::.gitmodules holds $refusal, so git refuses the whole file: it reads none of it, and no repository is named from it. git submodule status names the key and line git stops at."
+ exit 1
+ fi
+ fi
echo "::warning::submodule checkout failed (git exit $status) using credential source '$members_credential'. If the members are PRIVATE or internal sibling repositories, the default GITHUB_TOKEN cannot read them. Configure a GitHub App (SHAPE_LEGS_APP_ID + SHAPE_LEGS_APP_PRIVATE_KEY, preferred) or set SHAPE_LEGS_TOKEN — a fine-grained PAT — with contents:read on the members and their legs. Member URLs:"
- git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
- # A url holding a line break could print a workflow command of its own.
- case $url in *'
+ if [ -n "$readable" ]; then
+ git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
+ # A url holding a line break could print a workflow command of its own.
+ case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
- echo "::warning:: $url"
- done
+ echo "::warning:: $url"
+ done
+ fi
fi
- uses: actions/setup-python@v5
@@ -224,18 +327,30 @@ jobs:
# Misconfiguration must not be silent. Neither credential being set
# degrading a family with private members is expected and reported
# above; a credential that IS configured and still could not fetch is a
- # finding a human must act on, so this fails the job — the only case
- # that does. The root checkout never depended on it, so a failure here
- # can only mean that credential cannot read a MEMBER or one of its legs.
+ # finding a human must act on, so this fails the job — the only case that
+ # does, but for a `.gitmodules` git refuses, which the fetch step above
+ # fails whatever the credential, since none cures it. The root checkout
+ # never depended on it, so a failure here can only mean that credential
+ # cannot read a MEMBER or one of its legs.
- name: fail — a credential is configured but the members still could not be fetched
if: steps.submodules.outputs.members_available != 'true' && (env.SHAPE_LEGS_TOKEN_SET == 'true' || env.SHAPE_LEGS_APP_SET == 'true')
run: |
echo "::error::A member credential is configured (source: ${{ steps.submodules.outputs.members_credential }}) but 'git submodule update --init --recursive' still failed. The root checkout does not use this token — it reads with the workflow's default token — so this means the '${{ steps.submodules.outputs.members_credential }}' credential cannot read one or more of the following MEMBER repositories, or a leg of one (wrong token/App scope, a missing contents:read grant, an incomplete App installation, or a URL problem):"
- git config --file .gitmodules --name-only --get-regexp '\.url$' | while IFS= read -r key; do
- url=$(git config --file .gitmodules --get -- "$key") || continue
- # A url holding a line break could print a workflow command of its own.
- case $url in *'
+ if [ -f .gitmodules ] && [ -z "$(git ls-files -u -- .gitmodules)" ]; then
+ # No refusal here: the fetch step above exits 1 on a file git refuses, so
+ # this step is never reached with one.
+ git config --file .gitmodules --name-only --get-regexp '\.url$' | awk '!seen[$0]++' | while IFS= read -r key; do
+ case $key in submodule.?*.url) ;; *) continue ;; esac
+ name=${key#submodule.}
+ name=${name%.url}
+ case /$name/ in *[/\\]..[/\\]*) continue ;; esac
+ # A value beginning with - is ignored, that value alone: git keeps an earlier one.
+ url=$(git config --file .gitmodules --get -- "$key" '^([^-]|$)') || continue
+ git config --file .gitmodules --get -- "submodule.$name.path" '^([^-]|$)' >/dev/null || continue
+ # A url holding a line break could print a workflow command of its own.
+ case $url in *'
'*|*"$(printf '\r')"*) continue ;; esac
- echo "::error:: $url"
- done
+ echo "::error:: $url"
+ done
+ fi
exit 1
diff --git a/templates/family-root/README.md b/templates/family-root/README.md
index 9bfa6aa9..7e85df96 100644
--- a/templates/family-root/README.md
+++ b/templates/family-root/README.md
@@ -140,7 +140,8 @@ The same credentials the assembly roots use apply here, and are tried in the
same order: a GitHub App (`SHAPE_LEGS_APP_ID` + `SHAPE_LEGS_APP_PRIVATE_KEY`,
preferred, minted per run) then a fine-grained `SHAPE_LEGS_TOKEN` PAT with
`contents:read`. With neither, the workflow degrades: it still runs the checks
-that do not need the members checked out.
+that do not need the members checked out. A `.gitmodules` that git refuses
+fails the run whatever the credential, naming the key or line.
**On the GitHub Free plan, set these as REPOSITORY secrets.** GitHub delivers
an ORGANISATION secret only to PUBLIC repositories on Free, so on a private
diff --git a/tests/readme-cap.d/786.txt b/tests/readme-cap.d/786.txt
new file mode 100644
index 00000000..4e7f1721
--- /dev/null
+++ b/tests/readme-cap.d/786.txt
@@ -0,0 +1,7 @@
++1
+2026-10-17
+One line for one sentence #790 (#786) leaves false in the README once it lands:
+"fails outright only if a credential is configured" said a failed fetch with no
+credential never fails the run. A `.gitmodules` git refuses now fails the fetch
+step whatever the credential, since none cures it and a green run over it hid
+it, so the paragraph says so in the words the pull request's own body gives.
diff --git a/tests/test_validate_workflow_gitmodules_urls.py b/tests/test_validate_workflow_gitmodules_urls.py
index 5dfb96d1..0b23ffcb 100644
--- a/tests/test_validate_workflow_gitmodules_urls.py
+++ b/tests/test_validate_workflow_gitmodules_urls.py
@@ -23,6 +23,14 @@
printed it could be a workflow command of its own; each repository is named
once however many sections or paths give it; and a url key outside a
`submodule` section, even `[-x] url`, is read without git's usage dump.
+
+And the rules the doctor and `siblings.py` read the same file by (#786, from
+#733): `RULES` holds one row per rule, each a `.gitmodules` and the urls git
+registers for it. Three things check a row. `git submodule init` itself says
+what git registers, a `siblings.submodule_urls` that has #775's rules agrees
+(it skips until they are in main), and each of the three steps in both
+templates names or lists exactly that, or fails with the cause where git
+refuses the whole file.
"""
from __future__ import annotations
@@ -31,11 +39,13 @@
import re
import shutil
import subprocess
+import sys
+from dataclasses import dataclass
from pathlib import Path
import pytest
-from conftest import REPO, WINDOWS_SKIP, git
+from conftest import REPO, WINDOWS_SKIP, git, load_without_bytecode
from test_scaffold_e2e import _step_run_script
#: What each template calls its steps. The names step's output is `legs=` in
@@ -56,16 +66,17 @@
}
EACH_TEMPLATE = pytest.mark.parametrize("template", sorted(TEMPLATES))
-#: Section name -> url, in the order the file lists them. The first three are
-#: names git allows and `--get-regexp`'s `key value` line cannot hold: a
-#: space, a tab, and a leading `-` with `#` and `;`. The fourth is a url, not
-#: a name, with the same bytes in it (a leading `-`, a space, a tab, `#`, `;`
-#: and a trailing space); it names no repository, so the names step skips it.
+#: Section name -> url, in the order the file lists them; each section mounts
+#: at `mounts/`. The first three are names git allows and
+#: `--get-regexp`'s `key value` line cannot hold: a space, a tab, and a
+#: leading `-` with `#` and `;`. The fourth is a url, not a name, with the
+#: same bytes in it (a space, a tab, `#`, `;` and a trailing space); it names
+#: no repository, so the names step skips it.
SECTIONS = {
"members/My IRRS": "https://github.com/acme/IRRS.git",
"tab\there": "git@github.com:acme/Tabbed.git",
"-lead #hash;semi": "https://github.com/acme/Odd.git",
- "plain": "-odd url\twith #hash;semi ",
+ "plain": "odd url\twith #hash;semi ",
}
THE_LEGS = "IRRS,Tabbed,Odd"
#: Urls the steps skip, given by git's `\n` escape and a raw carriage return:
@@ -79,21 +90,31 @@
EACH_STEP = pytest.mark.parametrize("step", ["names", "fetch", "fail"])
-def acme(name: str, scheme: str = "https://github.com/") -> str:
- return f"url = {scheme}acme/{name}.git\n"
+def acme(name: str) -> str:
+ return f"https://github.com/acme/{name}.git"
+
+
+def section(name: str, *keys: str) -> str:
+ """A `[submodule "name"]` with each of `keys` on a line of its own."""
+ return f'[submodule "{name}"]\n' + "".join(f"\t{key}\n" for key in keys)
+
+
+def mounted(name: str, url: str, *keys: str, at: str = "") -> str:
+ """`section`, mounted at `at`, or at a path named for it, giving `url`."""
+ return section(name, f"path = {at or name}", f"url = {url}", *keys)
#: Each case a repository could be named twice, as a `.gitmodules` text and
#: the repositories the names step names for it: once each, first seen first.
DUPLICATES = {
"a url given twice in one section": (
- '[submodule "m"]\n\t' + acme("Stale") + "\t" + acme("A"), "A"),
+ mounted("m", acme("Stale"), f"url = {acme('A')}"), "A"),
"a section reopened further down": (
- '[submodule "m"]\n\t' + acme("Stale") + '[submodule "n"]\n\t'
- + acme("B") + '[submodule "m"]\n\t' + acme("A"), "A,B"),
+ mounted("m", acme("Stale")) + mounted("n", acme("B"))
+ + section("m", f"url = {acme('A')}"), "A,B"),
"two paths mounting one repository": (
- '[submodule "m"]\n\t' + acme("A") + '[submodule "n"]\n\t'
- + acme("A", "git@github.com:"), "A"),
+ mounted("m", acme("A")) + mounted("n", "git@github.com:acme/A.git"),
+ "A"),
}
@@ -105,24 +126,184 @@ def workflow_text(template: str) -> str:
@pytest.fixture
def holder(tmp_path: Path) -> Path:
- """A folder whose `.gitmodules` is `SECTIONS` and then `LINE_BREAKS`,
+ """A repository whose `.gitmodules` is `SECTIONS` and then `LINE_BREAKS`,
written by git so that its quoting is git's."""
work = tmp_path / "holder"
work.mkdir()
+ git("init", "-q", cwd=work)
for name, url in {**SECTIONS, **LINE_BREAKS}.items():
+ git("config", "--file", ".gitmodules", f"submodule.{name}.path",
+ f"mounts/{name}", cwd=work)
git("config", "--file", ".gitmodules", f"submodule.{name}.url", url,
cwd=work)
return work
-def holder_of(tmp_path: Path, gitmodules: str) -> Path:
- """A folder whose `.gitmodules` is `gitmodules`, as written."""
- work = tmp_path / "holder"
+@dataclass(frozen=True)
+class Rule:
+ """One row of `RULES`: a `.gitmodules`, the paths `git submodule init` is
+ asked about, and what git registers for it.
+
+ `urls` are the urls git registers, in the order of the file; none where it
+ reads none of the file. `refused` is the cause the steps name where git
+ refuses the whole file, and `named` what that message names of the file:
+ the key git stops at as the step prints it (`%` escaped), or its line.
+ `unmerged` leaves `.gitmodules` unmerged in the index with the file in the
+ working tree as it is, and a `gitmodules` of None writes no file at all.
+ """
+ gitmodules: str | None
+ paths: tuple
+ urls: tuple
+ refused: str = ""
+ named: str = ""
+ unmerged: bool = False
+
+
+GITLINK = "1234567890123456789012345678901234567890"
+
+
+def holder_of(tmp_path: Path, gitmodules: str, paths=(), unmerged=False,
+ name: str = "holder") -> Path:
+ """A repository whose `.gitmodules` is `gitmodules`, as written, with a
+ gitlink at each of `paths` and, for `unmerged`, `.gitmodules` in
+ conflict in the index."""
+ work = tmp_path / name
work.mkdir()
- (work / ".gitmodules").write_bytes(gitmodules.encode("utf-8"))
+ git("init", "-q", cwd=work)
+ if gitmodules is not None:
+ (work / ".gitmodules").write_bytes(gitmodules.encode("utf-8"))
+ for path in paths:
+ git("update-index", "--add", "--cacheinfo", f"160000,{GITLINK},{path}",
+ cwd=work)
+ if unmerged:
+ stages = ""
+ for stage, body in ((1, b"base\n"), (2, b"ours\n"), (3, b"theirs\n")):
+ blob = subprocess.run(
+ ["git", "hash-object", "-w", "--stdin"], cwd=work, input=body,
+ capture_output=True, check=True).stdout.decode().strip()
+ stages += f"100644 {blob} {stage}\t.gitmodules\n"
+ subprocess.run(["git", "update-index", "--index-info"], cwd=work,
+ input=stages.encode(), check=True)
return work
+OK_URL = acme("Ok")
+OK = mounted("ok", OK_URL)
+
+
+def accepted(*keys: str) -> Rule:
+ """A mounted section for each of `keys`, which it holds, and `ok`: git
+ reads every one of them."""
+ names = [f"s{n}" for n in range(len(keys))]
+ text = "".join(mounted(name, acme(name.upper()), key)
+ for name, key in zip(names, keys))
+ return Rule(text + OK, (*names, "ok"),
+ (*(acme(name.upper()) for name in names), OK_URL))
+
+
+def refusing(cause: str, *keys: str, named: str, name: str = "q",
+ at: str = "") -> Rule:
+ """`ok`, then a section named `name` and mounted at `at`, or at a path
+ named for it, holding `keys`: git refuses the whole file for them, `ok`
+ and all, and the steps say `cause`, naming `named`."""
+ return Rule(OK + mounted(name, acme("Q"), *keys, at=at),
+ ("ok", at or name), (), cause, named)
+
+
+NO_VALUE = " key with no value"
+UPDATE = "an update that is not none, checkout, rebase or merge"
+SHALLOW = "a shallow that is not a boolean"
+FETCH = "a fetchRecurseSubmodules that is neither a boolean nor on-demand"
+
+#: One row per rule, each measured against `git submodule init` by
+#: `test_git_registers_what_each_row_says`. Every row but the refusals keeps
+#: `ok`, so a rule that drops too much shows.
+RULES = {
+ "a url key outside a submodule section": Rule(
+ '[remote "origin"]\n\turl = ' + acme("Other") + "\n[-x]\n\turl = "
+ + acme("Dash") + "\n[submodule]\n\turl = " + acme("Bare") + "\n" + OK,
+ ("ok",), (OK_URL,)),
+ "an empty section name": Rule(
+ mounted("", acme("Empty"), at="e") + OK, ("e", "ok"), (OK_URL,)),
+ "a name holding a .. component": Rule(
+ mounted("../a", acme("DotA"), at="d1")
+ + mounted("a/../b", acme("DotB"), at="d2")
+ + mounted("..\\\\c", acme("DotC"), at="d3")
+ + mounted("..", acme("DotD"), at="d4")
+ + mounted("a/..", acme("DotE"), at="d5")
+ + mounted("a..b", acme("Dots"), at="d6") + OK,
+ ("d1", "d2", "d3", "d4", "d5", "d6", "ok"), (acme("Dots"), OK_URL)),
+ "a url beginning with a dash": Rule(
+ mounted("d", "-oProxyCommand=x") + OK, ("d", "ok"), (OK_URL,)),
+ "a path beginning with a dash": Rule(
+ section("p", "path = -p", f"url = {acme('P')}") + OK, ("-p", "ok"),
+ (OK_URL,)),
+ "a section with no path": Rule(
+ section("np", f"url = {acme('Nowhere')}") + OK, ("np", "ok"),
+ (OK_URL,)),
+ "a url given twice, the later beginning with a dash": Rule(
+ mounted("later", acme("Early"), "url = -x") + OK, ("later", "ok"),
+ (acme("Early"), OK_URL)),
+ "a path given twice, the later beginning with a dash": Rule(
+ section("lp", "path = lp", f"url = {acme('LP')}", "path = -p") + OK,
+ ("lp", "ok"), (acme("LP"), OK_URL)),
+ "no gitmodules at all": Rule(None, (), ()),
+ "a gitmodules left unmerged": Rule(OK, ("ok",), (), unmerged=True),
+ "a bare ignore in a section named with a backslash": Rule(
+ section("..\\\\x", "ignore") + OK, ("ok",), (OK_URL,)),
+ "a bare key where git ignores the section": Rule(
+ section("", "url") + section("../x", "path") + '[remote "r"]\n\turl\n'
+ + OK, ("ok",), (OK_URL,)),
+ "a file git cannot parse": Rule(
+ OK + '[submodule "x"\n', ("ok",), (), "a line git cannot parse",
+ "bad config line 5 in file .gitmodules"),
+ "a path with no value": refusing(
+ "a path" + NO_VALUE, "path", named="submodule.q.path"),
+ "a url with no value": refusing(
+ "a url" + NO_VALUE, "url", named="submodule.q.url"),
+ "an ignore with no value": refusing(
+ "an ignore" + NO_VALUE, "ignore", named="submodule.q.ignore"),
+ "an update with no value": refusing(
+ "an update" + NO_VALUE, "update", named="submodule.q.update"),
+ "a branch with no value": refusing(
+ "a branch" + NO_VALUE, "branch", named="submodule.q.branch"),
+ "an update git dies on": refusing(
+ UPDATE, "update = bogus", named="submodule.q.update"),
+ "an update that is a command": refusing(
+ UPDATE, "update = !echo hi", named="submodule.q.update"),
+ "an update in a section named with a percent sign": refusing(
+ UPDATE, "update = bogus", name="p%q", named="submodule.p%25q.update"),
+ "an update in a section named with a carriage return": refusing(
+ UPDATE, "update = bogus", name="q\rx", at="q",
+ named="submodule.q%0Dx.update"),
+ "each update git accepts": accepted(
+ "update = checkout", "update = merge", "update = rebase",
+ "update = none"),
+ "a shallow git dies on": refusing(
+ SHALLOW, "shallow = maybe", named="submodule.q.shallow"),
+ "each shallow git accepts": accepted(
+ "shallow = true", "shallow = no", "shallow = 1", "shallow = 0x10",
+ "shallow = ", "shallow"),
+ "a fetchRecurseSubmodules git dies on": refusing(
+ FETCH, "fetchRecurseSubmodules = sometimes",
+ named="submodule.q.fetchrecursesubmodules"),
+ "a fetchRecurseSubmodules spelt On-Demand": refusing(
+ FETCH, "fetchRecurseSubmodules = On-Demand",
+ named="submodule.q.fetchrecursesubmodules"),
+ "each fetchRecurseSubmodules git accepts": accepted(
+ "fetchRecurseSubmodules = on-demand", "fetchRecurseSubmodules = true",
+ "fetchRecurseSubmodules = 2", "fetchRecurseSubmodules"),
+}
+EACH_RULE = pytest.mark.parametrize("rule_id", sorted(RULES))
+#: Each step with each row, but for the failing step with a file git refuses:
+#: the fetch step exits 1 on one, so the failing step is never reached with
+#: it, and it carries no refusal of its own.
+EACH_STEP_AND_RULE = pytest.mark.parametrize("step, rule_id", [
+ (step, rule_id) for rule_id in sorted(RULES)
+ for step in ("names", "fetch", "fail")
+ if not (step == "fail" and RULES[rule_id].refused)])
+
+
def run_step(template: str, step: str, holder: Path, tmp_path: Path):
"""`(completed process, the step's $GITHUB_OUTPUT)` for one step's `run:`
body in `holder`, under `bash -e` as the runner starts it.
@@ -225,15 +406,135 @@ def test_the_names_step_names_a_repository_once(template, case, tmp_path):
assert output.splitlines() == [f"{TEMPLATES[template]['output']}={names}"]
+def registered(work: Path, paths) -> list:
+ """The urls `git submodule init` registers, asked of each of `paths` in
+ turn, since it stops at the first it can name no url for. It says so on
+ stderr."""
+ urls = []
+ for path in paths:
+ proc = subprocess.run(
+ ["git", "submodule", "init", "--", path], cwd=work,
+ capture_output=True, text=True, encoding="utf-8", check=False)
+ urls += re.findall(r"^Submodule '.*' \((.*)\) registered for path ",
+ proc.stdout + proc.stderr, re.MULTILINE)
+ return urls
+
+
+@EACH_RULE
+def test_git_registers_what_each_row_says(rule_id, tmp_path):
+ rule = RULES[rule_id]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths, rule.unmerged)
+ assert registered(work, rule.paths) == list(rule.urls)
+
+
+@pytest.fixture
+def siblings(monkeypatch):
+ """The TEMPLATE's `siblings.py`, once it has #775's rules; skipped until
+ they are in main. Loaded as `test_family_siblings_placement.py` loads it."""
+ scripts = REPO / "templates" / "family-root" / "scripts"
+ monkeypatch.setattr(sys, "path", list(sys.path))
+ monkeypatch.setitem(sys.modules, "bootstrap", load_without_bytecode(
+ scripts / "bootstrap.py", "ci_rules_family_bootstrap"))
+ module = load_without_bytecode(scripts / "siblings.py",
+ "ci_rules_family_siblings")
+ if not hasattr(module, "DOTDOT_COMPONENT"):
+ pytest.skip("siblings.py has no #733 rules until #775 is in main")
+ return module
+
+
+@pytest.mark.parametrize("rule_id", sorted(
+ rule_id for rule_id, rule in RULES.items()
+ if not rule.unmerged))
+def test_siblings_reads_what_each_row_says(rule_id, siblings, tmp_path):
+ """`siblings.submodule_urls` reads the file in the working tree, so a row
+ that leaves it unmerged in the index is not asked. A row with a carriage
+ return in a section name is: `siblings.py` has read `git config -z
+ --list` since #805 (#690), so a CR stays inside its key, and the refused
+ `update` is read as `update`."""
+ rule = RULES[rule_id]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths)
+ assert sorted(siblings.submodule_urls(work).values()) == sorted(rule.urls)
+
+
+def read_by(template: str, step: str, result, output: str) -> list:
+ """What one step named (its repositories) or listed (its urls)."""
+ if step == "names":
+ key = TEMPLATES[template]["output"]
+ [named] = [line[len(key) + 1:] for line in output.splitlines()
+ if line.startswith(f"{key}=")] or [""]
+ return named.split(",") if named else []
+ prefix = {"fetch": "::warning:: ", "fail": "::error:: "}[step]
+ return [line[len(prefix):] for line in result.stdout.split("\n")
+ if line.startswith(prefix)]
+
+
+def expected_of(step: str, rule: Rule) -> list:
+ """What git's urls come to in one step: repository names in the names step,
+ the urls themselves in the two that list them."""
+ if step != "names":
+ return list(rule.urls)
+ return [url.rsplit("/", 1)[-1].removesuffix(".git") for url in rule.urls]
+
+
@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is
-@EACH_STEP
+@EACH_STEP_AND_RULE
+@EACH_TEMPLATE
+def test_each_step_reads_what_git_reads(template, step, rule_id, tmp_path):
+ rule = RULES[rule_id]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths, rule.unmerged)
+ result, output = run_step(template, step, work, tmp_path)
+ assert read_by(template, step, result, output) == expected_of(step, rule)
+
+
+@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is
+@EACH_STEP_AND_RULE
@EACH_TEMPLATE
-def test_a_url_key_outside_a_submodule_section_is_read_without_a_usage_dump(
- template, step, tmp_path):
- """`--get-regexp '\\.url$'` lists any section's url, and `git config
- --get -x.url` reads `-x` as a switch: a usage dump on stderr, exit 129,
- and the url swallowed by the `|| continue` after it."""
- work = holder_of(tmp_path, '[remote "origin"]\n\t' + acme("Remote")
- + "[-x]\n\t" + acme("Dash"))
+def test_each_step_exits_as_expected_and_prints_nothing_on_stderr(
+ template, step, rule_id, tmp_path):
+ """Only a file git refuses whole fails a step that would not fail anyway:
+ the failing step fails always, and nothing reaches stderr."""
+ rule = RULES[rule_id]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths, rule.unmerged)
result, _ = run_step(template, step, work, tmp_path)
+ failed = bool(rule.refused) or step == "fail"
+ assert result.returncode == int(failed)
assert result.stderr == ""
+
+
+@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is
+@pytest.mark.parametrize("rule_id", sorted(
+ rule_id for rule_id, rule in RULES.items() if rule.refused))
+@pytest.mark.parametrize("step", ["names", "fetch"])
+@EACH_TEMPLATE
+def test_a_file_git_refuses_whole_is_named_with_its_cause_and_its_key(
+ template, step, rule_id, tmp_path):
+ rule = RULES[rule_id]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths)
+ result, _ = run_step(template, step, work, tmp_path)
+ causes = [line for line in result.stdout.split("\n")
+ if line.startswith("::error::") and rule.refused in line]
+ assert len(causes) == 1
+ assert rule.named in causes[0]
+
+
+@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is
+@EACH_TEMPLATE
+def test_the_fetch_step_gives_no_credential_advice_for_a_file_git_refuses(
+ template, tmp_path):
+ rule = RULES["a url with no value"]
+ work = holder_of(tmp_path, rule.gitmodules, rule.paths)
+ result, _ = run_step(template, "fetch", work, tmp_path)
+ assert "submodule checkout failed" not in result.stdout
+
+
+@WINDOWS_SKIP # it runs the step's `run:` body as the REAL bash it is
+@EACH_TEMPLATE
+def test_the_fetch_step_gives_its_credential_advice_before_it_lists_the_urls(
+ template, holder, tmp_path):
+ result, _ = run_step(template, "fetch", holder, tmp_path)
+ lines = result.stdout.split("\n")
+ advice = next(n for n, line in enumerate(lines)
+ if line.startswith("::warning::submodule checkout failed"))
+ first = next(n for n, line in enumerate(lines)
+ if line.startswith("::warning:: "))
+ assert advice < first