Hive
Registry sync can drop a required submodule whose pinned commit is not fetchable at –depth 1
GitHub issue · Open
Why is this needed?
Tuist.Registry.Swift.ReleaseWorker fetches every submodule with --depth 1. When a submodule is pinned to a commit that is not reachable from a ref tip — a routine situation, since a gitlink records an exact commit and the upstream branch moves on — that shallow fetch can fail on hosts that refuse to serve arbitrary SHAs.
Git’s run_update_procedure in builtin/submodule--helper.c shows why there is no recovery inside git:
if (!is_tip_reachable(ud->sm_path, &ud->oid) &&
fetch_in_submodule(ud->sm_path, ud->depth, ud->quiet, NULL) && !ud->quiet)
fprintf_ln(stderr, _("Unable to fetch in submodule path '%s'; "
"trying to directly fetch %s:"), ...);
/* Now we tried the usual fetch, but `oid` may not be reachable from any of the refs. */
if (!is_tip_reachable(ud->sm_path, &ud->oid) &&
fetch_in_submodule(ud->sm_path, ud->depth, ud->quiet, &ud->oid))
return die_message(_("Fetched in submodule path '%s', but it did not "
"contain %s. Direct fetching of that commit failed."), ...);
Both fetch attempts carry ud->depth. Git retries by asking for the commit directly, but never by deepening, so passing --depth 1 fully determines the outcome. Without it, the same submodule resolves.
The problem is what the worker then does with that failure. did not contain is in @skippable_submodule_failure_markers, so the failure classifies as permanent: handle_skipped_submodule/5 deletes the path and the walk continues. The release is published without a submodule that the package may genuinely require. Several neighbouring markers look like the same family — not our ref, unable to find current revision in submodule path, reference is not a tree, needed a single revision — though I only traced did not contain to its source.
That is a statement about our fetch strategy, not about whether the submodule is reachable. Treating it as permanent is the one irreversible direction: the incomplete archive is checksummed and recorded, do_sync_release/5 short-circuits that version from then on, the bucket keeps no object versions to fall back to, and a corrected archive is refused by ensure_checksum_change_allowed/5 because it checksums differently.
This is not a regression from #12232, and it predates the walk. But the exposure grew: before that change a nested failure of this kind deleted the whole parent submodule (strictly worse), and --recursive abandoned the rest of the subtree after the first failure. Now the walk continues past a skip, so more submodules reach a permanent-skip decision than before.
Worth noting the parity angle: SwiftPM’s own GitRepository.updateSubmoduleAndCleanNotOnQueue() runs git submodule update --init --recursive with no depth limit, so a package in this situation resolves fine from source control while the registry archive would be missing sources. The divergence is created entirely by our optimization.
What I could not verify. I did not reproduce this end to end. Local transports ignore --depth (“–depth is ignored in local clones”) and appear to bypass the SHA-in-want restriction, so a file:// fixture fetches the pinned commit successfully. Reproducing needs a real HTTP or git daemon server with uploadpack.allowReachableSHA1InWant and uploadpack.allowAnySHA1InWant off. The markers themselves were added in #10256 alongside the inaccessible-submodule handling, which suggests they were observed rather than anticipated — confirming how often this fires in practice is part of the work.
Steps to address the need
- Separate the shallow-fetch artifacts from the genuinely permanent markers in
Tuist.Registry.Swift.ReleaseWorker.did not containand its siblings say “not fetchable the way we asked”, whereasrepository not foundandwrite access to repository not grantedsay “not fetchable at all”. - On a shallow artifact, retry the submodule update without
--depth 1before classifying it. Only if the full fetch also fails should the failure be treated as permanent and the path skipped. - Log when the fallback is taken, including the submodule path, so the frequency becomes measurable — that data also decides whether
--depth 1is worth keeping as the default at all. - Prefer deepening over an unbounded fetch if it is cheap to express (
--depthescalation, or fetching the pinned SHA into a shallow repository), since a full-history fetch of a large submodule is exactly the cost #12233 is about. A correct archive matters more than a small one, but the two are not fully in tension here. - Tests in
server/test/tuist/registry/swift/release_worker_test.exs: a shallow failure that succeeds on the full retry must publish with the submodule present; a genuine permanent failure must still skip after the retry; and the retry must not fire for markers that are not shallow artifacts. TheSystem.cmdstubbing harness added in #12232 can drive these without a network fixture. - If an end-to-end fixture is wanted, it needs a
git daemon-backed server with theuploadpack.allow*SHA1InWantsettings off — the same harness would also cover the walk’s populating path, which is currently only pinned at the argv level.
Context
- Related to #12232, which did not introduce this but increased the population subject to it.
- Related to #12233, in that a full-depth fallback pulls more data, and the two should be designed together.
Investigate, reproduce, or fix this item in an isolated repository. Each Flight preserves its outcome and agent session.
Start a Flight and preserve its objective, outcome, and session here.
No GitHub comments yet.