From f867f3d5a16597bf6e0a76371987c801da9c98f4 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 7 Nov 2026 17:34:44 +0100 Subject: [PATCH 01/11] List cached Git repositories in the UI and the ecosystem gauges /ui/git lists the git cache's repositories with their upstream, clone URL, size, last check and use, requests served and the last refresh error. The per-ecosystem cache size and artifact gauges report the git cache as ecosystem="git". --- README.md | 2 +- docs/configuration.md | 14 ++ internal/gitcache/cache.go | 6 + internal/gitcache/refresh.go | 6 +- internal/gitcache/serve.go | 3 + internal/gitcache/stats.go | 148 ++++++++++++++++++ internal/gitcache/stats_test.go | 96 ++++++++++++ internal/server/git_ui.go | 110 +++++++++++++ internal/server/git_ui_test.go | 81 ++++++++++ internal/server/layout.go | 3 + internal/server/server.go | 9 +- internal/server/templates/layout/header.html | 1 + .../templates/pages/git_repositories.html | 40 +++++ 13 files changed, 516 insertions(+), 3 deletions(-) create mode 100644 internal/gitcache/stats.go create mode 100644 internal/gitcache/stats_test.go create mode 100644 internal/server/git_ui.go create mode 100644 internal/server/git_ui_test.go create mode 100644 internal/server/templates/pages/git_repositories.html diff --git a/README.md b/README.md index a43c417b..8e384b9b 100644 --- a/README.md +++ b/README.md @@ -1606,7 +1606,7 @@ The proxy exposes Prometheus metrics at `GET /metrics`. All metric names are pre | `proxy_ecosystem_downloaded_bytes` | gauge | `ecosystem` | Accumulated bytes served from cache: cache hits multiplied by the artifact size they served. | | `proxy_ecosystem_artifact_downloads` | gauge | `ecosystem` | Accumulated artifact downloads served from cache. | | `proxy_ecosystem_cache_size_bytes` | gauge | `ecosystem` | Size of cached artifacts per ecosystem. | -| `proxy_ecosystem_cached_artifacts` | gauge | `ecosystem` | Number of cached artifacts per ecosystem. | +| `proxy_ecosystem_cached_artifacts` | gauge | `ecosystem` | Number of cached artifacts per ecosystem; for `git`, cached repositories. | | `proxy_ecosystem_packages` | gauge | `ecosystem` | Known packages per ecosystem. | | `proxy_ecosystem_versions` | gauge | `ecosystem` | Known package versions per ecosystem. | | `proxy_response_bytes_total` | counter | `ecosystem` | Response body bytes written to clients. Route-labelled, see the label caveat below. | diff --git a/docs/configuration.md b/docs/configuration.md index 584896bc..3af1c6de 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -946,6 +946,20 @@ those yourself. The proxy locks `cache_dir` while it runs, through a `.gitcache-lock` file in it, so a second process given the same directory, such as another replica on a shared volume, fails to start; give each its own. +The web UI lists the cached repositories at `/ui/git`, linked as "Git" in the +navigation: each one's upstream, clone URL, size, the newest commit of the +branch HEAD names (ID, subject and age), how many branches and tags it has, +when the upstream was last checked and a client last read it, how many clones and fetches it served, +and, while the last refresh failed, the kind of error (such as +`fetch_failed` or `too_large`). The request counts and errors are kept in +memory and start over when the proxy restarts. Like the rest of the UI the +page has no authentication, so it shows the names of repositories from +upstreams with credentials to everyone who can reach it, as `/git` lets them +clone those. The `proxy_ecosystem_cache_size_bytes` and +`proxy_ecosystem_cached_artifacts` gauges report the git cache as +`ecosystem="git"`, counting repositories as artifacts, and clones and +fetches count as cache hits or misses there too. + Git LFS objects are cached in the artifact cache, so they use the configured storage backend and retention rather than `cache_dir`. git-lfs finds the LFS API under the rewritten remote, at `/git/{name}/{repo}.git/info/lfs`, with no diff --git a/internal/gitcache/cache.go b/internal/gitcache/cache.go index eb1578f7..29f944be 100644 --- a/internal/gitcache/cache.go +++ b/internal/gitcache/cache.go @@ -136,6 +136,12 @@ type repo struct { lastUsed atomic.Int64 lastChecked atomic.Int64 usedWritten atomic.Int64 + // requests counts clones and fetches, and lastError is the last failed + // refresh, nil once one succeeds. Neither is kept across restarts. + requests atomic.Int64 + lastError atomic.Pointer[refreshError] + // state is the last RepositoryState read. + state atomic.Pointer[cachedState] } var errLocked = errors.New("locked by another process") diff --git a/internal/gitcache/refresh.go b/internal/gitcache/refresh.go index 448b3d9a..e057cf56 100644 --- a/internal/gitcache/refresh.go +++ b/internal/gitcache/refresh.go @@ -151,9 +151,13 @@ func (c *Cache) startRefresh(rp *repo, repoURL string) *refreshCall { cancel() metrics.RecordUpstreamFetch(ecosystem, time.Since(start)) // A refresh cut short by Close is not an upstream failure. - if call.err != nil && c.ctx.Err() == nil { + switch { + case call.err == nil: + rp.lastError.Store(nil) + case c.ctx.Err() == nil: metrics.RecordUpstreamError(ecosystem, errorType(call.err), metrics.FailureDetail{URL: repoURL, Err: call.err}) c.logger.Warn("git cache: refresh failed", "repository", rp.key, "error", call.err) + rp.lastError.Store(&refreshError{kind: errorType(call.err), at: time.Now()}) } c.mu.Lock() rp.refreshing = nil diff --git a/internal/gitcache/serve.go b/internal/gitcache/serve.go index bb5cfb02..0b50291e 100644 --- a/internal/gitcache/serve.go +++ b/internal/gitcache/serve.go @@ -53,6 +53,9 @@ func (c *Cache) Open(ctx context.Context, key, repoURL string, opts OpenOptions) } rp := c.acquire(key) hit := rp.exists() + if opts.Refresh { + rp.requests.Add(1) + } if opts.Refresh || !hit { if err := c.ensureFresh(ctx, rp, repoURL); err != nil { c.release(rp) diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go new file mode 100644 index 00000000..2a8dca9b --- /dev/null +++ b/internal/gitcache/stats.go @@ -0,0 +1,148 @@ +package gitcache + +import ( + "context" + "slices" + "strconv" + "strings" + "time" +) + +// RepositoryStats describes a cached repository. +type RepositoryStats struct { + // Key is the repository's path in the cache, such as + // github-1a2b3c4d/acme/widgets.git. + Key string + Size int64 + // LastChecked is when the upstream's refs were last compared, and + // LastUsed when a client last read the repository. + LastChecked, LastUsed time.Time + // Requests counts clones and fetches since the cache was opened. + Requests int64 + // LastError names the kind of error the last refresh failed with, such + // as fetch_failed or too_large, and LastErrorAt when. Both are empty + // once a refresh succeeds. + LastError string + LastErrorAt time.Time +} + +// refreshError is the last failed refresh of a repository. +type refreshError struct { + kind string + at time.Time +} + +// Repositories describes the repositories in the cache, sorted by key. +func (c *Cache) Repositories() []RepositoryStats { + c.mu.Lock() + repos := make([]*repo, 0, len(c.repos)) + for _, rp := range c.repos { + repos = append(repos, rp) + } + c.mu.Unlock() + + var out []RepositoryStats + for _, rp := range repos { + if !rp.exists() { + continue + } + s := RepositoryStats{ + Key: rp.key, + Size: rp.size.Load(), + LastChecked: unixNanoTime(rp.lastChecked.Load()), + LastUsed: unixNanoTime(rp.lastUsed.Load()), + Requests: rp.requests.Load(), + } + if e := rp.lastError.Load(); e != nil { + s.LastError, s.LastErrorAt = e.kind, e.at + } + out = append(out, s) + } + slices.SortFunc(out, func(a, b RepositoryStats) int { return strings.Compare(a.Key, b.Key) }) + return out +} + +func unixNanoTime(n int64) time.Time { + if n == 0 { + return time.Time{} + } + return time.Unix(0, n) +} + +// RepositoryState is what a cached repository holds: the branch HEAD names +// with its newest commit, and how many branches and tags there are. +type RepositoryState struct { + Branch string + Commit string + CommitTime time.Time + // Subject is the first line of the commit message, at most + // maxSubjectLength bytes. + Subject string + Branches, Tags int +} + +// maxSubjectLength bounds RepositoryState.Subject. +const maxSubjectLength = 200 + +// headFields are the commit ID, time and subject readState asks git log for. +const headFields = 3 + +// cachedState is a repository's state as read after the check at checked. +type cachedState struct { + checked int64 + state RepositoryState +} + +// RepositoryState reads the state of the repository cached under key. It is +// read again only after the repository's next check of the upstream, so a +// page listing many repositories runs git for those that changed. +func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryState, error) { + c.mu.Lock() + rp := c.repos[key] + c.mu.Unlock() + if rp == nil || !rp.exists() { + return RepositoryState{}, ErrNotFound + } + checked := rp.lastChecked.Load() + if cached := rp.state.Load(); cached != nil && cached.checked == checked { + return cached.state, nil + } + state, err := c.readState(ctx, rp.dir) + if err != nil { + return RepositoryState{}, err + } + rp.state.Store(&cachedState{checked: checked, state: state}) + return state, nil +} + +func (c *Cache) readState(ctx context.Context, dir string) (RepositoryState, error) { + var state RepositoryState + refs, err := c.run(ctx, dir, "for-each-ref", "--format=%(refname)", "refs/heads", "refs/tags") + if err != nil { + return state, err + } + for ref := range strings.Lines(refs) { + if strings.HasPrefix(ref, "refs/heads/") { + state.Branches++ + } else { + state.Tags++ + } + } + // A repository with no branch has no HEAD commit to show. + if branch, err := c.run(ctx, dir, "symbolic-ref", "--quiet", "--short", "HEAD"); err == nil { + state.Branch = strings.TrimSpace(branch) + } + if head, err := c.run(ctx, dir, "log", "-1", "--format=%H%x00%ct%x00%s", "HEAD", "--"); err == nil { + fields := strings.SplitN(strings.TrimSpace(head), "\x00", headFields) + if len(fields) == headFields { + state.Commit, state.Subject = fields[0], fields[2] + if unix, err := strconv.ParseInt(fields[1], 10, 64); err == nil { + state.CommitTime = time.Unix(unix, 0) + } + if len(state.Subject) > maxSubjectLength { + state.Subject = strings.ToValidUTF8(state.Subject[:maxSubjectLength], "") + "…" + } + } + } + return state, nil +} diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go new file mode 100644 index 00000000..dc2cd87d --- /dev/null +++ b/internal/gitcache/stats_test.go @@ -0,0 +1,96 @@ +package gitcache + +import ( + "context" + "errors" + "testing" + "time" +) + +func TestRepositoriesDescribesCachedRepositories(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + if got := tc.Repositories(); len(got) != 0 { + t.Fatalf("empty cache lists %+v", got) + } + + start := time.Now().Add(-time.Second) + tc.clone(t, "repo") + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + got := tc.Repositories() + if len(got) != 1 { + t.Fatalf("repositories %+v, want one", got) + } + r := got[0] + if r.Key != "up/repo.git" || r.Size <= 0 || r.Requests != 2 || r.LastError != "" { + t.Errorf("repository %+v, want up/repo.git with a size, 2 requests and no error", r) + } + if r.LastChecked.Before(start) || r.LastUsed.Before(start) { + t.Errorf("checked %v, used %v, want both after %v", r.LastChecked, r.LastUsed, start) + } + + // A refresh that fails is reported until one succeeds. + upstream.server.Close() + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + r = tc.Repositories()[0] + if r.LastError != "fetch_failed" || r.LastErrorAt.Before(start) || r.Requests != 3 { + t.Errorf("after a failed refresh %+v, want fetch_failed and 3 requests", r) + } +} + +// TestRepositoriesListsRepositoriesFromDisk checks a cache opened over a +// cache_dir lists what is already there. +func TestRepositoriesListsRepositoriesFromDisk(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + dir := t.TempDir() + first := newTestCache(t, upstream.server.URL, func(o *Options) { o.CacheDir = dir }) + first.clone(t, "repo") + waitIdle(t, first.Cache) + if err := first.Close(); err != nil { + t.Fatal(err) + } + + second := newTestCache(t, upstream.server.URL, func(o *Options) { o.CacheDir = dir }) + got := second.Repositories() + if len(got) != 1 || got[0].Key != "up/repo.git" || got[0].Size <= 0 || got[0].LastChecked.IsZero() || got[0].Requests != 0 { + t.Errorf("repositories %+v, want up/repo.git with its size and last check", got) + } +} + +func TestRepositoryState(t *testing.T) { + upstream := newFakeUpstream(t) + work := upstream.newRepo("repo") + head := runGit(t, work, "rev-parse", "HEAD") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + + got, err := tc.RepositoryState(context.Background(), "up/repo.git") + if err != nil { + t.Fatalf("RepositoryState: %v", err) + } + if got.Branch != "main" || got.Commit != head || got.Subject != "first" || got.Branches != 1 || got.Tags != 1 || + time.Since(got.CommitTime) > time.Hour { + t.Errorf("state %+v, want main at %s, first, 1 branch and 1 tag", got, head) + } + + // The state follows a refresh that fetched. + runGit(t, work, "checkout", "--quiet", "-b", "feature") + runGit(t, work, "push", "--quiet", "origin", "feature") + runGit(t, work, "checkout", "--quiet", "main") + head = upstream.commit(work, "second") + runGit(t, work, "push", "--quiet", "origin", "main") + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if got, err = tc.RepositoryState(context.Background(), "up/repo.git"); err != nil || got.Commit != head || got.Subject != "second" || got.Branches != 2 { + t.Errorf("after a push: %+v, %v; want main at %s, second, 2 branches", got, err, head) + } + + if _, err := tc.RepositoryState(context.Background(), "up/missing.git"); !errors.Is(err, ErrNotFound) { + t.Errorf("state of an uncached repository: %v, want ErrNotFound", err) + } +} diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go new file mode 100644 index 00000000..11871dec --- /dev/null +++ b/internal/server/git_ui.go @@ -0,0 +1,110 @@ +package server + +import ( + "fmt" + "net/http" + "strings" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/gitcache" + "github.com/git-pkgs/proxy/internal/metrics" +) + +// GitRepositoriesData is the git page: the repositories in the git cache. +type GitRepositoriesData struct { + Layout + Rows []GitRepositoryRow + TotalSize string +} + +// GitRepositoryRow is one cached repository on the git page. +type GitRepositoryRow struct { + Upstream, Repo, CloneURL string + Size string + Requests string + LastChecked, LastUsed string + // LastError names the kind of error the last refresh failed with. + LastError, LastErrorAt string + // Branch, Commit, Subject and CommitAge describe the newest commit of + // the branch HEAD names, and Refs counts branches and tags. + Branch, Commit, Subject, CommitAge string + Refs string +} + +func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { + repos := s.git.Repositories() + state := func(key string) (gitcache.RepositoryState, bool) { + st, err := s.git.RepositoryState(r.Context(), key) + return st, err == nil + } + data := GitRepositoriesData{ + Layout: s.layoutFor(r), + Rows: gitRepositoryRows(s.cfg.BaseURL, s.cfg.Upstream.Git, repos, state), + TotalSize: formatSize(gitEcosystemStats(repos).CacheSize), + } + if err := s.templates.Render(w, "git_repositories", data); err != nil { + s.logger.Error("failed to render git repositories", "error", err) + } +} + +// gitRepositoryRows names each cached repository by its upstream and path, +// and gives the URL it is cloned from through the proxy. Repositories of an +// upstream no longer configured are left out. state reads what a repository +// holds, reporting false when it can't. +func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, repos []gitcache.RepositoryStats, + state func(key string) (gitcache.RepositoryState, bool)) []GitRepositoryRow { + names := make(map[string]string, len(upstreams)) + for name, u := range upstreams { + names[gitcache.UpstreamDir(name, u.URL)] = name + } + rows := make([]GitRepositoryRow, 0, len(repos)) + for _, repo := range repos { + dir, path, _ := strings.Cut(repo.Key, "/") + name, ok := names[dir] + if !ok { + continue + } + requests := fmt.Sprintf("%d requests", repo.Requests) + if repo.Requests == 1 { + requests = "1 request" + } + row := GitRepositoryRow{ + Upstream: name, + Repo: strings.TrimSuffix(path, ".git"), + CloneURL: strings.TrimSuffix(baseURL, "/") + "/git/" + name + "/" + path, + Size: formatSize(repo.Size), + Requests: requests, + LastChecked: formatTimeAgo(repo.LastChecked), + LastUsed: formatTimeAgo(repo.LastUsed), + LastError: repo.LastError, + LastErrorAt: formatTimeAgo(repo.LastErrorAt), + } + if st, ok := state(repo.Key); ok { + row.Branch, row.Subject, row.CommitAge = st.Branch, st.Subject, formatTimeAgo(st.CommitTime) + row.Commit = st.Commit[:min(len(st.Commit), shortCommitLength)] + row.Refs = plural(st.Branches, "branch", "branches") + ", " + plural(st.Tags, "tag", "tags") + } + rows = append(rows, row) + } + return rows +} + +// shortCommitLength is how much of a commit ID the git page shows. +const shortCommitLength = 7 + +func plural(n int, one, many string) string { + if n == 1 { + return "1 " + one + } + return fmt.Sprintf("%d %s", n, many) +} + +// gitEcosystemStats is the git cache's row of the per-ecosystem gauges: its +// repositories as artifacts, and their size. +func gitEcosystemStats(repos []gitcache.RepositoryStats) metrics.EcosystemStats { + stats := metrics.EcosystemStats{Ecosystem: "git", Artifacts: int64(len(repos))} + for _, r := range repos { + stats.CacheSize += r.Size + } + return stats +} diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go new file mode 100644 index 00000000..069fc5d2 --- /dev/null +++ b/internal/server/git_ui_test.go @@ -0,0 +1,81 @@ +package server + +import ( + "net/http" + "net/http/httptest" + "path/filepath" + "strings" + "testing" + "time" + + "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/gitcache" +) + +func TestGitRepositoriesPage(t *testing.T) { + root := t.TempDir() + newGitTestRepo(t, root, "team/repo") + upstream := httptest.NewServer(gitBackend(t, root)) + defer upstream.Close() + cfg, _ := startGitTestServer(t, func(cfg *config.Config) { + cfg.Upstream.Git = map[string]config.GitUpstream{"up": {URL: upstream.URL}} + }) + + if status, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); status != http.StatusOK || !strings.Contains(body, "No repository is cached yet") { + t.Errorf("empty page: status %d\n%s", status, body) + } + gitCommand(t, "", "clone", "--quiet", cfg.BaseURL+"/git/up/team/repo.git", filepath.Join(t.TempDir(), "clone")) + + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/", "", nil); !strings.Contains(body, `href="/ui/git"`) { + t.Error("navigation doesn't link the git page") + } + status, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil) + if status != http.StatusOK { + t.Fatalf("status %d", status) + } + for _, want := range []string{">up<", "team/repo", cfg.BaseURL + "/git/up/team/repo.git", "1 request", "main", "1 branch, 0 tags"} { + if !strings.Contains(body, want) { + t.Errorf("page lacks %q:\n%s", want, body) + } + } +} + +func TestGitRepositoriesPageNeedsUpstreamGit(t *testing.T) { + cfg, _ := startGitTestServer(t, func(*config.Config) {}) + if status, _ := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); status != http.StatusNotFound { + t.Errorf("status %d, want 404 without upstream.git", status) + } + if _, body := request(t, http.MethodGet, cfg.BaseURL+"/ui/", "", nil); strings.Contains(body, `href="/ui/git"`) { + t.Error("navigation links the git page without upstream.git") + } +} + +func TestGitRepositoryRows(t *testing.T) { + upstreams := map[string]config.GitUpstream{"github": {URL: "https://github.com"}} + dir := gitcache.UpstreamDir("github", "https://github.com") + failed := time.Date(2026, 11, 7, 12, 0, 0, 0, time.UTC) + state := func(key string) (gitcache.RepositoryState, bool) { + return gitcache.RepositoryState{ + Branch: "main", Commit: "0123456789abcdef0123456789abcdef01234567", Subject: "Fix the build", + CommitTime: time.Now().Add(-3 * time.Hour), Branches: 2, Tags: 1, + }, key == dir+"/acme/widgets.git" + } + rows := gitRepositoryRows("http://proxy.example", upstreams, []gitcache.RepositoryStats{ + {Key: dir + "/acme/widgets.git", Size: 3 << 20, Requests: 1, LastError: "fetch_failed", LastErrorAt: failed}, + {Key: "gone-12345678/x.git"}, + }, state) + if len(rows) != 1 { + t.Fatalf("rows %+v, want the configured upstream's one", rows) + } + r := rows[0] + if r.Upstream != "github" || r.Repo != "acme/widgets" || r.CloneURL != "http://proxy.example/git/github/acme/widgets.git" || + r.Size != "3.0 MB" || r.Requests != "1 request" || r.LastError != "fetch_failed" || + r.Branch != "main" || r.Commit != "0123456" || r.Subject != "Fix the build" || r.CommitAge != "3 hours ago" || r.Refs != "2 branches, 1 tag" { + t.Errorf("row %+v", r) + } + + stats := gitEcosystemStats([]gitcache.RepositoryStats{{Size: 10}, {Size: 32}}) + if stats.Ecosystem != "git" || stats.Artifacts != 2 || stats.CacheSize != 42 { + t.Errorf("ecosystem stats %+v, want git with 2 repositories and 42 bytes", stats) + } +} diff --git a/internal/server/layout.go b/internal/server/layout.go index 1a63d2ff..798fcd9d 100644 --- a/internal/server/layout.go +++ b/internal/server/layout.go @@ -19,6 +19,8 @@ type Layout struct { CacheDelete bool // Projects shows the project inventory pages. Projects bool + // Git shows the page listing the git cache's repositories. + Git bool } func (s *Server) layoutFor(r *http.Request) Layout { @@ -28,5 +30,6 @@ func (s *Server) layoutFor(r *http.Request) Layout { CanonicalPath: r.URL.Path, CacheDelete: s.cfg.CacheDelete, Projects: s.cfg.Projects.Enabled, + Git: s.git != nil, } } diff --git a/internal/server/server.go b/internal/server/server.go index df0dd25e..c1d7a94a 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -383,6 +383,9 @@ func (s *Server) serve(listener net.Listener) error { ui.Delete("/api/cache/{ecosystem}/*", s.handleCacheDelete) } s.mountProjectUI(ui) + if s.git != nil { + ui.Get("/git", s.handleGitRepositories) + } }) r.Get("/", func(w http.ResponseWriter, r *http.Request) { http.Redirect(w, r, "/ui/", http.StatusFound) @@ -749,7 +752,11 @@ func (s *Server) updateCacheStats() { s.logger.Warn("failed to get ecosystem stats for metrics", "error", err) return } - metrics.UpdateEcosystemStats(ecosystemMetrics(ecosystems)) + published := ecosystemMetrics(ecosystems) + if s.git != nil { + published = append(published, gitEcosystemStats(s.git.Repositories())) + } + metrics.UpdateEcosystemStats(published) } // ecosystemMetrics converts database rows into the metrics package's own diff --git a/internal/server/templates/layout/header.html b/internal/server/templates/layout/header.html index 8b4d94f0..a5740103 100644 --- a/internal/server/templates/layout/header.html +++ b/internal/server/templates/layout/header.html @@ -54,6 +54,7 @@ {{end}} {{define "nav_links"}} +{{if .Git}}Git{{end}} {{if .Projects}}Projects{{end}} Analytics Install diff --git a/internal/server/templates/pages/git_repositories.html b/internal/server/templates/pages/git_repositories.html new file mode 100644 index 00000000..b4015853 --- /dev/null +++ b/internal/server/templates/pages/git_repositories.html @@ -0,0 +1,40 @@ +{{define "title"}}Git repositories - git-pkgs proxy{{end}} + +{{define "content"}} +
+

Git repositories

+

{{len .Rows}} repositories in the git cache, {{.TotalSize}}

+
+ +{{if .Rows}} +
+
+ {{range .Rows}} +
+
+
{{.Upstream}} / {{.Repo}}
+
{{.CloneURL}}
+ {{if .Commit}} +
{{.Branch}}{{if .Branch}} @ {{end}}{{.Commit}} {{.Subject}}{{if .CommitAge}} · {{.CommitAge}}{{end}}
+ {{end}} + {{if .LastError}} +
Last refresh failed: {{.LastError}}{{if .LastErrorAt}}, {{.LastErrorAt}}{{end}}
+ {{end}} +
+
+ {{if .Refs}}{{.Refs}}{{end}} + {{.Size}} + {{.Requests}} + {{if .LastChecked}}checked {{.LastChecked}}{{end}} + {{if .LastUsed}}used {{.LastUsed}}{{end}} +
+
+ {{end}} +
+
+{{else}} +
+

No repository is cached yet

+
+{{end}} +{{end}} From 2b525dc44279938472276bd65a2b1647a4c2f6b3 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 7 Nov 2026 17:57:07 +0100 Subject: [PATCH 02/11] Count the git page's summary from its rows, and keep failed state reads The summary said "1 repositories" and counted the size of repositories it didn't list. A repository whose state can't be read is no longer read on every page view, only after its next upstream check. --- internal/gitcache/stats.go | 17 +++++++++------- internal/gitcache/stats_test.go | 20 +++++++++++++++++++ internal/server/git_ui.go | 20 +++++++++++-------- internal/server/git_ui_test.go | 8 ++++---- .../templates/pages/git_repositories.html | 2 +- 5 files changed, 47 insertions(+), 20 deletions(-) diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index 2a8dca9b..7237c938 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -87,15 +87,18 @@ const maxSubjectLength = 200 // headFields are the commit ID, time and subject readState asks git log for. const headFields = 3 -// cachedState is a repository's state as read after the check at checked. +// cachedState is a repository's state, or the error reading it failed +// with, as read after the check at checked. type cachedState struct { checked int64 state RepositoryState + err error } // RepositoryState reads the state of the repository cached under key. It is // read again only after the repository's next check of the upstream, so a -// page listing many repositories runs git for those that changed. +// page listing many repositories runs git for those that changed. A read +// that failed is kept the same way. func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryState, error) { c.mu.Lock() rp := c.repos[key] @@ -105,14 +108,14 @@ func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryStat } checked := rp.lastChecked.Load() if cached := rp.state.Load(); cached != nil && cached.checked == checked { - return cached.state, nil + return cached.state, cached.err } state, err := c.readState(ctx, rp.dir) - if err != nil { - return RepositoryState{}, err + // A read the request gave up on says nothing about the repository. + if ctx.Err() == nil { + rp.state.Store(&cachedState{checked: checked, state: state, err: err}) } - rp.state.Store(&cachedState{checked: checked, state: state}) - return state, nil + return state, err } func (c *Cache) readState(ctx context.Context, dir string) (RepositoryState, error) { diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index dc2cd87d..70cd304f 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -94,3 +94,23 @@ func TestRepositoryState(t *testing.T) { t.Errorf("state of an uncached repository: %v, want ErrNotFound", err) } } + +// TestRepositoryStateKeepsAFailedRead checks a repository whose state can't +// be read isn't read again until its next upstream check. +func TestRepositoryStateKeepsAFailedRead(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + rp := tc.repo("up/repo.git") + rp.state.Store(&cachedState{checked: rp.lastChecked.Load(), err: errors.New("unreadable")}) + + if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err == nil || err.Error() != "unreadable" { + t.Errorf("state = %v, want the kept error", err) + } + rp.lastChecked.Add(1) + if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil { + t.Errorf("state after a new check = %v, want it read again", err) + } +} diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index 11871dec..0670916b 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -13,8 +13,9 @@ import ( // GitRepositoriesData is the git page: the repositories in the git cache. type GitRepositoriesData struct { Layout - Rows []GitRepositoryRow - TotalSize string + Rows []GitRepositoryRow + // Summary counts the rows and their size. + Summary string } // GitRepositoryRow is one cached repository on the git page. @@ -37,10 +38,11 @@ func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { st, err := s.git.RepositoryState(r.Context(), key) return st, err == nil } + rows, size := gitRepositoryRows(s.cfg.BaseURL, s.cfg.Upstream.Git, repos, state) data := GitRepositoriesData{ - Layout: s.layoutFor(r), - Rows: gitRepositoryRows(s.cfg.BaseURL, s.cfg.Upstream.Git, repos, state), - TotalSize: formatSize(gitEcosystemStats(repos).CacheSize), + Layout: s.layoutFor(r), + Rows: rows, + Summary: plural(len(rows), "repository", "repositories") + " in the git cache, " + formatSize(size), } if err := s.templates.Render(w, "git_repositories", data); err != nil { s.logger.Error("failed to render git repositories", "error", err) @@ -50,14 +52,15 @@ func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { // gitRepositoryRows names each cached repository by its upstream and path, // and gives the URL it is cloned from through the proxy. Repositories of an // upstream no longer configured are left out. state reads what a repository -// holds, reporting false when it can't. +// holds, reporting false when it can't. It also returns the rows' total size. func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, repos []gitcache.RepositoryStats, - state func(key string) (gitcache.RepositoryState, bool)) []GitRepositoryRow { + state func(key string) (gitcache.RepositoryState, bool)) ([]GitRepositoryRow, int64) { names := make(map[string]string, len(upstreams)) for name, u := range upstreams { names[gitcache.UpstreamDir(name, u.URL)] = name } rows := make([]GitRepositoryRow, 0, len(repos)) + var size int64 for _, repo := range repos { dir, path, _ := strings.Cut(repo.Key, "/") name, ok := names[dir] @@ -85,8 +88,9 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, row.Refs = plural(st.Branches, "branch", "branches") + ", " + plural(st.Tags, "tag", "tags") } rows = append(rows, row) + size += repo.Size } - return rows + return rows, size } // shortCommitLength is how much of a commit ID the git page shows. diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index 069fc5d2..b65707d3 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -33,7 +33,7 @@ func TestGitRepositoriesPage(t *testing.T) { if status != http.StatusOK { t.Fatalf("status %d", status) } - for _, want := range []string{">up<", "team/repo", cfg.BaseURL + "/git/up/team/repo.git", "1 request", "main", "1 branch, 0 tags"} { + for _, want := range []string{"1 repository in the git cache", ">up<", "team/repo", cfg.BaseURL + "/git/up/team/repo.git", "1 request", "main", "1 branch, 0 tags"} { if !strings.Contains(body, want) { t.Errorf("page lacks %q:\n%s", want, body) } @@ -60,12 +60,12 @@ func TestGitRepositoryRows(t *testing.T) { CommitTime: time.Now().Add(-3 * time.Hour), Branches: 2, Tags: 1, }, key == dir+"/acme/widgets.git" } - rows := gitRepositoryRows("http://proxy.example", upstreams, []gitcache.RepositoryStats{ + rows, size := gitRepositoryRows("http://proxy.example", upstreams, []gitcache.RepositoryStats{ {Key: dir + "/acme/widgets.git", Size: 3 << 20, Requests: 1, LastError: "fetch_failed", LastErrorAt: failed}, {Key: "gone-12345678/x.git"}, }, state) - if len(rows) != 1 { - t.Fatalf("rows %+v, want the configured upstream's one", rows) + if len(rows) != 1 || size != 3<<20 { + t.Fatalf("rows %+v, size %d; want the configured upstream's one and its size", rows, size) } r := rows[0] if r.Upstream != "github" || r.Repo != "acme/widgets" || r.CloneURL != "http://proxy.example/git/github/acme/widgets.git" || diff --git a/internal/server/templates/pages/git_repositories.html b/internal/server/templates/pages/git_repositories.html index b4015853..a0f5749b 100644 --- a/internal/server/templates/pages/git_repositories.html +++ b/internal/server/templates/pages/git_repositories.html @@ -3,7 +3,7 @@ {{define "content"}}

Git repositories

-

{{len .Rows}} repositories in the git cache, {{.TotalSize}}

+

{{.Summary}}

{{if .Rows}} From abdda9857f6363f760c7ebede175055b0451247a Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 10:08:10 +0100 Subject: [PATCH 03/11] Report the git cache under gauges of its own Git LFS objects are artifacts under ecosystem git, so adding the repositories to the per-ecosystem gauges mixed the two and broke the per-ecosystem figures adding up to the totals. The git cache now has proxy_git_cache_size_bytes and proxy_git_cached_repositories, updated on each tick apart from the database gauges so a failed stats query leaves them current. --- README.md | 4 ++- docs/configuration.md | 8 ++--- internal/metrics/metrics.go | 24 ++++++++++++++ internal/metrics/metrics_test.go | 17 ++++++++++ internal/server/analytics_coverage_test.go | 3 ++ internal/server/git_ui.go | 12 +++---- internal/server/git_ui_test.go | 38 ++++++++++++++++++++-- internal/server/server.go | 19 +++++++---- 8 files changed, 104 insertions(+), 21 deletions(-) diff --git a/README.md b/README.md index 8e384b9b..44642755 100644 --- a/README.md +++ b/README.md @@ -1590,6 +1590,8 @@ The proxy exposes Prometheus metrics at `GET /metrics`. All metric names are pre | `proxy_metadata_cache_served_bytes_total` | counter | `ecosystem` | Metadata bytes served from the metadata cache. | | `proxy_metadata_cache_size_bytes` | gauge | `ecosystem` | Size of cached metadata documents per ecosystem. | | `proxy_metadata_cache_entries` | gauge | `ecosystem` | Number of cached metadata documents per ecosystem. | +| `proxy_git_cache_size_bytes` | gauge | | Size of the cached Git repositories. Git LFS objects are artifacts and count under `proxy_cache_size_bytes`. | +| `proxy_git_cached_repositories` | gauge | | Number of cached Git repositories. | | `proxy_artifacts_evicted_total` | counter | `reason`, `ecosystem` | Artifacts evicted by the size limit (`lru`) or by age (`retention`) | | `proxy_metadata_evicted_total` | counter | `ecosystem` | Cached metadata documents evicted by the size limit | | `proxy_metadata_expired_by_change_feed_total` | counter | `ecosystem` | Cached metadata documents set to revalidate because an upstream [change feed](docs/configuration.md#change-feeds) listed them | @@ -1606,7 +1608,7 @@ The proxy exposes Prometheus metrics at `GET /metrics`. All metric names are pre | `proxy_ecosystem_downloaded_bytes` | gauge | `ecosystem` | Accumulated bytes served from cache: cache hits multiplied by the artifact size they served. | | `proxy_ecosystem_artifact_downloads` | gauge | `ecosystem` | Accumulated artifact downloads served from cache. | | `proxy_ecosystem_cache_size_bytes` | gauge | `ecosystem` | Size of cached artifacts per ecosystem. | -| `proxy_ecosystem_cached_artifacts` | gauge | `ecosystem` | Number of cached artifacts per ecosystem; for `git`, cached repositories. | +| `proxy_ecosystem_cached_artifacts` | gauge | `ecosystem` | Number of cached artifacts per ecosystem. | | `proxy_ecosystem_packages` | gauge | `ecosystem` | Known packages per ecosystem. | | `proxy_ecosystem_versions` | gauge | `ecosystem` | Known package versions per ecosystem. | | `proxy_response_bytes_total` | counter | `ecosystem` | Response body bytes written to clients. Route-labelled, see the label caveat below. | diff --git a/docs/configuration.md b/docs/configuration.md index 3af1c6de..f2738d18 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -955,10 +955,10 @@ and, while the last refresh failed, the kind of error (such as memory and start over when the proxy restarts. Like the rest of the UI the page has no authentication, so it shows the names of repositories from upstreams with credentials to everyone who can reach it, as `/git` lets them -clone those. The `proxy_ecosystem_cache_size_bytes` and -`proxy_ecosystem_cached_artifacts` gauges report the git cache as -`ecosystem="git"`, counting repositories as artifacts, and clones and -fetches count as cache hits or misses there too. +clone those. The `proxy_git_cache_size_bytes` and +`proxy_git_cached_repositories` gauges report the git cache's size and +repository count, apart from the artifact gauges, and clones and fetches +count as cache hits or misses under `ecosystem="git"`. Git LFS objects are cached in the artifact cache, so they use the configured storage backend and retention rather than `cache_dir`. git-lfs finds the LFS diff --git a/internal/metrics/metrics.go b/internal/metrics/metrics.go index 85eeef46..af73eea1 100644 --- a/internal/metrics/metrics.go +++ b/internal/metrics/metrics.go @@ -264,6 +264,22 @@ var ( []string{"ecosystem"}, ) + // Git cache gauges. Repositories are kept apart from the artifact + // figures, which hold Git LFS objects under ecosystem git. + GitCacheSize = prometheus.NewGauge( + prometheus.GaugeOpts{ + Name: "proxy_git_cache_size_bytes", + Help: "Size of the cached Git repositories in bytes", + }, + ) + + GitCachedRepositories = prometheus.NewGauge( + prometheus.GaugeOpts{ + Name: "proxy_git_cached_repositories", + Help: "Number of cached Git repositories", + }, + ) + ResponseBytes = prometheus.NewCounterVec( prometheus.CounterOpts{ Name: "proxy_response_bytes_total", @@ -348,6 +364,8 @@ func init() { EcosystemVersions, MetadataCacheSize, MetadataCacheEntries, + GitCacheSize, + GitCachedRepositories, ResponseBytes, ClientRequests, ClientResponseBytes, @@ -523,6 +541,12 @@ func UpdateCacheStats(sizeBytes, artifactCount int64) { CachedArtifacts.Set(float64(artifactCount)) } +// UpdateGitCacheStats sets the git cache gauges. +func UpdateGitCacheStats(repositories int, sizeBytes int64) { + GitCachedRepositories.Set(float64(repositories)) + GitCacheSize.Set(float64(sizeBytes)) +} + // EcosystemStats is one ecosystem's row of the snapshot published as gauges. type EcosystemStats struct { Ecosystem string diff --git a/internal/metrics/metrics_test.go b/internal/metrics/metrics_test.go index 25ae1cd4..8193462b 100644 --- a/internal/metrics/metrics_test.go +++ b/internal/metrics/metrics_test.go @@ -57,6 +57,23 @@ func TestUpdateCacheStats(t *testing.T) { // No panics = success } +func TestUpdateGitCacheStats(t *testing.T) { + UpdateGitCacheStats(3, 42) + if got := testutil.ToFloat64(GitCachedRepositories); got != 3 { + t.Errorf("proxy_git_cached_repositories = %v, want 3", got) + } + if got := testutil.ToFloat64(GitCacheSize); got != 42 { + t.Errorf("proxy_git_cache_size_bytes = %v, want 42", got) + } + snap, err := Gather() + if err != nil { + t.Fatal(err) + } + if len(snap.Samples("proxy_git_cached_repositories")) != 1 || len(snap.Samples("proxy_git_cache_size_bytes")) != 1 { + t.Error("git cache gauges are not registered") + } +} + func TestCircuitBreakerMetrics(t *testing.T) { UpdateCircuitBreakerState("npmjs.org", 0) // closed UpdateCircuitBreakerState("npmjs.org", 2) // open diff --git a/internal/server/analytics_coverage_test.go b/internal/server/analytics_coverage_test.go index 02e67e2c..6190ed0f 100644 --- a/internal/server/analytics_coverage_test.go +++ b/internal/server/analytics_coverage_test.go @@ -27,6 +27,9 @@ var metricSurface = map[string]string{ "proxy_cached_artifacts_total": "Cached artifacts tile", "proxy_metadata_cache_size_bytes": "Metadata cache card", "proxy_metadata_cache_entries": "Metadata cache card", + // Not on /ui/analytics: the git page's summary shows the same figures. + "proxy_git_cache_size_bytes": "/ui/git summary", + "proxy_git_cached_repositories": "/ui/git summary", // Request-path counters. "proxy_response_bytes_total": "Runtime: Served", diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index 0670916b..15192f5a 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -103,12 +103,12 @@ func plural(n int, one, many string) string { return fmt.Sprintf("%d %s", n, many) } -// gitEcosystemStats is the git cache's row of the per-ecosystem gauges: its -// repositories as artifacts, and their size. -func gitEcosystemStats(repos []gitcache.RepositoryStats) metrics.EcosystemStats { - stats := metrics.EcosystemStats{Ecosystem: "git", Artifacts: int64(len(repos))} +// updateGitCacheStats publishes the git cache gauges. +func (s *Server) updateGitCacheStats() { + repos := s.git.Repositories() + var size int64 for _, r := range repos { - stats.CacheSize += r.Size + size += r.Size } - return stats + metrics.UpdateGitCacheStats(len(repos), size) } diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index b65707d3..cbaf463c 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -1,6 +1,8 @@ package server import ( + "io" + "log/slog" "net/http" "net/http/httptest" "path/filepath" @@ -9,7 +11,10 @@ import ( "time" "github.com/git-pkgs/proxy/internal/config" + "github.com/git-pkgs/proxy/internal/database" "github.com/git-pkgs/proxy/internal/gitcache" + "github.com/git-pkgs/proxy/internal/metrics" + "github.com/prometheus/client_golang/prometheus/testutil" ) func TestGitRepositoriesPage(t *testing.T) { @@ -73,9 +78,36 @@ func TestGitRepositoryRows(t *testing.T) { r.Branch != "main" || r.Commit != "0123456" || r.Subject != "Fix the build" || r.CommitAge != "3 hours ago" || r.Refs != "2 branches, 1 tag" { t.Errorf("row %+v", r) } +} + +// TestGitCacheGauges checks the git cache has gauges of its own, published +// even when the database can't be read. +func TestGitCacheGauges(t *testing.T) { + cacheDir := t.TempDir() + upstreamDir := gitcache.UpstreamDir("up", "https://example.com") + gitCommand(t, "", "init", "--quiet", "--bare", filepath.Join(cacheDir, upstreamDir, "team", "repo.git")) + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + cache, err := gitcache.New(gitcache.Options{ + CacheDir: cacheDir, RefreshWait: time.Second, StaleMaxAge: time.Hour, + Client: &http.Client{}, Logger: logger, Upstreams: map[string]string{"up": upstreamDir}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cache.Close() }) + db, err := database.Create(filepath.Join(t.TempDir(), "proxy.db")) + if err != nil { + t.Fatal(err) + } + _ = db.Close() - stats := gitEcosystemStats([]gitcache.RepositoryStats{{Size: 10}, {Size: 32}}) - if stats.Ecosystem != "git" || stats.Artifacts != 2 || stats.CacheSize != 42 { - t.Errorf("ecosystem stats %+v, want git with 2 repositories and 42 bytes", stats) + metrics.UpdateGitCacheStats(0, 0) + s := &Server{db: db, git: cache, logger: logger} + s.updateGauges() + if got := testutil.ToFloat64(metrics.GitCachedRepositories); got != 1 { + t.Errorf("proxy_git_cached_repositories = %v, want 1", got) + } + if got := testutil.ToFloat64(metrics.GitCacheSize); got <= 0 { + t.Errorf("proxy_git_cache_size_bytes = %v, want the repository's size", got) } } diff --git a/internal/server/server.go b/internal/server/server.go index c1d7a94a..b219ed2f 100644 --- a/internal/server/server.go +++ b/internal/server/server.go @@ -713,8 +713,7 @@ func (s *Server) updateCacheStatsMetrics(ctx context.Context) { defer ticker.Stop() for { - s.updateCacheStats() - s.updateMetadataCacheStats() + s.updateGauges() select { case <-ctx.Done(): return @@ -723,6 +722,16 @@ func (s *Server) updateCacheStatsMetrics(ctx context.Context) { } } +// updateGauges republishes the cache gauges. Each group is updated on its +// own, so one failing leaves the others current. +func (s *Server) updateGauges() { + s.updateCacheStats() + s.updateMetadataCacheStats() + if s.git != nil { + s.updateGitCacheStats() + } +} + // updateMetadataCacheStats republishes the metadata cache gauges from a fresh // read of the snapshot the analytics page also serves. It runs separately // from updateCacheStats so a failure in one leaves the other's gauges current. @@ -752,11 +761,7 @@ func (s *Server) updateCacheStats() { s.logger.Warn("failed to get ecosystem stats for metrics", "error", err) return } - published := ecosystemMetrics(ecosystems) - if s.git != nil { - published = append(published, gitEcosystemStats(s.git.Repositories())) - } - metrics.UpdateEcosystemStats(published) + metrics.UpdateEcosystemStats(ecosystemMetrics(ecosystems)) } // ecosystemMetrics converts database rows into the metrics package's own From 38db7d1c0acfa053f3aa78ae37f75760ab1daa3d Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 10:08:41 +0100 Subject: [PATCH 04/11] Escape the git page's clone URLs Repository paths can hold spaces, '#', '?' or '%', as Azure DevOps project and repository names do, so each segment is escaped as the LFS URLs are. The requests label also uses plural() now. --- internal/server/git_ui.go | 18 ++++++++++++------ internal/server/git_ui_test.go | 15 +++++++++++++++ 2 files changed, 27 insertions(+), 6 deletions(-) diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index 15192f5a..e9c2dbff 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -3,6 +3,7 @@ package server import ( "fmt" "net/http" + "net/url" "strings" "github.com/git-pkgs/proxy/internal/config" @@ -67,16 +68,12 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, if !ok { continue } - requests := fmt.Sprintf("%d requests", repo.Requests) - if repo.Requests == 1 { - requests = "1 request" - } row := GitRepositoryRow{ Upstream: name, Repo: strings.TrimSuffix(path, ".git"), - CloneURL: strings.TrimSuffix(baseURL, "/") + "/git/" + name + "/" + path, + CloneURL: strings.TrimSuffix(baseURL, "/") + "/git/" + escapePath(name+"/"+path), Size: formatSize(repo.Size), - Requests: requests, + Requests: plural(int(repo.Requests), "request", "requests"), LastChecked: formatTimeAgo(repo.LastChecked), LastUsed: formatTimeAgo(repo.LastUsed), LastError: repo.LastError, @@ -93,6 +90,15 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, return rows, size } +// escapePath escapes each segment of a slash-separated path. +func escapePath(path string) string { + segments := strings.Split(path, "/") + for i, s := range segments { + segments[i] = url.PathEscape(s) + } + return strings.Join(segments, "/") +} + // shortCommitLength is how much of a commit ID the git page shows. const shortCommitLength = 7 diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index cbaf463c..e2861ae4 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -80,6 +80,21 @@ func TestGitRepositoryRows(t *testing.T) { } } +// TestGitRepositoryRowsEscapeCloneURL checks path segments such as Azure +// DevOps project names, which may hold spaces, '#' or '%', are escaped. +func TestGitRepositoryRowsEscapeCloneURL(t *testing.T) { + upstreams := map[string]config.GitUpstream{"azure": {URL: "https://dev.azure.com"}} + dir := gitcache.UpstreamDir("azure", "https://dev.azure.com") + none := func(string) (gitcache.RepositoryState, bool) { return gitcache.RepositoryState{}, false } + rows, _ := gitRepositoryRows("http://proxy.example/", upstreams, []gitcache.RepositoryStats{ + {Key: dir + "/org/My Project/_git/My Repo#1?x%.git"}, + }, none) + want := "http://proxy.example/git/azure/org/My%20Project/_git/My%20Repo%231%3Fx%25.git" + if len(rows) != 1 || rows[0].CloneURL != want { + t.Errorf("rows %+v, want clone URL %s", rows, want) + } +} + // TestGitCacheGauges checks the git cache has gauges of its own, published // even when the database can't be read. func TestGitCacheGauges(t *testing.T) { From a06803bdde577cc087917414c4bf9e27dd49b223 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sun, 8 Nov 2026 10:11:20 +0100 Subject: [PATCH 05/11] Report failed git state reads, and retry them after a minute readState ignored failures of symbolic-ref and git log, so a transient one was kept as a state with no commit until the upstream was next checked. It now returns them, telling an empty repository or detached HEAD (exit code 1) apart from other failures, and a failed read is kept for a minute only. --- internal/gitcache/stats.go | 62 +++++++++++++++++++++++---------- internal/gitcache/stats_test.go | 45 +++++++++++++++++++----- 2 files changed, 80 insertions(+), 27 deletions(-) diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index 7237c938..6ce6effe 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -2,6 +2,9 @@ package gitcache import ( "context" + "errors" + "fmt" + "os/exec" "slices" "strconv" "strings" @@ -87,18 +90,23 @@ const maxSubjectLength = 200 // headFields are the commit ID, time and subject readState asks git log for. const headFields = 3 +// stateRetryDelay is how long a failed state read is kept. +const stateRetryDelay = time.Minute + // cachedState is a repository's state, or the error reading it failed -// with, as read after the check at checked. +// with, as read after the check at checked. A failed read is kept until +// retryAt. type cachedState struct { checked int64 state RepositoryState err error + retryAt time.Time } // RepositoryState reads the state of the repository cached under key. It is // read again only after the repository's next check of the upstream, so a // page listing many repositories runs git for those that changed. A read -// that failed is kept the same way. +// that failed is kept for stateRetryDelay. func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryState, error) { c.mu.Lock() rp := c.repos[key] @@ -107,13 +115,14 @@ func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryStat return RepositoryState{}, ErrNotFound } checked := rp.lastChecked.Load() - if cached := rp.state.Load(); cached != nil && cached.checked == checked { + if cached := rp.state.Load(); cached != nil && cached.checked == checked && + (cached.err == nil || time.Now().Before(cached.retryAt)) { return cached.state, cached.err } state, err := c.readState(ctx, rp.dir) // A read the request gave up on says nothing about the repository. if ctx.Err() == nil { - rp.state.Store(&cachedState{checked: checked, state: state, err: err}) + rp.state.Store(&cachedState{checked: checked, state: state, err: err, retryAt: time.Now().Add(stateRetryDelay)}) } return state, err } @@ -131,21 +140,38 @@ func (c *Cache) readState(ctx context.Context, dir string) (RepositoryState, err state.Tags++ } } - // A repository with no branch has no HEAD commit to show. - if branch, err := c.run(ctx, dir, "symbolic-ref", "--quiet", "--short", "HEAD"); err == nil { - state.Branch = strings.TrimSpace(branch) + // Both exit 1, and print nothing, for a detached HEAD and for one with + // no commit yet; any other failure is an error. + branch, err := c.run(ctx, dir, "symbolic-ref", "--quiet", "--short", "HEAD") + if err != nil && !exitedWith(err, 1) { + return state, err } - if head, err := c.run(ctx, dir, "log", "-1", "--format=%H%x00%ct%x00%s", "HEAD", "--"); err == nil { - fields := strings.SplitN(strings.TrimSpace(head), "\x00", headFields) - if len(fields) == headFields { - state.Commit, state.Subject = fields[0], fields[2] - if unix, err := strconv.ParseInt(fields[1], 10, 64); err == nil { - state.CommitTime = time.Unix(unix, 0) - } - if len(state.Subject) > maxSubjectLength { - state.Subject = strings.ToValidUTF8(state.Subject[:maxSubjectLength], "") + "…" - } - } + state.Branch = strings.TrimSpace(branch) + if _, err := c.run(ctx, dir, "rev-parse", "--verify", "--quiet", "HEAD"); exitedWith(err, 1) { + return state, nil + } else if err != nil { + return state, err + } + head, err := c.run(ctx, dir, "log", "-1", "--format=%H%x00%ct%x00%s", "HEAD", "--") + if err != nil { + return state, err + } + fields := strings.SplitN(strings.TrimSpace(head), "\x00", headFields) + if len(fields) != headFields { + return state, fmt.Errorf("git log: unexpected output %q", head) + } + state.Commit, state.Subject = fields[0], fields[2] + if unix, err := strconv.ParseInt(fields[1], 10, 64); err == nil { + state.CommitTime = time.Unix(unix, 0) + } + if len(state.Subject) > maxSubjectLength { + state.Subject = strings.ToValidUTF8(state.Subject[:maxSubjectLength], "") + "…" } return state, nil } + +// exitedWith reports whether err is from a git process that exited with code. +func exitedWith(err error, code int) bool { + var exit *exec.ExitError + return errors.As(err, &exit) && exit.ExitCode() == code +} diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index 70cd304f..c09095ee 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -3,6 +3,9 @@ package gitcache import ( "context" "errors" + "os" + "path/filepath" + "strings" "testing" "time" ) @@ -95,22 +98,46 @@ func TestRepositoryState(t *testing.T) { } } -// TestRepositoryStateKeepsAFailedRead checks a repository whose state can't -// be read isn't read again until its next upstream check. -func TestRepositoryStateKeepsAFailedRead(t *testing.T) { +// TestRepositoryStateRetriesAFailedRead checks a read that fails, here on +// a branch whose commit is missing, is reported as an error and read again +// a while later even if the upstream isn't checked in between. +func TestRepositoryStateRetriesAFailedRead(t *testing.T) { upstream := newFakeUpstream(t) upstream.newRepo("repo") tc := newTestCache(t, upstream.server.URL, nil) tc.clone(t, "repo") waitIdle(t, tc.Cache) rp := tc.repo("up/repo.git") - rp.state.Store(&cachedState{checked: rp.lastChecked.Load(), err: errors.New("unreadable")}) + ref := filepath.Join(rp.dir, "refs", "heads", "main") + head := runGit(t, rp.dir, "rev-parse", "main") + if err := os.WriteFile(ref, []byte(strings.Repeat("1", len(head))+"\n"), 0o600); err != nil { + t.Fatal(err) + } - if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err == nil || err.Error() != "unreadable" { - t.Errorf("state = %v, want the kept error", err) + if st, err := tc.RepositoryState(context.Background(), "up/repo.git"); err == nil { + t.Fatalf("state %+v of a branch with no commit, want an error", st) + } + if err := os.WriteFile(ref, []byte(head+"\n"), 0o600); err != nil { + t.Fatal(err) + } + if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err == nil { + t.Error("failed read not kept for a while") } - rp.lastChecked.Add(1) - if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil { - t.Errorf("state after a new check = %v, want it read again", err) + rp.state.Load().retryAt = time.Now().Add(-time.Second) + if st, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil || st.Commit != head { + t.Errorf("state after the retry delay = %+v, %v; want %s", st, err, head) + } +} + +// TestReadStateOfAnEmptyRepository checks a repository with no commits has +// a state, not an error. +func TestReadStateOfAnEmptyRepository(t *testing.T) { + upstream := newFakeUpstream(t) + tc := newTestCache(t, upstream.server.URL, nil) + dir := filepath.Join(t.TempDir(), "empty.git") + runGit(t, "", "init", "--quiet", "--bare", "--initial-branch=main", dir) + st, err := tc.readState(context.Background(), dir) + if err != nil || st.Branch != "main" || st.Commit != "" || st.Branches != 0 { + t.Errorf("state %+v, %v; want main with no commit", st, err) } } From 0b7f15fb947680e382bc0c7225ad665fa3032040 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Mon, 9 Nov 2026 10:17:51 +0100 Subject: [PATCH 06/11] Fold case when looking up a repository's state Since #678, repositories are tracked under a lowercased key where the cache directory ignores case, so RepositoryState looks the key up the same way. --- internal/gitcache/stats.go | 2 +- internal/gitcache/stats_test.go | 15 +++++++++++++++ 2 files changed, 16 insertions(+), 1 deletion(-) diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index 6ce6effe..da011c9f 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -109,7 +109,7 @@ type cachedState struct { // that failed is kept for stateRetryDelay. func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryState, error) { c.mu.Lock() - rp := c.repos[key] + rp := c.repos[c.repoKey(key)] c.mu.Unlock() if rp == nil || !rp.exists() { return RepositoryState{}, ErrNotFound diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index c09095ee..c89c4cee 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -101,6 +101,21 @@ func TestRepositoryState(t *testing.T) { // TestRepositoryStateRetriesAFailedRead checks a read that fails, here on // a branch whose commit is missing, is reported as an error and read again // a while later even if the upstream isn't checked in between. +// TestRepositoryStateFoldsCase checks that, where keys are folded, a +// repository's state is found under any spelling of its key. +func TestRepositoryStateFoldsCase(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.foldCase = true + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + + if _, err := tc.RepositoryState(context.Background(), "up/REPO.git"); err != nil { + t.Errorf("RepositoryState(up/REPO.git): %v", err) + } +} + func TestRepositoryStateRetriesAFailedRead(t *testing.T) { upstream := newFakeUpstream(t) upstream.newRepo("repo") From 53afa2cbd01ae35cd7e0583ea55a40392f3cb18c Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Mon, 9 Nov 2026 10:30:32 +0100 Subject: [PATCH 07/11] Set foldCase under the cache lock in the test --- internal/gitcache/stats_test.go | 2 ++ 1 file changed, 2 insertions(+) diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index c89c4cee..61ff9eb8 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -107,7 +107,9 @@ func TestRepositoryStateFoldsCase(t *testing.T) { upstream := newFakeUpstream(t) upstream.newRepo("repo") tc := newTestCache(t, upstream.server.URL, nil) + tc.mu.Lock() tc.foldCase = true + tc.mu.Unlock() tc.clone(t, "repo") waitIdle(t, tc.Cache) From e6c3b1e64eb261a53318a914cdabbd5b81793d25 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Mon, 9 Nov 2026 10:44:47 +0100 Subject: [PATCH 08/11] Keep the git page to what /git serves Since main gained the repos allowlist and the refusal of repositories the upstream refused, the page could still list a repository the allowlist no longer allows and show the cached commit of one the upstream refused. Repositories now reports Refused, and the page leaves out what the allowlist doesn't allow and shows no cached state for a refused one. --- docs/configuration.md | 6 ++++-- internal/gitcache/stats.go | 6 ++++++ internal/gitcache/stats_test.go | 28 ++++++++++++++++++++++++++++ internal/server/git_ui.go | 16 +++++++++++++--- internal/server/git_ui_test.go | 25 +++++++++++++++++++++++++ 5 files changed, 76 insertions(+), 5 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 950b2b80..6d675091 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -965,8 +965,10 @@ navigation: each one's upstream, clone URL, size, the newest commit of the branch HEAD names (ID, subject and age), how many branches and tags it has, when the upstream was last checked and a client last read it, how many clones and fetches it served, and, while the last refresh failed, the kind of error (such as -`fetch_failed` or `too_large`). The request counts and errors are kept in -memory and start over when the proxy restarts. Like the rest of the UI the +`fetch_failed` or `too_large`). A repository the upstream refused shows no +commit, and one the `repos` allowlist no longer allows isn't listed, as `/git` +serves neither. The request counts and errors are kept in memory and start over +when the proxy restarts. Like the rest of the UI the page has no authentication, so it shows the names of repositories from upstreams with credentials to everyone who can reach it, as `/git` lets them clone those. The `proxy_git_cache_size_bytes` and diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index da011c9f..40ee30ea 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -27,6 +27,9 @@ type RepositoryStats struct { // once a refresh succeeds. LastError string LastErrorAt time.Time + // Refused is set while the upstream's last answer refused the + // repository or reported it missing, so it isn't served. + Refused bool } // refreshError is the last failed refresh of a repository. @@ -39,8 +42,10 @@ type refreshError struct { func (c *Cache) Repositories() []RepositoryStats { c.mu.Lock() repos := make([]*repo, 0, len(c.repos)) + refused := make(map[*repo]bool) for _, rp := range c.repos { repos = append(repos, rp) + refused[rp] = rp.refused != nil } c.mu.Unlock() @@ -55,6 +60,7 @@ func (c *Cache) Repositories() []RepositoryStats { LastChecked: unixNanoTime(rp.lastChecked.Load()), LastUsed: unixNanoTime(rp.lastUsed.Load()), Requests: rp.requests.Load(), + Refused: refused[rp], } if e := rp.lastError.Load(); e != nil { s.LastError, s.LastErrorAt = e.kind, e.at diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index 61ff9eb8..aa616ba4 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -3,6 +3,7 @@ package gitcache import ( "context" "errors" + "net/http" "os" "path/filepath" "strings" @@ -44,6 +45,33 @@ func TestRepositoriesDescribesCachedRepositories(t *testing.T) { } } +// TestRepositoriesReportsRefusal checks a repository the upstream refused is +// reported as refused until a check succeeds, unlike a failed fetch. +func TestRepositoriesReportsRefusal(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if r := tc.Repositories()[0]; r.Refused { + t.Fatalf("fresh copy reported refused: %+v", r) + } + + upstream.status.Store(http.StatusForbidden) + _, _ = tryGit(t, "", "ls-remote", tc.url("repo")) + waitIdle(t, tc.Cache) + if r := tc.Repositories()[0]; !r.Refused { + t.Errorf("refused copy not reported refused: %+v", r) + } + + upstream.status.Store(0) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if r := tc.Repositories()[0]; r.Refused { + t.Errorf("still reported refused after a check succeeded: %+v", r) + } +} + // TestRepositoriesListsRepositoriesFromDisk checks a cache opened over a // cache_dir lists what is already there. func TestRepositoriesListsRepositoriesFromDisk(t *testing.T) { diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index e9c2dbff..dca26209 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -52,7 +52,9 @@ func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { // gitRepositoryRows names each cached repository by its upstream and path, // and gives the URL it is cloned from through the proxy. Repositories of an -// upstream no longer configured are left out. state reads what a repository +// upstream no longer configured, or that its repos allowlist no longer +// allows, are left out, and a repository the upstream refused shows no +// cached state, as /git serves none of them. state reads what a repository // holds, reporting false when it can't. It also returns the rows' total size. func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, repos []gitcache.RepositoryStats, state func(key string) (gitcache.RepositoryState, bool)) ([]GitRepositoryRow, int64) { @@ -65,7 +67,7 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, for _, repo := range repos { dir, path, _ := strings.Cut(repo.Key, "/") name, ok := names[dir] - if !ok { + if !ok || !upstreams[name].AllowsRepo(strings.TrimSuffix(path, ".git")) { continue } row := GitRepositoryRow{ @@ -79,7 +81,7 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, LastError: repo.LastError, LastErrorAt: formatTimeAgo(repo.LastErrorAt), } - if st, ok := state(repo.Key); ok { + if st, ok := rowState(repo, state); ok { row.Branch, row.Subject, row.CommitAge = st.Branch, st.Subject, formatTimeAgo(st.CommitTime) row.Commit = st.Commit[:min(len(st.Commit), shortCommitLength)] row.Refs = plural(st.Branches, "branch", "branches") + ", " + plural(st.Tags, "tag", "tags") @@ -90,6 +92,14 @@ func gitRepositoryRows(baseURL string, upstreams map[string]config.GitUpstream, return rows, size } +// rowState is what repo holds, unless the upstream refused it. +func rowState(repo gitcache.RepositoryStats, state func(key string) (gitcache.RepositoryState, bool)) (gitcache.RepositoryState, bool) { + if repo.Refused { + return gitcache.RepositoryState{}, false + } + return state(repo.Key) +} + // escapePath escapes each segment of a slash-separated path. func escapePath(path string) string { segments := strings.Split(path, "/") diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index e2861ae4..7d9ec244 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -80,6 +80,31 @@ func TestGitRepositoryRows(t *testing.T) { } } +// TestGitRepositoryRowsHideWhatGitRefuses checks the page leaves out +// repositories the repos allowlist no longer allows, and shows no cached +// commit of one the upstream refused, as /git serves neither. +func TestGitRepositoryRowsHideWhatGitRefuses(t *testing.T) { + upstreams := map[string]config.GitUpstream{"github": {URL: "https://github.com", Repos: []string{"acme/*"}}} + dir := gitcache.UpstreamDir("github", "https://github.com") + state := func(string) (gitcache.RepositoryState, bool) { + return gitcache.RepositoryState{Branch: "main", Commit: "0123456789", Subject: "Secret plans", Branches: 1}, true + } + rows, size := gitRepositoryRows("http://proxy.example", upstreams, []gitcache.RepositoryStats{ + {Key: dir + "/acme/widgets.git", Size: 1, Refused: true, LastError: "refused"}, + {Key: dir + "/acme/gadgets.git", Size: 2, LastError: "fetch_failed"}, + {Key: dir + "/other/secret.git", Size: 4}, + }, state) + if len(rows) != 2 || size != 3 { + t.Fatalf("rows %+v, size %d; want the two allowed repositories", rows, size) + } + if r := rows[0]; r.Repo != "acme/widgets" || r.Commit != "" || r.Subject != "" || r.Branch != "" || r.Refs != "" || r.LastError != "refused" { + t.Errorf("refused row %+v, want no cached state but the error", r) + } + if r := rows[1]; r.Repo != "acme/gadgets" || r.Subject != "Secret plans" { + t.Errorf("row after a failed fetch %+v, want its cached state", r) + } +} + // TestGitRepositoryRowsEscapeCloneURL checks path segments such as Azure // DevOps project names, which may hold spaces, '#' or '%', are escaped. func TestGitRepositoryRowsEscapeCloneURL(t *testing.T) { From 3ea4464f48d4787c60d8b6f57f7a50fdb2ad3710 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Tue, 10 Nov 2026 06:47:13 +0100 Subject: [PATCH 09/11] List git repositories under their directory's spelling Where cache_dir ignores case, the cache tracks repositories under a lowercased key, and Repositories returned that key. The git page then found no configured upstream for an upstream with an uppercase letter, hid repositories that a repos pattern with one allows, and showed lowercased names and clone URLs. The key now comes from the repository's directory, which keeps the case of the clone that created it. The docs also say the page shows default branches and commit subjects, and that the request count includes ls-remote. --- docs/configuration.md | 13 ++++++++----- internal/gitcache/stats.go | 20 +++++++++++++++++--- internal/gitcache/stats_test.go | 29 ++++++++++++++++++++++++++--- 3 files changed, 51 insertions(+), 11 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 6d675091..69e85c64 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -963,15 +963,18 @@ for an upstream whose repositories differ only in case. The web UI lists the cached repositories at `/ui/git`, linked as "Git" in the navigation: each one's upstream, clone URL, size, the newest commit of the branch HEAD names (ID, subject and age), how many branches and tags it has, -when the upstream was last checked and a client last read it, how many clones and fetches it served, +when the upstream was last checked and a client last read it, how many +times clients read its refs (each clone, fetch and `git ls-remote` does), and, while the last refresh failed, the kind of error (such as `fetch_failed` or `too_large`). A repository the upstream refused shows no commit, and one the `repos` allowlist no longer allows isn't listed, as `/git` serves neither. The request counts and errors are kept in memory and start over -when the proxy restarts. Like the rest of the UI the -page has no authentication, so it shows the names of repositories from -upstreams with credentials to everyone who can reach it, as `/git` lets them -clone those. The `proxy_git_cache_size_bytes` and +when the proxy restarts. Like the rest of the UI the page has no +authentication, so it shows everyone who can reach it the names, default +branches and newest commit subjects of repositories from upstreams with +credentials. `/git` lets the same people clone them, but where `/ui` and +`/git` sit behind different access rules, the page shows more than `/git` +does. The `proxy_git_cache_size_bytes` and `proxy_git_cached_repositories` gauges report the git cache's size and repository count, apart from the artifact gauges, and clones and fetches count as cache hits or misses under `ecosystem="git"`. diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index 40ee30ea..510a455b 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -5,6 +5,7 @@ import ( "errors" "fmt" "os/exec" + "path/filepath" "slices" "strconv" "strings" @@ -14,13 +15,16 @@ import ( // RepositoryStats describes a cached repository. type RepositoryStats struct { // Key is the repository's path in the cache, such as - // github-1a2b3c4d/acme/widgets.git. + // github-1a2b3c4d/acme/widgets.git. It is spelled as the directory is, + // also where the cache folds keys, so it keeps the case of the clone + // that created it. Key string Size int64 // LastChecked is when the upstream's refs were last compared, and // LastUsed when a client last read the repository. LastChecked, LastUsed time.Time - // Requests counts clones and fetches since the cache was opened. + // Requests counts the times clients read the repository's refs, which + // each clone, fetch and ls-remote does, since the cache was opened. Requests int64 // LastError names the kind of error the last refresh failed with, such // as fetch_failed or too_large, and LastErrorAt when. Both are empty @@ -55,7 +59,7 @@ func (c *Cache) Repositories() []RepositoryStats { continue } s := RepositoryStats{ - Key: rp.key, + Key: c.dirKey(rp), Size: rp.size.Load(), LastChecked: unixNanoTime(rp.lastChecked.Load()), LastUsed: unixNanoTime(rp.lastUsed.Load()), @@ -71,6 +75,16 @@ func (c *Cache) Repositories() []RepositoryStats { return out } +// dirKey is the key of rp spelled as its directory is, which rp.key is not +// where keys are folded. +func (c *Cache) dirKey(rp *repo) string { + rel, err := filepath.Rel(c.dir, rp.dir) + if err != nil { + return rp.key + } + return filepath.ToSlash(rel) +} + func unixNanoTime(n int64) time.Time { if n == 0 { return time.Time{} diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index aa616ba4..3ed7e693 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -126,9 +126,6 @@ func TestRepositoryState(t *testing.T) { } } -// TestRepositoryStateRetriesAFailedRead checks a read that fails, here on -// a branch whose commit is missing, is reported as an error and read again -// a while later even if the upstream isn't checked in between. // TestRepositoryStateFoldsCase checks that, where keys are folded, a // repository's state is found under any spelling of its key. func TestRepositoryStateFoldsCase(t *testing.T) { @@ -146,6 +143,32 @@ func TestRepositoryStateFoldsCase(t *testing.T) { } } +// TestRepositoriesKeepCaseWhereKeysAreFolded checks that, where keys are +// folded, a repository is listed under its directory's spelling, not the +// folded key, so its upstream and path match the configuration and the +// upstream's names. +func TestRepositoriesKeepCaseWhereKeysAreFolded(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("Repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.mu.Lock() + tc.foldCase = true + tc.mu.Unlock() + tc.clone(t, "Repo") + waitIdle(t, tc.Cache) + + got := tc.Repositories() + if len(got) != 1 || got[0].Key != "up/Repo.git" { + t.Fatalf("repositories %+v, want up/Repo.git", got) + } + if _, err := tc.RepositoryState(context.Background(), got[0].Key); err != nil { + t.Errorf("RepositoryState(%s): %v", got[0].Key, err) + } +} + +// TestRepositoryStateRetriesAFailedRead checks a read that fails, here on +// a branch whose commit is missing, is reported as an error and read again +// a while later even if the upstream isn't checked in between. func TestRepositoryStateRetriesAFailedRead(t *testing.T) { upstream := newFakeUpstream(t) upstream.newRepo("repo") From c878b0c090f62d03710a302a5a2ad1d7556c031c Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Tue, 10 Nov 2026 07:00:03 +0100 Subject: [PATCH 10/11] Bound what a view of the git page spends on reading repositories A view ran four git processes for each repository whose state wasn't cached, with no limit, and each concurrent view did the same. RepositoryState now runs one read at a time and gives up with its context. The page reads for at most two seconds, lists the rest without branches and commits, and says how many are left; the cache keeps what was read, so each reload reads more. Also test that an upstream, a repos pattern and a repository with uppercase letters are listed as spelled, and correct two comments. --- docs/configuration.md | 5 +- internal/gitcache/cache.go | 8 ++- internal/gitcache/stats.go | 41 +++++++++-- internal/gitcache/stats_test.go | 34 +++++++++ internal/server/analytics_coverage_test.go | 3 +- internal/server/git_ui.go | 22 +++++- internal/server/git_ui_test.go | 72 +++++++++++++++++++ .../templates/pages/git_repositories.html | 1 + 8 files changed, 175 insertions(+), 11 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 69e85c64..012e38b5 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -968,7 +968,10 @@ times clients read its refs (each clone, fetch and `git ls-remote` does), and, while the last refresh failed, the kind of error (such as `fetch_failed` or `too_large`). A repository the upstream refused shows no commit, and one the `repos` allowlist no longer allows isn't listed, as `/git` -serves neither. The request counts and errors are kept in memory and start over +serves neither. Branches and commits are read with git, one repository at a +time and for at most two seconds per view, and kept until the repository's +next check of the upstream; the page says how many it hasn't read yet, and +each reload reads more. The request counts and errors are kept in memory and start over when the proxy restarts. Like the rest of the UI the page has no authentication, so it shows everyone who can reach it the names, default branches and newest commit subjects of repositories from upstreams with diff --git a/internal/gitcache/cache.go b/internal/gitcache/cache.go index 457f5747..99c0c55d 100644 --- a/internal/gitcache/cache.go +++ b/internal/gitcache/cache.go @@ -98,6 +98,8 @@ type Cache struct { // foldCase is set where the cache directory ignores case, so keys that // differ only in case, which name one directory, are one repository. foldCase bool + // stateReads lets one RepositoryState read run git at a time. + stateReads chan struct{} ctx context.Context cancel context.CancelFunc @@ -142,8 +144,9 @@ type repo struct { lastUsed atomic.Int64 lastChecked atomic.Int64 usedWritten atomic.Int64 - // requests counts clones and fetches, and lastError is the last failed - // refresh, nil once one succeeds. Neither is kept across restarts. + // requests counts the times clients read the refs, and lastError is the + // last failed refresh, nil once one succeeds. Neither is kept across + // restarts. requests atomic.Int64 lastError atomic.Pointer[refreshError] // state is the last RepositoryState read. @@ -219,6 +222,7 @@ func New(opts Options) (*Cache, error) { fetches: make(chan struct{}, maxConcurrentFetches), uploadPacks: make(chan struct{}, uploadPacks), refListings: make(chan struct{}, refListings), + stateReads: make(chan struct{}, 1), refCheckTimeout: defaultRefCheckTimeout, lowSpeedTime: defaultLowSpeedTime, maxRequestBody: defaultMaxRequestBody, diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index 510a455b..c1d276e2 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -127,6 +127,11 @@ type cachedState struct { // read again only after the repository's next check of the upstream, so a // page listing many repositories runs git for those that changed. A read // that failed is kept for stateRetryDelay. +// +// One read runs at a time, however many callers ask, so listing a large +// cache doesn't start git processes without bound. A caller whose ctx ends +// before its read is done gets ctx's error; a state already read is +// returned without waiting. func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryState, error) { c.mu.Lock() rp := c.repos[c.repoKey(key)] @@ -134,19 +139,43 @@ func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryStat if rp == nil || !rp.exists() { return RepositoryState{}, ErrNotFound } - checked := rp.lastChecked.Load() - if cached := rp.state.Load(); cached != nil && cached.checked == checked && - (cached.err == nil || time.Now().Before(cached.retryAt)) { + if cached := rp.knownState(); cached != nil { + return cached.state, cached.err + } + if err := ctx.Err(); err != nil { + return RepositoryState{}, err + } + select { + case c.stateReads <- struct{}{}: + defer func() { <-c.stateReads }() + case <-ctx.Done(): + return RepositoryState{}, ctx.Err() + } + // The read this one waited for may have been of the same repository. + if cached := rp.knownState(); cached != nil { return cached.state, cached.err } + checked := rp.lastChecked.Load() state, err := c.readState(ctx, rp.dir) - // A read the request gave up on says nothing about the repository. - if ctx.Err() == nil { - rp.state.Store(&cachedState{checked: checked, state: state, err: err, retryAt: time.Now().Add(stateRetryDelay)}) + if err != nil && ctx.Err() != nil { + // A read the caller gave up on says nothing about the repository. + return RepositoryState{}, ctx.Err() } + rp.state.Store(&cachedState{checked: checked, state: state, err: err, retryAt: time.Now().Add(stateRetryDelay)}) return state, err } +// knownState is the state read since rp's last check of the upstream, or nil +// if it has to be read. +func (rp *repo) knownState() *cachedState { + cached := rp.state.Load() + if cached == nil || cached.checked != rp.lastChecked.Load() || + (cached.err != nil && !time.Now().Before(cached.retryAt)) { + return nil + } + return cached +} + func (c *Cache) readState(ctx context.Context, dir string) (RepositoryState, error) { var state RepositoryState refs, err := c.run(ctx, dir, "for-each-ref", "--format=%(refname)", "refs/heads", "refs/tags") diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index 3ed7e693..6be59e85 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -197,6 +197,40 @@ func TestRepositoryStateRetriesAFailedRead(t *testing.T) { } } +// TestRepositoryStateReadsOneAtATime checks a state that isn't cached waits +// for the read running before it, gives up with its context, and that a +// cached state needs neither. +func TestRepositoryStateReadsOneAtATime(t *testing.T) { + upstream := newFakeUpstream(t) + upstream.newRepo("repo") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + + tc.stateReads <- struct{}{} + ctx, cancel := context.WithTimeout(context.Background(), 50*time.Millisecond) + defer cancel() + if st, err := tc.RepositoryState(ctx, "up/repo.git"); !errors.Is(err, context.DeadlineExceeded) { + t.Fatalf("state read while another runs = %+v, %v; want the context's error", st, err) + } + if tc.repo("up/repo.git").state.Load() != nil { + t.Error("a read that gave up was kept") + } + <-tc.stateReads + want, err := tc.RepositoryState(context.Background(), "up/repo.git") + if err != nil || want.Commit == "" { + t.Fatalf("state once the other read is done = %+v, %v", want, err) + } + + tc.stateReads <- struct{}{} + defer func() { <-tc.stateReads }() + done, stop := context.WithCancel(context.Background()) + stop() + if got, err := tc.RepositoryState(done, "up/repo.git"); err != nil || got != want { + t.Errorf("cached state with a read running and the context done = %+v, %v; want %+v", got, err, want) + } +} + // TestReadStateOfAnEmptyRepository checks a repository with no commits has // a state, not an error. func TestReadStateOfAnEmptyRepository(t *testing.T) { diff --git a/internal/server/analytics_coverage_test.go b/internal/server/analytics_coverage_test.go index 6190ed0f..cd15d7c1 100644 --- a/internal/server/analytics_coverage_test.go +++ b/internal/server/analytics_coverage_test.go @@ -27,7 +27,8 @@ var metricSurface = map[string]string{ "proxy_cached_artifacts_total": "Cached artifacts tile", "proxy_metadata_cache_size_bytes": "Metadata cache card", "proxy_metadata_cache_entries": "Metadata cache card", - // Not on /ui/analytics: the git page's summary shows the same figures. + // Not on /ui/analytics: the git page's summary shows these figures for + // the repositories it lists. "proxy_git_cache_size_bytes": "/ui/git summary", "proxy_git_cached_repositories": "/ui/git summary", diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index dca26209..f32ac94c 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -1,10 +1,13 @@ package server import ( + "context" + "errors" "fmt" "net/http" "net/url" "strings" + "time" "github.com/git-pkgs/proxy/internal/config" "github.com/git-pkgs/proxy/internal/gitcache" @@ -17,6 +20,9 @@ type GitRepositoriesData struct { Rows []GitRepositoryRow // Summary counts the rows and their size. Summary string + // Pending counts the rows whose branches and commits this view had no + // time left to read, and is empty when it read them all. + Pending string } // GitRepositoryRow is one cached repository on the git page. @@ -33,10 +39,21 @@ type GitRepositoryRow struct { Refs string } +// gitStateBudget is how long one view of the git page spends reading what +// the repositories hold. The cache keeps what a view read, so the next one +// goes on where this one stopped. +const gitStateBudget = 2 * time.Second + func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { repos := s.git.Repositories() + ctx, cancel := context.WithTimeout(r.Context(), gitStateBudget) + defer cancel() + pending := 0 state := func(key string) (gitcache.RepositoryState, bool) { - st, err := s.git.RepositoryState(r.Context(), key) + st, err := s.git.RepositoryState(ctx, key) + if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { + pending++ + } return st, err == nil } rows, size := gitRepositoryRows(s.cfg.BaseURL, s.cfg.Upstream.Git, repos, state) @@ -45,6 +62,9 @@ func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { Rows: rows, Summary: plural(len(rows), "repository", "repositories") + " in the git cache, " + formatSize(size), } + if pending > 0 { + data.Pending = plural(pending, "repository", "repositories") + } if err := s.templates.Render(w, "git_repositories", data); err != nil { s.logger.Error("failed to render git repositories", "error", err) } diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index 7d9ec244..eed0a34a 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -1,6 +1,7 @@ package server import ( + "context" "io" "log/slog" "net/http" @@ -45,6 +46,77 @@ func TestGitRepositoriesPage(t *testing.T) { } } +// TestGitRepositoriesPageSaysWhatIsNotReadYet checks a view that is out of +// time for reading commits lists the repositories without them and says so. +func TestGitRepositoriesPageSaysWhatIsNotReadYet(t *testing.T) { + cacheDir := t.TempDir() + upstreams := map[string]config.GitUpstream{"up": {URL: "https://example.com"}} + upstreamDir := gitcache.UpstreamDir("up", "https://example.com") + for _, name := range []string{"one", "two"} { + gitCommand(t, "", "init", "--quiet", "--bare", "--initial-branch=main", filepath.Join(cacheDir, upstreamDir, "team", name+".git")) + } + logger := slog.New(slog.NewTextHandler(io.Discard, nil)) + cache, err := gitcache.New(gitcache.Options{ + CacheDir: cacheDir, RefreshWait: time.Second, StaleMaxAge: time.Hour, + Client: &http.Client{}, Logger: logger, Upstreams: map[string]string{"up": upstreamDir}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cache.Close() }) + s := &Server{cfg: &config.Config{BaseURL: "http://proxy.example"}, git: cache, logger: logger, templates: &Templates{}} + s.cfg.Upstream.Git = upstreams + + view := func(ctx context.Context) string { + w := httptest.NewRecorder() + s.handleGitRepositories(w, httptest.NewRequest(http.MethodGet, "/ui/git", nil).WithContext(ctx)) + return w.Body.String() + } + done, stop := context.WithCancel(context.Background()) + stop() + body := view(done) + for _, want := range []string{"2 repositories in the git cache", "team/one", "team/two", "not read yet for 2 repositories"} { + if !strings.Contains(body, want) { + t.Errorf("page out of time lacks %q:\n%s", want, body) + } + } + if strings.Contains(body, "0 branches") { + t.Errorf("page out of time shows a state:\n%s", body) + } + body = view(context.Background()) + if !strings.Contains(body, "0 branches, 0 tags") || strings.Contains(body, "not read yet") { + t.Errorf("page with time shows no state, or says one is missing:\n%s", body) + } +} + +// TestGitRepositoryRowsKeepCase checks an upstream, a repos pattern and a +// repository with uppercase letters are listed as they are spelled. Where +// cache_dir ignores case, as on macOS and Windows, the cache tracks them +// under a lowercased key. +func TestGitRepositoryRowsKeepCase(t *testing.T) { + cacheDir := t.TempDir() + upstreams := map[string]config.GitUpstream{"Up": {URL: "https://example.com", Repos: []string{"Team/*"}}} + upstreamDir := gitcache.UpstreamDir("Up", "https://example.com") + gitCommand(t, "", "init", "--quiet", "--bare", filepath.Join(cacheDir, upstreamDir, "Team", "Repo.git")) + cache, err := gitcache.New(gitcache.Options{ + CacheDir: cacheDir, RefreshWait: time.Second, StaleMaxAge: time.Hour, + Client: &http.Client{}, Logger: slog.New(slog.NewTextHandler(io.Discard, nil)), Upstreams: map[string]string{"Up": upstreamDir}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cache.Close() }) + + rows, _ := gitRepositoryRows("http://proxy.example", upstreams, cache.Repositories(), func(key string) (gitcache.RepositoryState, bool) { + st, err := cache.RepositoryState(context.Background(), key) + return st, err == nil + }) + if len(rows) != 1 || rows[0].Upstream != "Up" || rows[0].Repo != "Team/Repo" || + rows[0].CloneURL != "http://proxy.example/git/Up/Team/Repo.git" || rows[0].Refs == "" { + t.Errorf("rows %+v, want Up / Team/Repo with its clone URL and state", rows) + } +} + func TestGitRepositoriesPageNeedsUpstreamGit(t *testing.T) { cfg, _ := startGitTestServer(t, func(*config.Config) {}) if status, _ := request(t, http.MethodGet, cfg.BaseURL+"/ui/git", "", nil); status != http.StatusNotFound { diff --git a/internal/server/templates/pages/git_repositories.html b/internal/server/templates/pages/git_repositories.html index a0f5749b..fb9bee2c 100644 --- a/internal/server/templates/pages/git_repositories.html +++ b/internal/server/templates/pages/git_repositories.html @@ -4,6 +4,7 @@

Git repositories

{{.Summary}}

+ {{if .Pending}}

Branches and commits not read yet for {{.Pending}}. Reload to see more.

{{end}}
{{if .Rows}} From 4a317730da45ee8809f73cc004015865b3535a1c Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Tue, 10 Nov 2026 17:47:20 +0100 Subject: [PATCH 11/11] Keep a repository's state until its refs are written The state shown on the git page was read again after every check of the upstream, and a check runs on every client info/refs. On a busy proxy each view then read the same repositories again and never got past what fits in two seconds. The state is now kept until the repository's refs or HEAD are written: by a clone, a fetch, a fetch of wanted commits, or a change of the default branch. A check that finds nothing new leaves it in place. A state that fails to read for another reason than the view's deadline is now logged at warn with the repository key. --- docs/configuration.md | 6 ++-- internal/gitcache/cache.go | 8 +++-- internal/gitcache/refresh.go | 21 ++++++++---- internal/gitcache/stats.go | 25 ++++++++------- internal/gitcache/stats_test.go | 57 +++++++++++++++++++++++++++++++++ internal/gitcache/wants.go | 1 + internal/server/git_ui.go | 6 +++- internal/server/git_ui_test.go | 38 ++++++++++++++++++++++ 8 files changed, 138 insertions(+), 24 deletions(-) diff --git a/docs/configuration.md b/docs/configuration.md index 012e38b5..44fe821c 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -969,9 +969,9 @@ and, while the last refresh failed, the kind of error (such as `fetch_failed` or `too_large`). A repository the upstream refused shows no commit, and one the `repos` allowlist no longer allows isn't listed, as `/git` serves neither. Branches and commits are read with git, one repository at a -time and for at most two seconds per view, and kept until the repository's -next check of the upstream; the page says how many it hasn't read yet, and -each reload reads more. The request counts and errors are kept in memory and start over +time and for at most two seconds per view, and kept until a fetch changes +the repository; the page says how many it hasn't read yet, and each reload +reads more. The request counts and errors are kept in memory and start over when the proxy restarts. Like the rest of the UI the page has no authentication, so it shows everyone who can reach it the names, default branches and newest commit subjects of repositories from upstreams with diff --git a/internal/gitcache/cache.go b/internal/gitcache/cache.go index 99c0c55d..2bef68e7 100644 --- a/internal/gitcache/cache.go +++ b/internal/gitcache/cache.go @@ -149,8 +149,12 @@ type repo struct { // restarts. requests atomic.Int64 lastError atomic.Pointer[refreshError] - // state is the last RepositoryState read. - state atomic.Pointer[cachedState] + // state is the last RepositoryState read, and refsWritten counts the + // times the repository's refs or HEAD were written: by a clone, a fetch, + // a fetch of wanted commits and a change of the default branch. A state + // is current while the count is the one it was read at. + state atomic.Pointer[cachedState] + refsWritten atomic.Int64 } var errLocked = errors.New("locked by another process") diff --git a/internal/gitcache/refresh.go b/internal/gitcache/refresh.go index 6a855860..683be053 100644 --- a/internal/gitcache/refresh.go +++ b/internal/gitcache/refresh.go @@ -227,7 +227,7 @@ func (c *Cache) clone(ctx context.Context, rp *repo, repoURL string, remote remo if err := c.fetch(ctx, tmp, repoURL, c.opts.MaxRepoSize, res, refreshFlags, branchesAndTags...); err != nil { return err } - if err := c.setHead(ctx, tmp, remote, ""); err != nil { + if _, err := c.setHead(ctx, tmp, remote, ""); err != nil { return err } size := dirSize(tmp) @@ -240,6 +240,7 @@ func (c *Cache) clone(ctx context.Context, rp *repo, repoURL string, remote remo if err := os.Rename(tmp, rp.dir); err != nil { return fmt.Errorf("moving clone into place: %w", err) } + rp.refsWritten.Add(1) res.settle(rp, size) c.logger.Info("git cache: cloned repository", "repository", rp.key, "size", size) return nil @@ -266,6 +267,7 @@ func (c *Cache) update(ctx context.Context, rp *repo, repoURL string, remote rem } res := c.newReservation() err = c.fetch(ctx, rp.dir, repoURL, limit, res, refreshFlags, branchesAndTags...) + rp.refsWritten.Add(1) res.settle(rp, dirSize(rp.dir)) if err != nil { return err @@ -275,7 +277,11 @@ func (c *Cache) update(ctx context.Context, rp *repo, repoURL string, remote rem c.logger.Warn("git cache: pruning wanted refs failed", "repository", rp.key, "error", err) } head, _ := c.run(ctx, rp.dir, "symbolic-ref", "--quiet", "HEAD") - return c.setHead(ctx, rp.dir, remote, strings.TrimSpace(head)) + moved, err := c.setHead(ctx, rp.dir, remote, strings.TrimSpace(head)) + if moved { + rp.refsWritten.Add(1) + } + return err } // branchesAndTags are the refspecs of a refresh. A bare clone has none of its @@ -340,16 +346,17 @@ func (c *Cache) fetch(ctx context.Context, dir, repoURL string, limit int64, res } // setHead points HEAD at the upstream's default branch, when the upstream -// names one and it is among the cached branches. -func (c *Cache) setHead(ctx context.Context, dir string, remote remoteRefs, current string) error { +// names one and it is among the cached branches. It reports whether it +// wrote HEAD. +func (c *Cache) setHead(ctx context.Context, dir string, remote remoteRefs, current string) (bool, error) { if remote.head == "" || remote.head == current { - return nil + return false, nil } if _, ok := remote.refs[remote.head]; !ok { - return nil + return false, nil } _, err := c.run(ctx, dir, "symbolic-ref", "HEAD", remote.head) - return err + return err == nil, err } // localRefs lists the cached branches and tags, and the names of the wanted diff --git a/internal/gitcache/stats.go b/internal/gitcache/stats.go index c1d276e2..8e3b9ab7 100644 --- a/internal/gitcache/stats.go +++ b/internal/gitcache/stats.go @@ -114,19 +114,20 @@ const headFields = 3 const stateRetryDelay = time.Minute // cachedState is a repository's state, or the error reading it failed -// with, as read after the check at checked. A failed read is kept until -// retryAt. +// with, as read when the repository's refs had been written written times. +// A failed read is kept until retryAt. type cachedState struct { - checked int64 + written int64 state RepositoryState err error retryAt time.Time } // RepositoryState reads the state of the repository cached under key. It is -// read again only after the repository's next check of the upstream, so a -// page listing many repositories runs git for those that changed. A read -// that failed is kept for stateRetryDelay. +// read again only once the repository's refs or HEAD were written, not after +// a check of the upstream that found nothing new, so a page listing many +// repositories runs git for those that changed. A read that failed is kept +// for stateRetryDelay. // // One read runs at a time, however many callers ask, so listing a large // cache doesn't start git processes without bound. A caller whose ctx ends @@ -155,21 +156,23 @@ func (c *Cache) RepositoryState(ctx context.Context, key string) (RepositoryStat if cached := rp.knownState(); cached != nil { return cached.state, cached.err } - checked := rp.lastChecked.Load() + // Counted before the read: a write during it leaves this state stale, + // and it is then read again. + written := rp.refsWritten.Load() state, err := c.readState(ctx, rp.dir) if err != nil && ctx.Err() != nil { // A read the caller gave up on says nothing about the repository. return RepositoryState{}, ctx.Err() } - rp.state.Store(&cachedState{checked: checked, state: state, err: err, retryAt: time.Now().Add(stateRetryDelay)}) + rp.state.Store(&cachedState{written: written, state: state, err: err, retryAt: time.Now().Add(stateRetryDelay)}) return state, err } -// knownState is the state read since rp's last check of the upstream, or nil -// if it has to be read. +// knownState is the state read since rp's refs were last written, or nil if +// it has to be read. func (rp *repo) knownState() *cachedState { cached := rp.state.Load() - if cached == nil || cached.checked != rp.lastChecked.Load() || + if cached == nil || cached.written != rp.refsWritten.Load() || (cached.err != nil && !time.Now().Before(cached.retryAt)) { return nil } diff --git a/internal/gitcache/stats_test.go b/internal/gitcache/stats_test.go index 6be59e85..e159ee33 100644 --- a/internal/gitcache/stats_test.go +++ b/internal/gitcache/stats_test.go @@ -126,6 +126,63 @@ func TestRepositoryState(t *testing.T) { } } +// TestRepositoryStateIsKeptWhileRefsStay checks a check of the upstream that +// finds nothing new leaves the state as read, so a repository clients keep +// cloning isn't read again on every view, and that a new default branch, +// which moves HEAD but no ref, has it read again. +func TestRepositoryStateIsKeptWhileRefsStay(t *testing.T) { + upstream := newFakeUpstream(t) + work := upstream.newRepo("repo") + runGit(t, work, "push", "--quiet", "origin", "main:trunk") + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + rp := tc.repo("up/repo.git") + if st, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil || st.Branch != "main" { + t.Fatalf("state %+v, %v; want main", st, err) + } + read, checked := rp.state.Load(), rp.lastChecked.Load() + + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if rp.lastChecked.Load() == checked { + t.Fatal("the second clone did not check the upstream") + } + if rp.knownState() != read { + t.Error("a check that found no change dropped the state") + } + + runGit(t, upstream.bare(), "symbolic-ref", "HEAD", "refs/heads/trunk") + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if st, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil || st.Branch != "trunk" { + t.Errorf("state after the default branch changed = %+v, %v; want trunk", st, err) + } +} + +// TestRepositoryStateIsReadAgainAfterAWantedFetch checks a commit fetched by +// ID, which writes a ref too, has the state read again. +func TestRepositoryStateIsReadAgainAfterAWantedFetch(t *testing.T) { + upstream := newFakeUpstream(t) + work := upstream.newRepo("repo") + runGit(t, upstream.bare(), "config", "uploadpack.allowAnySHA1InWant", "true") + sha := upstream.pushPullRequest(work, 1, nil) + tc := newTestCache(t, upstream.server.URL, nil) + tc.clone(t, "repo") + waitIdle(t, tc.Cache) + if _, err := tc.RepositoryState(context.Background(), "up/repo.git"); err != nil { + t.Fatal(err) + } + + if err := fetchByID(t, tc.url("repo"), sha); err != nil { + t.Fatalf("fetch by ID: %v", err) + } + waitIdle(t, tc.Cache) + if tc.repo("up/repo.git").knownState() != nil { + t.Error("state kept over a fetch of a wanted commit") + } +} + // TestRepositoryStateFoldsCase checks that, where keys are folded, a // repository's state is found under any spelling of its key. func TestRepositoryStateFoldsCase(t *testing.T) { diff --git a/internal/gitcache/wants.go b/internal/gitcache/wants.go index 8755ba1b..0c42aa57 100644 --- a/internal/gitcache/wants.go +++ b/internal/gitcache/wants.go @@ -132,6 +132,7 @@ func (c *Cache) fetchWants(ctx context.Context, rp *repo, repoURL string, oids [ } res := c.newReservation() err = c.fetch(ctx, rp.dir, repoURL, limit, res, []string{"--atomic"}, refspecs...) + rp.refsWritten.Add(1) res.settle(rp, dirSize(rp.dir)) return err } diff --git a/internal/server/git_ui.go b/internal/server/git_ui.go index f32ac94c..661d48c1 100644 --- a/internal/server/git_ui.go +++ b/internal/server/git_ui.go @@ -51,8 +51,12 @@ func (s *Server) handleGitRepositories(w http.ResponseWriter, r *http.Request) { pending := 0 state := func(key string) (gitcache.RepositoryState, bool) { st, err := s.git.RepositoryState(ctx, key) - if errors.Is(err, context.DeadlineExceeded) || errors.Is(err, context.Canceled) { + switch { + case err == nil: + case errors.Is(err, context.DeadlineExceeded), errors.Is(err, context.Canceled): pending++ + default: + s.logger.Warn("failed to read git repository state", "repository", key, "error", err) } return st, err == nil } diff --git a/internal/server/git_ui_test.go b/internal/server/git_ui_test.go index eed0a34a..d5d2c558 100644 --- a/internal/server/git_ui_test.go +++ b/internal/server/git_ui_test.go @@ -1,11 +1,13 @@ package server import ( + "bytes" "context" "io" "log/slog" "net/http" "net/http/httptest" + "os" "path/filepath" "strings" "testing" @@ -89,6 +91,42 @@ func TestGitRepositoriesPageSaysWhatIsNotReadYet(t *testing.T) { } } +// TestGitRepositoriesPageLogsAStateItCannotRead checks a repository git +// fails to read is still listed, and the log says why it shows no commit. +func TestGitRepositoriesPageLogsAStateItCannotRead(t *testing.T) { + cacheDir := t.TempDir() + upstreamDir := gitcache.UpstreamDir("up", "https://example.com") + repoDir := filepath.Join(cacheDir, upstreamDir, "team", "broken.git") + gitCommand(t, "", "init", "--quiet", "--bare", "--initial-branch=main", repoDir) + // A branch naming a commit the repository doesn't have. + if err := os.WriteFile(filepath.Join(repoDir, "refs", "heads", "main"), []byte(strings.Repeat("1", 40)+"\n"), 0o600); err != nil { + t.Fatal(err) + } + var logs bytes.Buffer + logger := slog.New(slog.NewTextHandler(&logs, nil)) + cache, err := gitcache.New(gitcache.Options{ + CacheDir: cacheDir, RefreshWait: time.Second, StaleMaxAge: time.Hour, + Client: &http.Client{}, Logger: logger, Upstreams: map[string]string{"up": upstreamDir}, + }) + if err != nil { + t.Fatal(err) + } + t.Cleanup(func() { _ = cache.Close() }) + s := &Server{cfg: &config.Config{BaseURL: "http://proxy.example"}, git: cache, logger: logger, templates: &Templates{}} + s.cfg.Upstream.Git = map[string]config.GitUpstream{"up": {URL: "https://example.com"}} + + w := httptest.NewRecorder() + s.handleGitRepositories(w, httptest.NewRequest(http.MethodGet, "/ui/git", nil)) + if body := w.Body.String(); !strings.Contains(body, "team/broken") || strings.Contains(body, "not read yet") { + t.Errorf("page doesn't list the repository, or says it has yet to read it:\n%s", body) + } + for _, want := range []string{"level=WARN", "repository=" + upstreamDir + "/team/broken.git", "error="} { + if !strings.Contains(logs.String(), want) { + t.Errorf("log lacks %q:\n%s", want, logs.String()) + } + } +} + // TestGitRepositoryRowsKeepCase checks an upstream, a repos pattern and a // repository with uppercase letters are listed as they are spelled. Where // cache_dir ignores case, as on macOS and Windows, the cache tracks them