From b4fcb48b10a598feadacf15f73e13585b2747011 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 25 Aug 2026 15:17:45 +0100 Subject: [PATCH 1/3] Expose deterministic submodule metadata --- README.md | 12 ++ go.mod | 10 +- go.sum | 6 + submodule.go | 293 ++++++++++++++++++++++++++++++++++++++++++++++ submodule_test.go | 198 +++++++++++++++++++++++++++++++ 5 files changed, 518 insertions(+), 1 deletion(-) create mode 100644 submodule.go create mode 100644 submodule_test.go diff --git a/README.md b/README.md index 03d4bdb..25c5d82 100644 --- a/README.md +++ b/README.md @@ -81,6 +81,18 @@ if err != nil { fmt.Println(commit, cache.DiskBytes("https://github.com/git-pkgs/clone")) ``` +Call `Submodules` on the prepared checkout to read pinned identities without changing `Prepare`'s return values. Declared submodules that could not be fetched are included with `Initialized` set to false, `Status` set to `SubmoduleStatusUnavailable`, and a credential-free error string. + +```go +submodules, err := clone.Submodules(ctx, "/tmp/job/src") +if err != nil { + log.Fatal(err) +} +for _, submodule := range submodules { + fmt.Println(submodule.Path, submodule.PURL, submodule.Status) +} +``` + When a historical commit is missing from the shallow cache, `EnsureCommit` unshallows the checkout: ```go diff --git a/go.mod b/go.mod index 80ee1eb..fb8efcd 100644 --- a/go.mod +++ b/go.mod @@ -2,4 +2,12 @@ module github.com/git-pkgs/clone go 1.25.6 -require github.com/git-pkgs/magic v0.2.0 +require ( + github.com/git-pkgs/magic v0.2.0 + github.com/git-pkgs/purl v0.1.17 +) + +require ( + github.com/git-pkgs/vers v0.3.1 // indirect + github.com/package-url/packageurl-go v0.1.6 // indirect +) diff --git a/go.sum b/go.sum index 41f45ab..0836b61 100644 --- a/go.sum +++ b/go.sum @@ -1,2 +1,8 @@ github.com/git-pkgs/magic v0.2.0 h1:c7HqVxnP8c88EaVMH0/KraDFVTcmiXckRiSvNZEnvMQ= github.com/git-pkgs/magic v0.2.0/go.mod h1:3ndidt+yvFaI1M0aEkkzkOlFnLPkeVQASIUojazcxCI= +github.com/git-pkgs/purl v0.1.17 h1:oRSd8tqllTLl74Wa4WnuqU500hXd9OdUnImOEswQUVE= +github.com/git-pkgs/purl v0.1.17/go.mod h1:7u7ora8tQdrkS7Auclr5v8dCJdjN4ej6AbrvYZi2b7k= +github.com/git-pkgs/vers v0.3.1 h1:jy/ht2wIRJI5zQrccm6GTeYr+hGFwe2z8LV1HOr4Wco= +github.com/git-pkgs/vers v0.3.1/go.mod h1:biTbSQK1qdbrsxDEKnqe3Jzclxz8vW6uDcwKjfUGcOo= +github.com/package-url/packageurl-go v0.1.6 h1:YO3p6u1XmCUliivUg/qWphaY8vI6hxSnnPv7Bfg3m5M= +github.com/package-url/packageurl-go v0.1.6/go.mod h1:nKAWB8E6uk1MHqiS/lQb9pYBGH2+mdJ2PJc2s50dQY0= diff --git a/submodule.go b/submodule.go new file mode 100644 index 0000000..da3b0dc --- /dev/null +++ b/submodule.go @@ -0,0 +1,293 @@ +package clone + +import ( + "context" + "fmt" + "net/url" + "path" + "path/filepath" + "sort" + "strings" + + "github.com/git-pkgs/purl" +) + +// SubmoduleStatus describes whether a declared submodule is available at its +// pinned commit. +type SubmoduleStatus string + +const ( + SubmoduleStatusInitialized SubmoduleStatus = "initialized" + SubmoduleStatusUnavailable SubmoduleStatus = "unavailable" +) + +// Submodule describes a gitlink declared by a checkout. Path is relative to +// the top-level checkout, URL is resolved, Commit is the exact gitlink object, +// and PURL is pinned to Commit. Initialized is true only when the submodule +// checkout's HEAD matches Commit. URL, PURL, and Error omit URL userinfo. +type Submodule struct { + Path string + URL string + Commit string + PURL string + Initialized bool + Status SubmoduleStatus + Error string +} + +type submoduleDefinition struct { + configKey string + path string + url string +} + +const gitmodulesBlob = "HEAD:.gitmodules" + +// Submodules returns deterministic metadata for the submodules declared by +// the checkout in dir. Unavailable submodules are returned with Status set to +// SubmoduleStatusUnavailable rather than failing the query. +func Submodules(ctx context.Context, dir string) ([]Submodule, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + + modules, err := submodulesAt(ctx, dir, "") + if err != nil { + return nil, err + } + sort.Slice(modules, func(i, j int) bool { + return modules[i].Path < modules[j].Path + }) + return modules, nil +} + +func submodulesAt(ctx context.Context, dir, parentPath string) ([]Submodule, error) { + if err := ctx.Err(); err != nil { + return nil, err + } + definitions, err := readSubmoduleDefinitions(ctx, dir) + if err != nil { + return nil, err + } + + var modules []Submodule + for _, definition := range definitions { + if err := ctx.Err(); err != nil { + return nil, err + } + + module := Submodule{ + Path: path.Join(parentPath, definition.path), + Status: SubmoduleStatusUnavailable, + } + commit, err := gitlinkCommit(ctx, dir, definition.path) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + module.Error = "gitlink commit is unavailable" + modules = append(modules, module) + continue + } + module.Commit = commit + + resolvedURL, err := submoduleURL(ctx, dir, definition) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + module.Error = "resolved repository URL is unavailable" + } else { + module.URL, module.PURL, err = submoduleIdentity(resolvedURL, commit) + if err != nil { + module.Error = "package URL is unavailable" + } + } + + childDir := pathFromGit(dir, definition.path) + head, headErr := submoduleHead(ctx, childDir) + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + switch { + case headErr == nil && head == commit: + module.Initialized = true + module.Status = SubmoduleStatusInitialized + case headErr == nil: + addSubmoduleError(&module, fmt.Sprintf("checked out commit %s does not match gitlink %s", head, commit)) + default: + addSubmoduleError(&module, "submodule checkout is unavailable") + } + modules = append(modules, module) + + if !module.Initialized { + continue + } + nested, err := submodulesAt(ctx, childDir, module.Path) + if err != nil { + return nil, err + } + modules = append(modules, nested...) + } + return modules, nil +} + +func readSubmoduleDefinitions(ctx context.Context, dir string) ([]submoduleDefinition, error) { + if _, err := Run(ctx, dir, nil, "rev-parse", "--verify", "HEAD^{commit}"); err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, fmt.Errorf("read checkout HEAD: %w", err) + } + if _, err := Run(ctx, dir, nil, "cat-file", "-e", gitmodulesBlob); err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + return nil, nil + } + out, err := Run(ctx, dir, nil, + "config", "-z", "--blob", gitmodulesBlob, "--get-regexp", `^submodule\..*\.path$`, + ) + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + if out == "" { + return nil, nil + } + return nil, fmt.Errorf("read .gitmodules: %w", err) + } + + var definitions []submoduleDefinition + for entry := range strings.SplitSeq(strings.TrimSuffix(out, "\x00"), "\x00") { + key, submodulePath, ok := strings.Cut(entry, "\n") + if !ok || !strings.HasSuffix(key, ".path") { + return nil, fmt.Errorf("read .gitmodules: invalid path entry") + } + submodulePath, ok = SanitizePath(submodulePath) + if !ok { + return nil, fmt.Errorf("read .gitmodules: invalid submodule path") + } + configKey := strings.TrimSuffix(key, ".path") + submoduleURL, err := gitConfigValue(ctx, dir, gitmodulesBlob, configKey+".url") + if err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return nil, ctxErr + } + submoduleURL = "" + } + definitions = append(definitions, submoduleDefinition{ + configKey: configKey, + path: submodulePath, + url: submoduleURL, + }) + } + sort.Slice(definitions, func(i, j int) bool { + return definitions[i].path < definitions[j].path + }) + return definitions, nil +} + +func gitConfigValue(ctx context.Context, dir, blob, key string) (string, error) { + args := []string{"config", "-z"} + if blob != "" { + args = append(args, "--blob", blob) + } + args = append(args, "--get", key) + out, err := Run(ctx, dir, nil, args...) + if err != nil { + return "", err + } + return strings.TrimSuffix(out, "\x00"), nil +} + +func submoduleURL(ctx context.Context, dir string, definition submoduleDefinition) (string, error) { + resolved, err := gitConfigValue(ctx, dir, "", definition.configKey+".url") + if err == nil && resolved != "" { + return resolved, nil + } + if ctxErr := ctx.Err(); ctxErr != nil { + return "", ctxErr + } + parsed, parseErr := url.Parse(definition.url) + if parseErr == nil && parsed.IsAbs() { + return definition.url, nil + } + if err != nil { + return "", err + } + if parseErr != nil { + return "", parseErr + } + return "", fmt.Errorf("relative repository URL is unresolved") +} + +func gitlinkCommit(ctx context.Context, dir, submodulePath string) (string, error) { + out, err := Run(ctx, dir, nil, "ls-tree", "-z", "HEAD", "--", submodulePath) + if err != nil { + return "", err + } + entry := strings.TrimSuffix(out, "\x00") + metadata, treePath, ok := strings.Cut(entry, "\t") + if !ok || treePath != submodulePath { + return "", fmt.Errorf("gitlink not found") + } + fields := strings.Fields(metadata) + if len(fields) != 3 || fields[0] != "160000" || fields[1] != "commit" || !ValidCommit(fields[2]) { + return "", fmt.Errorf("invalid gitlink") + } + return fields[2], nil +} + +func submoduleHead(ctx context.Context, dir string) (string, error) { + out, err := Run(ctx, dir, nil, "rev-parse", "HEAD") + if err != nil { + return "", err + } + head := strings.TrimSpace(out) + if !ValidCommit(head) { + return "", fmt.Errorf("invalid HEAD") + } + return head, nil +} + +func submoduleIdentity(repositoryURL, commit string) (string, string, error) { + redacted := RedactURL(repositoryURL) + parsed, err := url.Parse(redacted) + if err != nil || parsed.Scheme == "" || parsed.Host == "" { + return "", "", fmt.Errorf("invalid repository URL") + } + parsed.User = nil + parsed.ForceQuery = false + parsed.RawQuery = "" + parsed.Fragment = "" + credentialFreeURL := parsed.String() + repositoryPath := strings.TrimSuffix(strings.Trim(parsed.Path, "/"), ".git") + if strings.EqualFold(parsed.Hostname(), "github.com") { + owner, name, ok := strings.Cut(repositoryPath, "/") + if !ok || owner == "" || name == "" || strings.Contains(name, "/") { + return credentialFreeURL, "", fmt.Errorf("invalid GitHub repository URL") + } + return credentialFreeURL, purl.New("github", owner, name, commit, nil).String(), nil + } + + name := path.Base(repositoryPath) + if name == "." || name == "" { + return credentialFreeURL, "", fmt.Errorf("repository name is unavailable") + } + vcsURL := "git+" + credentialFreeURL + "@" + commit + identity := purl.New("generic", "", name, "", map[string]string{"vcs_url": vcsURL}) + return credentialFreeURL, identity.String(), nil +} + +func pathFromGit(dir, submodulePath string) string { + return filepath.Join(dir, filepath.FromSlash(submodulePath)) +} + +func addSubmoduleError(module *Submodule, message string) { + if module.Error == "" { + module.Error = message + return + } + module.Error += "; " + message +} diff --git a/submodule_test.go b/submodule_test.go new file mode 100644 index 0000000..0495300 --- /dev/null +++ b/submodule_test.go @@ -0,0 +1,198 @@ +package clone + +import ( + "context" + "fmt" + "os" + "path/filepath" + "reflect" + "strings" + "testing" +) + +type testRepository struct { + dir string + commit string +} + +func newTestRepository(t *testing.T, filename string) testRepository { + t.Helper() + + dir := t.TempDir() + runGitTest(t, dir, "init", "--quiet", "-b", "main") + if err := os.WriteFile(filepath.Join(dir, filename), []byte(filename+"\n"), 0o644); err != nil { + t.Fatal(err) + } + runGitTest(t, dir, "add", filename) + runGitTest(t, dir, "commit", "--quiet", "-m", "initial") + return testRepository{dir: dir, commit: runGitTest(t, dir, "rev-parse", "HEAD")} +} + +func configureTestURLs(t *testing.T, repositories map[string]string) { + t.Helper() + + i := 0 + for repositoryURL, dir := range repositories { + t.Setenv(fmt.Sprintf("GIT_CONFIG_KEY_%d", i), "url.file://"+dir+".insteadOf") + t.Setenv(fmt.Sprintf("GIT_CONFIG_VALUE_%d", i), repositoryURL) + i++ + } + t.Setenv(fmt.Sprintf("GIT_CONFIG_KEY_%d", i), "protocol.file.allow") + t.Setenv(fmt.Sprintf("GIT_CONFIG_VALUE_%d", i), "always") + t.Setenv("GIT_CONFIG_COUNT", fmt.Sprint(i+1)) + t.Setenv("GIT_ALLOW_PROTOCOL", "https:file") +} + +func TestSubmodulesReportsInitializedNestedIdentities(t *testing.T) { + requireGit(t) + + leaf := newTestRepository(t, "leaf.txt") + outer := newTestRepository(t, "outer.txt") + generic := newTestRepository(t, "generic.txt") + parent := newTestRepository(t, "parent.txt") + + const ( + parentURL = "https://clone.test/parent.git" + genericURL = "https://clone.test/generic.git" + outerURL = "https://github.com/Example/Outer.git" + leafURL = "https://github.com/Example/Leaf.git" + ) + configureTestURLs(t, map[string]string{ + parentURL: parent.dir, + genericURL: generic.dir, + outerURL: outer.dir, + leafURL: leaf.dir, + }) + + runGitTest(t, outer.dir, "submodule", "add", "--quiet", "--name", "leaf", leafURL, "deps/leaf") + runGitTest(t, outer.dir, "commit", "--quiet", "-am", "add nested submodule") + outer.commit = runGitTest(t, outer.dir, "rev-parse", "HEAD") + + runGitTest(t, parent.dir, "submodule", "add", "--quiet", "--name", "generic", genericURL, "third_party/generic") + runGitTest(t, parent.dir, "config", "--file", ".gitmodules", "submodule.generic.url", "../generic.git") + runGitTest(t, parent.dir, "submodule", "add", "--quiet", "--name", "outer", outerURL, "vendor/outer") + runGitTest(t, parent.dir, "commit", "--quiet", "-am", "add submodules") + parent.commit = runGitTest(t, parent.dir, "rev-parse", "HEAD") + + cache := Cache{Root: t.TempDir(), RecurseSubmodules: true} + dst := filepath.Join(t.TempDir(), "workspace", "src") + commit, err := cache.Prepare(context.Background(), parentURL, "", dst) + if err != nil { + t.Fatalf("Prepare: %v", err) + } + if commit != parent.commit { + t.Fatalf("commit = %q, want %q", commit, parent.commit) + } + if err := os.WriteFile(filepath.Join(dst, ".gitmodules"), nil, 0o644); err != nil { + t.Fatal(err) + } + + modules, err := Submodules(context.Background(), dst) + if err != nil { + t.Fatalf("Submodules: %v", err) + } + want := []Submodule{ + { + Path: "third_party/generic", + URL: genericURL, + Commit: generic.commit, + PURL: fmt.Sprintf("pkg:generic/generic?vcs_url=git%%2Bhttps:%%2F%%2Fclone.test%%2Fgeneric.git%%40%s", generic.commit), + Initialized: true, + Status: SubmoduleStatusInitialized, + }, + { + Path: "vendor/outer", + URL: outerURL, + Commit: outer.commit, + PURL: "pkg:github/example/outer@" + outer.commit, + Initialized: true, + Status: SubmoduleStatusInitialized, + }, + { + Path: "vendor/outer/deps/leaf", + URL: leafURL, + Commit: leaf.commit, + PURL: "pkg:github/example/leaf@" + leaf.commit, + Initialized: true, + Status: SubmoduleStatusInitialized, + }, + } + if !reflect.DeepEqual(modules, want) { + t.Fatalf("Submodules() = %#v, want %#v", modules, want) + } +} + +func TestSubmodulesReportsUnavailableWithoutCredentials(t *testing.T) { + requireGit(t) + + missing := newTestRepository(t, "missing.txt") + parent := newTestRepository(t, "parent.txt") + const ( + parentURL = "https://clone.test/unavailable.git" + username = "credential-user" + secret = "submodule-secret" + ) + credentialURL := "https://" + username + ":" + secret + "@127.0.0.1:1/owner/missing.git" + configureTestURLs(t, map[string]string{parentURL: parent.dir}) + + runGitTest(t, parent.dir, "config", "--file", ".gitmodules", "submodule.missing.path", "deps/missing") + runGitTest(t, parent.dir, "config", "--file", ".gitmodules", "submodule.missing.url", credentialURL) + runGitTest(t, parent.dir, "add", ".gitmodules") + runGitTest(t, parent.dir, "update-index", "--add", "--cacheinfo", "160000,"+missing.commit+",deps/missing") + runGitTest(t, parent.dir, "commit", "--quiet", "-m", "add unavailable submodule") + parent.commit = runGitTest(t, parent.dir, "rev-parse", "HEAD") + + cache := Cache{ + Root: t.TempDir(), + RecurseSubmodules: true, + Retry: Retry{Attempts: 1}, + } + dst := filepath.Join(t.TempDir(), "workspace", "src") + commit, err := cache.Prepare(context.Background(), parentURL, "", dst) + if err != nil { + t.Fatalf("Prepare: %v", err) + } + if commit != parent.commit { + t.Fatalf("commit = %q, want %q", commit, parent.commit) + } + + modules, err := Submodules(context.Background(), dst) + if err != nil { + t.Fatalf("Submodules: %v", err) + } + if len(modules) != 1 { + t.Fatalf("len(Submodules()) = %d, want 1", len(modules)) + } + module := modules[0] + if module.Path != "deps/missing" || module.Commit != missing.commit { + t.Errorf("submodule path and commit = %q, %q", module.Path, module.Commit) + } + if module.URL != "https://127.0.0.1:1/owner/missing.git" { + t.Error("URL is not credential-free") + } + wantPURL := fmt.Sprintf( + "pkg:generic/missing?vcs_url=git%%2Bhttps:%%2F%%2F127.0.0.1:1%%2Fowner%%2Fmissing.git%%40%s", + missing.commit, + ) + if module.PURL != wantPURL { + t.Error("PURL is not the expected credential-free identity") + } + if module.Initialized || module.Status != SubmoduleStatusUnavailable || module.Error == "" { + t.Error("unavailable submodule status is incomplete") + } + metadata := fmt.Sprintf("%+v", modules) + if strings.Contains(metadata, username) || strings.Contains(metadata, secret) { + t.Fatal("submodule metadata contains credential") + } +} + +func TestSubmodulesReturnsEmptyForCheckoutWithoutSubmodules(t *testing.T) { + repository := newTestRepository(t, "file.txt") + modules, err := Submodules(context.Background(), repository.dir) + if err != nil { + t.Fatalf("Submodules: %v", err) + } + if len(modules) != 0 { + t.Fatalf("Submodules() = %#v, want none", modules) + } +} From f5568bdac3ccfc17456440ef94ebc54193e5b083 Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 25 Aug 2026 15:38:37 +0100 Subject: [PATCH 2/3] Sync changed submodule URLs --- ensure.go | 14 ++++++++--- ensure_test.go | 31 +++++++++++++++++-------- submodule_test.go | 59 +++++++++++++++++++++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 12 deletions(-) diff --git a/ensure.go b/ensure.go index 38c910f..fddc134 100644 --- a/ensure.go +++ b/ensure.go @@ -102,7 +102,13 @@ func updateSubmodules(ctx context.Context, retry Retry, dst string, enabled bool if !enabled { return nil } - if _, err := retry.Do(ctx, Command{ + policy := retry.Resolved() + if _, err := policy.Run(ctx, dst, remoteEnv(), "submodule", "sync", "--recursive"); err != nil { + if ctxErr := ctx.Err(); ctxErr != nil { + return ctxErr + } + } + if _, err := policy.Do(ctx, Command{ Label: "submodule", Dir: dst, Env: remoteEnv(), @@ -124,7 +130,7 @@ func fetchRef(ctx context.Context, retry Retry, url, dst, ref string, full bool) if target == "" { target = "HEAD" //nolint:goconst // Git's default ref is clearest by its literal name. } - args := []string{"-C", dst, "fetch", "--quiet"} //nolint:goconst // Git argv is clearer with literal subcommands and flags. + args := []string{"-C", dst, "fetch", "--quiet", "--no-recurse-submodules"} //nolint:goconst // Git argv is clearer with literal subcommands and flags. if full { out, _ := policy.Run(ctx, "", nil, "-C", dst, "rev-parse", "--is-shallow-repository") if strings.TrimSpace(out) == "true" { @@ -140,7 +146,9 @@ func fetchRef(ctx context.Context, retry Retry, url, dst, ref string, full bool) if err != nil { return fmt.Errorf("%s: %w", strings.TrimSpace(out), err) } - out, err = policy.Run(ctx, "", nil, "-C", dst, "reset", "--quiet", "--hard", "FETCH_HEAD") + out, err = policy.Run(ctx, "", nil, + "-C", dst, "reset", "--quiet", "--hard", "--no-recurse-submodules", "FETCH_HEAD", + ) if err != nil { return fmt.Errorf("%s: %w", strings.TrimSpace(out), err) } diff --git a/ensure_test.go b/ensure_test.go index fa8a3d9..d949d9f 100644 --- a/ensure_test.go +++ b/ensure_test.go @@ -210,8 +210,10 @@ func TestEnsureWithOptionsIgnoresSubmoduleFailure(t *testing.T) { t.Fatal(err) } - var submoduleArgs []string - var submoduleEnv []string + var syncArgs []string + var syncEnv []string + var updateArgs []string + var updateEnv []string retry := Retry{ Attempts: 1, Run: func(_ context.Context, dir string, env []string, args ...string) (string, error) { @@ -222,8 +224,13 @@ func TestEnsureWithOptionsIgnoresSubmoduleFailure(t *testing.T) { if dir != dst { t.Errorf("submodule dir = %q, want %q", dir, dst) } - submoduleArgs = append([]string(nil), args...) - submoduleEnv = append([]string(nil), env...) + if slices.Contains(args, "sync") { + syncArgs = append([]string(nil), args...) + syncEnv = append([]string(nil), env...) + return "", nil + } + updateArgs = append([]string(nil), args...) + updateEnv = append([]string(nil), env...) return "fatal: repository not found", errGitExit default: return "", errors.New("unexpected Git command") @@ -237,12 +244,18 @@ func TestEnsureWithOptionsIgnoresSubmoduleFailure(t *testing.T) { ); err != nil { t.Fatalf("EnsureWithOptions: %v", err) } - wantArgs := []string{"submodule", "update", "--init", "--recursive", "--depth", "1"} - if !slices.Equal(submoduleArgs, wantArgs) { - t.Errorf("submodule args = %v, want %v", submoduleArgs, wantArgs) + wantSyncArgs := []string{"submodule", "sync", "--recursive"} + if !slices.Equal(syncArgs, wantSyncArgs) { + t.Errorf("submodule sync args = %v, want %v", syncArgs, wantSyncArgs) + } + wantUpdateArgs := []string{"submodule", "update", "--init", "--recursive", "--depth", "1"} + if !slices.Equal(updateArgs, wantUpdateArgs) { + t.Errorf("submodule update args = %v, want %v", updateArgs, wantUpdateArgs) } - if !slices.Contains(submoduleEnv, "GIT_PROTOCOL_FROM_USER=0") { - t.Errorf("submodule env = %v", submoduleEnv) + for name, env := range map[string][]string{"sync": syncEnv, "update": updateEnv} { + if !slices.Contains(env, "GIT_PROTOCOL_FROM_USER=0") { + t.Errorf("submodule %s env = %v", name, env) + } } } diff --git a/submodule_test.go b/submodule_test.go index 0495300..9faaa7b 100644 --- a/submodule_test.go +++ b/submodule_test.go @@ -122,6 +122,65 @@ func TestSubmodulesReportsInitializedNestedIdentities(t *testing.T) { } } +func TestCachePrepareSyncsChangedSubmoduleURL(t *testing.T) { + requireGit(t) + + first := newTestRepository(t, "first.txt") + second := newTestRepository(t, "second.txt") + parent := newTestRepository(t, "parent.txt") + const ( + parentURL = "https://clone.test/retargeted.git" + firstURL = "https://github.com/example/first.git" + secondURL = "https://github.com/example/second.git" + ) + configureTestURLs(t, map[string]string{ + parentURL: parent.dir, + firstURL: first.dir, + secondURL: second.dir, + }) + + runGitTest(t, parent.dir, "submodule", "add", "--quiet", "--name", "dependency", firstURL, "deps/library") + runGitTest(t, parent.dir, "commit", "--quiet", "-am", "add submodule") + + cache := Cache{Root: t.TempDir(), RecurseSubmodules: true} + dst := filepath.Join(t.TempDir(), "workspace", "src") + if _, err := cache.Prepare(context.Background(), parentURL, "", dst); err != nil { + t.Fatalf("first Prepare: %v", err) + } + + runGitTest(t, parent.dir, "config", "--file", ".gitmodules", "submodule.dependency.url", secondURL) + runGitTest(t, parent.dir, "add", ".gitmodules") + runGitTest(t, parent.dir, "update-index", "--cacheinfo", "160000,"+second.commit+",deps/library") + runGitTest(t, parent.dir, "commit", "--quiet", "-m", "retarget submodule") + + if _, err := cache.Prepare(context.Background(), parentURL, "", dst); err != nil { + t.Fatalf("second Prepare: %v", err) + } + content, err := os.ReadFile(filepath.Join(dst, "deps", "library", "second.txt")) + if err != nil { + t.Fatal(err) + } + if string(content) != "second.txt\n" { + t.Errorf("submodule content = %q", content) + } + + modules, err := Submodules(context.Background(), dst) + if err != nil { + t.Fatalf("Submodules: %v", err) + } + want := []Submodule{{ + Path: "deps/library", + URL: secondURL, + Commit: second.commit, + PURL: "pkg:github/example/second@" + second.commit, + Initialized: true, + Status: SubmoduleStatusInitialized, + }} + if !reflect.DeepEqual(modules, want) { + t.Fatalf("Submodules() = %#v, want %#v", modules, want) + } +} + func TestSubmodulesReportsUnavailableWithoutCredentials(t *testing.T) { requireGit(t) From cb6e68ee6e12cd291ade3be5fb6a1e4e262c148a Mon Sep 17 00:00:00 2001 From: Andrew Nesbitt Date: Tue, 25 Aug 2026 15:43:01 +0100 Subject: [PATCH 3/3] Stabilize unresolved submodule URL errors --- submodule.go | 3 --- submodule_test.go | 16 ++++++++++++++++ 2 files changed, 16 insertions(+), 3 deletions(-) diff --git a/submodule.go b/submodule.go index da3b0dc..db02d16 100644 --- a/submodule.go +++ b/submodule.go @@ -213,9 +213,6 @@ func submoduleURL(ctx context.Context, dir string, definition submoduleDefinitio if parseErr == nil && parsed.IsAbs() { return definition.url, nil } - if err != nil { - return "", err - } if parseErr != nil { return "", parseErr } diff --git a/submodule_test.go b/submodule_test.go index 9faaa7b..bfedade 100644 --- a/submodule_test.go +++ b/submodule_test.go @@ -246,6 +246,8 @@ func TestSubmodulesReportsUnavailableWithoutCredentials(t *testing.T) { } func TestSubmodulesReturnsEmptyForCheckoutWithoutSubmodules(t *testing.T) { + requireGit(t) + repository := newTestRepository(t, "file.txt") modules, err := Submodules(context.Background(), repository.dir) if err != nil { @@ -255,3 +257,17 @@ func TestSubmodulesReturnsEmptyForCheckoutWithoutSubmodules(t *testing.T) { t.Fatalf("Submodules() = %#v, want none", modules) } } + +func TestSubmoduleURLReturnsStableErrorForUninitializedRelativeURL(t *testing.T) { + requireGit(t) + + repository := newTestRepository(t, "file.txt") + definition := submoduleDefinition{ + configKey: "submodule.missing", + url: "../missing.git", + } + _, err := submoduleURL(context.Background(), repository.dir, definition) + if err == nil || err.Error() != "relative repository URL is unresolved" { + t.Fatalf("error = %v, want stable unresolved-relative-URL error", err) + } +}