From f867f3d5a16597bf6e0a76371987c801da9c98f4 Mon Sep 17 00:00:00 2001 From: Simon Chrzanowski Date: Sat, 7 Nov 2026 17:34:44 +0100 Subject: [PATCH 1/5] 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 2/5] 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 3/5] 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 4/5] 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 5/5] 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) } }