Resolve relative submodule URLs for mirrors - #4287
Open
davido wants to merge 1 commit into
Open
Conversation
Submodule URLs in .gitmodules may be relative to the superproject remote. The mirror prefetch path previously passed those raw values directly to git clone --mirror from the mirror cache directory. Values such as ../java-prettify were therefore interpreted as local paths under the mirror cache, not as repositories relative to the superproject remote. Resolve relative submodule URLs before updating a submodule mirror, while leaving the actual git submodule update command to Git. If relative URL resolution or the resolved mirror update fails, warn and fall back to a plain submodule update so Git can apply its authoritative relative URL semantics. Absolute submodule mirror failures retain the existing hard-fail behavior. Add resolver unit coverage for absolute, relative, HTTP(S), SSH, scp-like, local path, malformed URL, and relative-detection cases. Add integration coverage proving relative submodules are mirror-cloned via the resolved repository URL, raw relative URLs are never passed to git clone --mirror, and failed resolved mirrors fall back to non-referenced submodule update. Closes buildkite#4282. Refs bazelbuild/continuous-integration#2815.
Contributor
|
Thanks for opening this PR. I’ve created a ticket to track it internally. |
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Description
When
git-mirrors-pathis enabled, the agent pre-populates a Git mirror for eachsubmodule enumerated from
.gitmodulesbefore runninggit submodule update.Submodule URLs in
.gitmodulesmay be relative to the superproject remote,e.g.
url = ../java-prettify. The mirror prefetch path passed those raw valuesdirectly to
git clone --mirrorwhilecd'd into the mirror cache directory, so../java-prettifywas interpreted as a local filesystem path undergit-mirrors-pathinstead of a repository relative to the superproject remote —and checkout failed before any job command ran.
This PR resolves relative submodule URLs against
e.Repositoryonly for mirrorselection/population. The actual
git submodule updatecommand is leftunchanged (aside from whether it receives
--reference <mirrorDir>), so Gitremains the authority on submodule checkout semantics.
Resolution is fail-open for relative submodules: if URL resolution or the
resolved mirror update fails, the agent warns and falls back to a plain
git submodule update(no--reference), letting Git apply its ownauthoritative relative-URL handling. Absolute submodule mirror failures keep the
existing hard-fail behavior.
Alternatives considered: a minimal fix that simply skipped mirrors for any
relative submodule URL. It was safe but gave up the mirror optimization even when
the agent can resolve the URL correctly. The chosen approach keeps that safety net
(via the fail-open fallback) while still getting mirror performance in the common
case where the resolver agrees with Git.
Context
Example resolution:
https://gerrit.googlesource.com/gerrit.git+../java-prettify→
https://gerrit.googlesource.com/java-prettify.Changes
internal/job/checkout.go— inupdateGitSubmodules, resolve relativesubmodule URLs before the mirror update; add fail-open fallback for relative
submodules (resolver error or resolved-mirror failure → plain submodule update);
preserve hard-fail for absolute submodules.
internal/job/git.go— addresolveGitSubmoduleURLandisRelativeSubmoduleURL. Resolution preserves the original URL form:https/sshviaurl.URL, scp-like (git@host:path, including absolutegit@host:/abs/path) via string reconstruction, and local paths viafilepath.Join.scpLikeURLPatternnow captures the leading slash after thecolon so absolute scp remote paths survive resolution;
parseGittableURLisupdated to preserve that slash and is behavior-identical for all other inputs.
git.repoas theresolved URL and adds
git.raw_repo(the raw.gitmodulesvalue) only when itdiffers; a comment logs the resolution. Repository URLs are credential-redacted
in logs and span attributes.
absolute-path/local, cleaned paths, bare name unchanged, malformed-URL parse
error, relative-URL detection) plus a
parseGittableURLabsolute-scp case; twointegration tests (
..._WithRelativeSubmoduleURL_WithGitMirrorsand..._WithRelativeSubmoduleURLFallsBackWhenMirrorFails_WithGitMirrors) sharing asetupRelativeSubmoduleFixturehelper.Testing
go test ./...). Buildkite employees may check this if the pipeline has run automatically.go tool gofumpt -extra -w .)Focused runs:
Affiliation (optional, external contributors)
External contributor, filing on behalf of the Gerrit project's Bazel downstream Buildkite CI pipeline.