93270f2
93270f214a7178454d6af97bd0a66f14076263e59b4cf0d
5084267| Status | File | + | - |
|---|---|---|---|
| M |
internal/repos/git/remotes.go
|
9 | 1 |
| M |
internal/web/handlers/repo/code_tree_rows_test.go
|
15 | 0 |
| A |
internal/web/handlers/repo/source_remote_test.go
|
32 | 0 |
| M |
internal/web/handlers/repo/submodule_links.go
|
58 | 4 |
internal/repos/git/remotes.gomodified@@ -6,6 +6,7 @@ import ( | ||
| 6 | 6 | "context" |
| 7 | 7 | "errors" |
| 8 | 8 | "fmt" |
| 9 | + "os" | |
| 9 | 10 | "os/exec" |
| 10 | 11 | "strings" |
| 11 | 12 | ) |
@@ -22,7 +23,9 @@ func FetchRemoteHeadsAndTags(ctx context.Context, gitDir, remoteURL string) erro | ||
| 22 | 23 | return errors.New("git fetch: remoteURL is required") |
| 23 | 24 | } |
| 24 | 25 | //nolint:gosec // G204: gitDir is RepoFS-derived at call sites; remoteURL is caller-allowlisted and passed as argv, not shell. |
| 25 | - cmd := exec.CommandContext(ctx, "git", "-C", gitDir, | |
| 26 | + cmd := exec.CommandContext(ctx, "git", | |
| 27 | + "-c", "protocol.ext.allow=never", | |
| 28 | + "-C", gitDir, | |
| 26 | 29 | "fetch", |
| 27 | 30 | "--quiet", |
| 28 | 31 | "--no-recurse-submodules", |
@@ -30,6 +33,11 @@ func FetchRemoteHeadsAndTags(ctx context.Context, gitDir, remoteURL string) erro | ||
| 30 | 33 | "refs/heads/*:refs/heads/*", |
| 31 | 34 | "refs/tags/*:refs/tags/*", |
| 32 | 35 | ) |
| 36 | + cmd.Env = append(os.Environ(), | |
| 37 | + "GIT_CONFIG_NOSYSTEM=1", | |
| 38 | + "GIT_CONFIG_GLOBAL=/dev/null", | |
| 39 | + "GIT_CONFIG_XDG=/dev/null", | |
| 40 | + ) | |
| 33 | 41 | out, err := cmd.CombinedOutput() |
| 34 | 42 | if err != nil { |
| 35 | 43 | return fmt.Errorf("git fetch remote refs: %w (%s)", err, strings.TrimSpace(string(out))) |
internal/web/handlers/repo/code_tree_rows_test.gomodified@@ -201,3 +201,18 @@ func TestGitHubSubmoduleFetchURL_RejectsUnsupportedRemotes(t *testing.T) { | ||
| 201 | 201 | }) |
| 202 | 202 | } |
| 203 | 203 | } |
| 204 | + | |
| 205 | +func TestAppendSubmoduleBackfillCandidate_DedupesURLs(t *testing.T) { | |
| 206 | + t.Parallel() | |
| 207 | + seen := map[string]struct{}{} | |
| 208 | + var got []submoduleBackfillFetchCandidate | |
| 209 | + got = appendSubmoduleBackfillCandidate(got, seen, submoduleBackfillFetchCandidate{URL: " https://git.example.com/owner/repo.git ", SourceRepoID: 42}) | |
| 210 | + got = appendSubmoduleBackfillCandidate(got, seen, submoduleBackfillFetchCandidate{URL: "https://git.example.com/owner/repo.git"}) | |
| 211 | + got = appendSubmoduleBackfillCandidate(got, seen, submoduleBackfillFetchCandidate{}) | |
| 212 | + if len(got) != 1 { | |
| 213 | + t.Fatalf("candidate count = %d, want 1", len(got)) | |
| 214 | + } | |
| 215 | + if got[0].URL != "https://git.example.com/owner/repo.git" || got[0].SourceRepoID != 42 { | |
| 216 | + t.Fatalf("candidate = %+v", got[0]) | |
| 217 | + } | |
| 218 | +} | |
internal/web/handlers/repo/source_remote_test.goadded@@ -0,0 +1,32 @@ | ||
| 1 | +// SPDX-License-Identifier: AGPL-3.0-or-later | |
| 2 | + | |
| 3 | +package repo | |
| 4 | + | |
| 5 | +import ( | |
| 6 | + "testing" | |
| 7 | + | |
| 8 | + repogit "github.com/tenseleyFlow/shithub/internal/repos/git" | |
| 9 | +) | |
| 10 | + | |
| 11 | +func TestChooseFetchedDefaultBranch(t *testing.T) { | |
| 12 | + t.Parallel() | |
| 13 | + branches := []repogit.RefEntry{ | |
| 14 | + {Name: "feature", OID: "1111111111111111111111111111111111111111"}, | |
| 15 | + {Name: "main", OID: "2222222222222222222222222222222222222222"}, | |
| 16 | + {Name: "trunk", OID: "3333333333333333333333333333333333333333"}, | |
| 17 | + } | |
| 18 | + name, oid := chooseFetchedDefaultBranch("trunk", branches) | |
| 19 | + if name != "trunk" || oid != branches[2].OID { | |
| 20 | + t.Fatalf("current default = %s %s, want trunk %s", name, oid, branches[2].OID) | |
| 21 | + } | |
| 22 | + | |
| 23 | + name, oid = chooseFetchedDefaultBranch("missing", branches) | |
| 24 | + if name != "trunk" || oid != branches[2].OID { | |
| 25 | + t.Fatalf("fallback default = %s %s, want trunk %s", name, oid, branches[2].OID) | |
| 26 | + } | |
| 27 | + | |
| 28 | + name, oid = chooseFetchedDefaultBranch("", []repogit.RefEntry{{Name: "zeta", OID: "aaaa"}}) | |
| 29 | + if name != "zeta" || oid != "aaaa" { | |
| 30 | + t.Fatalf("first default = %s %s, want zeta aaaa", name, oid) | |
| 31 | + } | |
| 32 | +} | |
internal/web/handlers/repo/submodule_links.gomodified@@ -14,6 +14,7 @@ import ( | ||
| 14 | 14 | "github.com/jackc/pgx/v5/pgtype" |
| 15 | 15 | |
| 16 | 16 | orgsdb "github.com/tenseleyFlow/shithub/internal/orgs/sqlc" |
| 17 | + "github.com/tenseleyFlow/shithub/internal/repos" | |
| 17 | 18 | repogit "github.com/tenseleyFlow/shithub/internal/repos/git" |
| 18 | 19 | reposdb "github.com/tenseleyFlow/shithub/internal/repos/sqlc" |
| 19 | 20 | "github.com/tenseleyFlow/shithub/internal/worker" |
@@ -62,13 +63,23 @@ func (h *Handlers) submoduleTreeURL(ctx context.Context, cc *codeContext, remote | ||
| 62 | 63 | return route.RepoURL |
| 63 | 64 | } |
| 64 | 65 | if !existsAtCommit { |
| 65 | - if fetchURL, ok := githubSubmoduleFetchURL(cfg, remoteURL); ok { | |
| 66 | - backfilled, backfillErr := h.backfillSubmoduleCommit(ctx, gitDir, fetchURL, oid) | |
| 66 | + for _, candidate := range h.submoduleBackfillFetchCandidates(ctx, cfg, route, remoteURL) { | |
| 67 | + backfilled, backfillErr := h.backfillSubmoduleCommit(ctx, gitDir, candidate.URL, oid) | |
| 67 | 68 | if backfillErr != nil { |
| 69 | + if candidate.SourceRepoID != 0 { | |
| 70 | + h.markRepoSourceRemoteFetchError(ctx, candidate.SourceRepoID, backfillErr) | |
| 71 | + } | |
| 68 | 72 | if h.d.Logger != nil { |
| 69 | - h.d.Logger.WarnContext(ctx, "code: submodule backfill fetch", "error", backfillErr, "owner", route.Owner, "repo", route.RepoName, "oid", oid, "remote", fetchURL) | |
| 73 | + h.d.Logger.WarnContext(ctx, "code: submodule backfill fetch", "error", backfillErr, "owner", route.Owner, "repo", route.RepoName, "oid", oid, "remote", candidate.URL) | |
| 74 | + } | |
| 75 | + continue | |
| 76 | + } | |
| 77 | + if backfilled { | |
| 78 | + if candidate.SourceRepoID != 0 { | |
| 79 | + if err := h.rq.MarkRepoSourceRemoteFetched(ctx, h.d.Pool, candidate.SourceRepoID); err != nil && h.d.Logger != nil { | |
| 80 | + h.d.Logger.WarnContext(ctx, "code: submodule backfill mark source fetched", "error", err, "repo_id", candidate.SourceRepoID) | |
| 81 | + } | |
| 70 | 82 | } |
| 71 | - } else if backfilled { | |
| 72 | 83 | h.recordSubmoduleBackfill(ctx, route.Owner, route.RepoName, gitDir) |
| 73 | 84 | return route.TreeURL |
| 74 | 85 | } |
@@ -100,6 +111,49 @@ func (h *Handlers) backfillSubmoduleCommit(ctx context.Context, gitDir, fetchURL | ||
| 100 | 111 | return v.(bool), nil |
| 101 | 112 | } |
| 102 | 113 | |
| 114 | +type submoduleBackfillFetchCandidate struct { | |
| 115 | + URL string | |
| 116 | + SourceRepoID int64 | |
| 117 | +} | |
| 118 | + | |
| 119 | +func (h *Handlers) submoduleBackfillFetchCandidates(ctx context.Context, cfg submoduleRouteConfig, route submoduleRoute, remoteURL string) []submoduleBackfillFetchCandidate { | |
| 120 | + var out []submoduleBackfillFetchCandidate | |
| 121 | + seen := map[string]struct{}{} | |
| 122 | + if row, ok := h.lookupSubmoduleRepoRow(ctx, route.Owner, route.RepoName); ok { | |
| 123 | + if sourceRemote, err := h.rq.GetRepoSourceRemote(ctx, h.d.Pool, row.ID); err == nil { | |
| 124 | + if normalized, sourceErr := repos.ValidateSourceRemoteURL(ctx, sourceRemote.RemoteUrl); sourceErr == nil && normalized != "" { | |
| 125 | + out = appendSubmoduleBackfillCandidate(out, seen, submoduleBackfillFetchCandidate{ | |
| 126 | + URL: normalized, | |
| 127 | + SourceRepoID: row.ID, | |
| 128 | + }) | |
| 129 | + } else if sourceErr != nil { | |
| 130 | + h.markRepoSourceRemoteFetchError(ctx, row.ID, sourceErr) | |
| 131 | + if h.d.Logger != nil { | |
| 132 | + h.d.Logger.WarnContext(ctx, "code: submodule stored source remote rejected", "error", sourceErr, "repo_id", row.ID, "remote", sourceRemote.RemoteUrl) | |
| 133 | + } | |
| 134 | + } | |
| 135 | + } else if !errors.Is(err, pgx.ErrNoRows) && h.d.Logger != nil { | |
| 136 | + h.d.Logger.WarnContext(ctx, "code: submodule source remote lookup", "error", err, "repo_id", row.ID) | |
| 137 | + } | |
| 138 | + } | |
| 139 | + if fetchURL, ok := githubSubmoduleFetchURL(cfg, remoteURL); ok { | |
| 140 | + out = appendSubmoduleBackfillCandidate(out, seen, submoduleBackfillFetchCandidate{URL: fetchURL}) | |
| 141 | + } | |
| 142 | + return out | |
| 143 | +} | |
| 144 | + | |
| 145 | +func appendSubmoduleBackfillCandidate(out []submoduleBackfillFetchCandidate, seen map[string]struct{}, candidate submoduleBackfillFetchCandidate) []submoduleBackfillFetchCandidate { | |
| 146 | + candidate.URL = strings.TrimSpace(candidate.URL) | |
| 147 | + if candidate.URL == "" { | |
| 148 | + return out | |
| 149 | + } | |
| 150 | + if _, ok := seen[candidate.URL]; ok { | |
| 151 | + return out | |
| 152 | + } | |
| 153 | + seen[candidate.URL] = struct{}{} | |
| 154 | + return append(out, candidate) | |
| 155 | +} | |
| 156 | + | |
| 103 | 157 | func (h *Handlers) recordSubmoduleBackfill(ctx context.Context, owner, repoName, gitDir string) { |
| 104 | 158 | row, ok := h.lookupSubmoduleRepoRow(ctx, owner, repoName) |
| 105 | 159 | if !ok { |