From 2c6915267b8df2ab19462e5a61c47c1ac7cd84f5 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Tue, 3 Feb 2026 19:37:28 +1100 Subject: [PATCH 01/12] feat: Private gomod repos --- internal/gitclone/manager.go | 34 +++ internal/gitclone/manager_test.go | 49 ++++ internal/strategy/git/git.go | 3 + internal/strategy/gomod/fetcher.go | 55 ++++ internal/strategy/gomod/gomod.go | 50 +++- internal/strategy/gomod/matcher.go | 32 +++ internal/strategy/gomod/matcher_test.go | 123 +++++++++ internal/strategy/gomod/private_fetcher.go | 292 +++++++++++++++++++++ 8 files changed, 625 insertions(+), 13 deletions(-) create mode 100644 internal/strategy/gomod/fetcher.go create mode 100644 internal/strategy/gomod/matcher.go create mode 100644 internal/strategy/gomod/matcher_test.go create mode 100644 internal/strategy/gomod/private_fetcher.go diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 79de399..ed2804b 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -15,6 +15,27 @@ import ( "github.com/alecthomas/errors" ) +var ( + sharedManager *Manager + sharedManagerMu sync.RWMutex +) + +// SetShared stores a Manager instance to be shared across strategies. +// This should be called by the git strategy after creating its Manager. +func SetShared(m *Manager) { + sharedManagerMu.Lock() + defer sharedManagerMu.Unlock() + sharedManager = m +} + +// GetShared retrieves the shared Manager instance if one exists. +// Returns nil if no shared Manager has been set. +func GetShared() *Manager { + sharedManagerMu.RLock() + defer sharedManagerMu.RUnlock() + return sharedManager +} + type State int const ( @@ -429,3 +450,16 @@ func (r *Repository) GetUpstreamRefs(ctx context.Context) (map[string]string, er return ParseGitRefs(output), nil } + +// HasCommit checks if a specific commit/ref exists in the repository. +// Uses git cat-file -e to efficiently check object existence. +// Thread-safe - acquires read lock. +func (r *Repository) HasCommit(ctx context.Context, ref string) bool { + r.mu.RLock() + defer r.mu.RUnlock() + + // #nosec G204 - r.path and ref are controlled by us + cmd := exec.CommandContext(ctx, "git", "-C", r.path, "cat-file", "-e", ref) + err := cmd.Run() + return err == nil +} diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index 6ad97df..ec60e26 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -3,6 +3,7 @@ package gitclone //nolint:testpackage // white-box testing required for unexport import ( "context" "os" + "os/exec" "path/filepath" "testing" "time" @@ -214,3 +215,51 @@ func TestState_String(t *testing.T) { assert.Equal(t, "cloning", StateCloning.String()) assert.Equal(t, "ready", StateReady.String()) } + +func TestRepository_HasCommit(t *testing.T) { + ctx := context.Background() + tmpDir := t.TempDir() + repoPath := filepath.Join(tmpDir, "test-repo") + + // Create a real git repo with a commit + assert.NoError(t, os.MkdirAll(repoPath, 0o755)) + + // Initialize git repo + cmd := exec.Command("git", "-C", repoPath, "init") + assert.NoError(t, cmd.Run()) + + // Configure git user + cmd = exec.Command("git", "-C", repoPath, "config", "user.email", "test@example.com") + assert.NoError(t, cmd.Run()) + cmd = exec.Command("git", "-C", repoPath, "config", "user.name", "Test User") + assert.NoError(t, cmd.Run()) + + // Create a file and commit + testFile := filepath.Join(repoPath, "test.txt") + assert.NoError(t, os.WriteFile(testFile, []byte("test content"), 0o644)) + cmd = exec.Command("git", "-C", repoPath, "add", "test.txt") + assert.NoError(t, cmd.Run()) + cmd = exec.Command("git", "-C", repoPath, "commit", "-m", "Initial commit") + assert.NoError(t, cmd.Run()) + + // Create a tag + cmd = exec.Command("git", "-C", repoPath, "tag", "v1.0.0") + assert.NoError(t, cmd.Run()) + + // Create Repository instance + repo := &Repository{ + state: StateReady, + path: repoPath, + upstreamURL: "https://example.com/test-repo", + fetchSem: make(chan struct{}, 1), + } + repo.fetchSem <- struct{}{} + + // Test HasCommit with existing ref + assert.True(t, repo.HasCommit(ctx, "HEAD")) + assert.True(t, repo.HasCommit(ctx, "v1.0.0")) + + // Test HasCommit with non-existent ref + assert.False(t, repo.HasCommit(ctx, "nonexistent")) + assert.False(t, repo.HasCommit(ctx, "v9.9.9")) +} diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index c1605d1..a5cfc88 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -68,6 +68,9 @@ func New(ctx context.Context, config Config, scheduler jobscheduler.Scheduler, c return nil, errors.Wrap(err, "create clone manager") } + // Register as shared Manager for use by other strategies (e.g., gomod) + gitclone.SetShared(cloneManager) + s := &Strategy{ config: config, cache: cache, diff --git a/internal/strategy/gomod/fetcher.go b/internal/strategy/gomod/fetcher.go new file mode 100644 index 0000000..a67020b --- /dev/null +++ b/internal/strategy/gomod/fetcher.go @@ -0,0 +1,55 @@ +package gomod + +import ( + "context" + "io" + "time" + + "github.com/alecthomas/errors" + "github.com/goproxy/goproxy" +) + +type compositeFetcher struct { + publicFetcher goproxy.Fetcher + privateFetcher goproxy.Fetcher + matcher *ModulePathMatcher +} + +func newCompositeFetcher( + publicFetcher goproxy.Fetcher, + privateFetcher goproxy.Fetcher, + patterns []string, +) *compositeFetcher { + return &compositeFetcher{ + publicFetcher: publicFetcher, + privateFetcher: privateFetcher, + matcher: NewModulePathMatcher(patterns), + } +} + +func (c *compositeFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { + if c.matcher.IsPrivate(path) { + v, tm, err := c.privateFetcher.Query(ctx, path, query) + return v, tm, errors.Wrap(err, "private fetcher query") + } + v, tm, err := c.publicFetcher.Query(ctx, path, query) + return v, tm, errors.Wrap(err, "public fetcher query") +} + +func (c *compositeFetcher) List(ctx context.Context, path string) (versions []string, err error) { + if c.matcher.IsPrivate(path) { + v, err := c.privateFetcher.List(ctx, path) + return v, errors.Wrap(err, "private fetcher list") + } + v, err := c.publicFetcher.List(ctx, path) + return v, errors.Wrap(err, "public fetcher list") +} + +func (c *compositeFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { + if c.matcher.IsPrivate(path) { + i, m, z, err := c.privateFetcher.Download(ctx, path, version) + return i, m, z, errors.Wrap(err, "private fetcher download") + } + i, m, z, err := c.publicFetcher.Download(ctx, path, version) + return i, m, z, errors.Wrap(err, "public fetcher download") +} diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index 3fc91b3..4a42d38 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -7,9 +7,11 @@ import ( "net/http" "net/url" + "github.com/alecthomas/errors" "github.com/goproxy/goproxy" "github.com/block/cachew/internal/cache" + "github.com/block/cachew/internal/gitclone" "github.com/block/cachew/internal/jobscheduler" "github.com/block/cachew/internal/logging" "github.com/block/cachew/internal/strategy" @@ -20,15 +22,17 @@ func init() { } type Config struct { - Proxy string `hcl:"proxy,optional" help:"Upstream Go module proxy URL (defaults to proxy.golang.org)" default:"https://proxy.golang.org"` + Proxy string `hcl:"proxy,optional" help:"Upstream Go module proxy URL (defaults to proxy.golang.org)" default:"https://proxy.golang.org"` + PrivatePaths []string `hcl:"private-paths,optional" help:"Module path patterns for private repositories"` } type Strategy struct { - config Config - cache cache.Cache - logger *slog.Logger - proxy *url.URL - goproxy *goproxy.Goproxy + config Config + cache cache.Cache + logger *slog.Logger + proxy *url.URL + goproxy *goproxy.Goproxy + cloneManager *gitclone.Manager } var _ strategy.Strategy = (*Strategy)(nil) @@ -46,14 +50,34 @@ func New(ctx context.Context, config Config, _ jobscheduler.Scheduler, cache cac proxy: parsedURL, } - s.goproxy = &goproxy.Goproxy{ - Logger: s.logger, - Fetcher: &goproxy.GoFetcher{ - Env: []string{ - "GOPROXY=" + config.Proxy, - "GOSUMDB=off", // Disable checksum database validation in fetcher, to prevent unneccessary double validation - }, + publicFetcher := &goproxy.GoFetcher{ + Env: []string{ + "GOPROXY=" + config.Proxy, + "GOSUMDB=off", // Disable checksum database validation in fetcher, to prevent unneccessary double validation }, + } + + var fetcher goproxy.Fetcher = publicFetcher + + // Configure private repository support if private-paths is specified + if len(config.PrivatePaths) > 0 { + // Get shared Manager from git strategy + cloneManager := gitclone.GetShared() + if cloneManager == nil { + return nil, errors.New("private-paths configured but git strategy not initialized - git strategy with mirror-root is required for private module support") + } + + s.cloneManager = cloneManager + privateFetcher := newPrivateFetcher(s, cloneManager) + fetcher = newCompositeFetcher(publicFetcher, privateFetcher, config.PrivatePaths) + + s.logger.InfoContext(ctx, "Configured private module support", + slog.Any("private_paths", config.PrivatePaths)) + } + + s.goproxy = &goproxy.Goproxy{ + Logger: s.logger, + Fetcher: fetcher, Cacher: &goproxyCacher{ cache: cache, }, diff --git a/internal/strategy/gomod/matcher.go b/internal/strategy/gomod/matcher.go new file mode 100644 index 0000000..3a163f9 --- /dev/null +++ b/internal/strategy/gomod/matcher.go @@ -0,0 +1,32 @@ +package gomod + +import ( + "path" + "strings" +) + +// ModulePathMatcher matches module paths against patterns. +type ModulePathMatcher struct { + patterns []string +} + +// NewModulePathMatcher creates a new matcher with the given patterns. +func NewModulePathMatcher(patterns []string) *ModulePathMatcher { + return &ModulePathMatcher{patterns: patterns} +} + +// IsPrivate checks if a module path matches any private pattern. +func (m *ModulePathMatcher) IsPrivate(modulePath string) bool { + for _, pattern := range m.patterns { + matched, err := path.Match(pattern, modulePath) + if err == nil && matched { + return true + } + + if strings.HasPrefix(modulePath, pattern+"/") || modulePath == pattern { + return true + } + } + + return false +} diff --git a/internal/strategy/gomod/matcher_test.go b/internal/strategy/gomod/matcher_test.go new file mode 100644 index 0000000..d1ce7b0 --- /dev/null +++ b/internal/strategy/gomod/matcher_test.go @@ -0,0 +1,123 @@ +package gomod_test + +import ( + "testing" + + "github.com/block/cachew/internal/strategy/gomod" +) + +func TestModulePathMatcher(t *testing.T) { + tests := []struct { + name string + patterns []string + modulePath string + want bool + }{ + { + name: "exact match single pattern", + patterns: []string{"github.com/squareup"}, + modulePath: "github.com/squareup", + want: true, + }, + { + name: "exact match with multiple patterns", + patterns: []string{"github.com/org1", "github.com/squareup", "github.com/org2"}, + modulePath: "github.com/squareup", + want: true, + }, + { + name: "prefix match - one level deep", + patterns: []string{"github.com/squareup"}, + modulePath: "github.com/squareup/repo", + want: true, + }, + { + name: "prefix match - two levels deep", + patterns: []string{"github.com/squareup"}, + modulePath: "github.com/squareup/repo/submodule", + want: true, + }, + { + name: "prefix match with multiple patterns", + patterns: []string{"github.com/org1", "github.com/squareup"}, + modulePath: "github.com/squareup/repo", + want: true, + }, + { + name: "wildcard match", + patterns: []string{"github.com/squareup/*"}, + modulePath: "github.com/squareup/repo", + want: true, + }, + { + name: "wildcard match - multiple levels", + patterns: []string{"github.com/*/*"}, + modulePath: "github.com/squareup/repo", + want: true, + }, + { + name: "no match - different org", + patterns: []string{"github.com/squareup"}, + modulePath: "github.com/other/repo", + want: false, + }, + { + name: "no match - different host", + patterns: []string{"github.com/squareup"}, + modulePath: "gitlab.com/squareup/repo", + want: false, + }, + { + name: "no match - prefix without slash", + patterns: []string{"github.com/square"}, + modulePath: "github.com/squareup/repo", + want: false, + }, + { + name: "no match - empty patterns", + patterns: []string{}, + modulePath: "github.com/squareup/repo", + want: false, + }, + { + name: "empty module path", + patterns: []string{"github.com/squareup"}, + modulePath: "", + want: false, + }, + { + name: "multiple patterns with no match", + patterns: []string{"github.com/org1", "github.com/org2", "github.com/org3"}, + modulePath: "github.com/squareup/repo", + want: false, + }, + { + name: "pattern with trailing slash", + patterns: []string{"github.com/squareup/"}, + modulePath: "github.com/squareup/repo", + want: false, + }, + { + name: "gopkg.in pattern", + patterns: []string{"gopkg.in/square"}, + modulePath: "gopkg.in/square/go-jose.v2", + want: true, + }, + { + name: "nested GitHub org pattern", + patterns: []string{"github.com/squareup/internal"}, + modulePath: "github.com/squareup/internal/auth", + want: true, + }, + } + + for _, tt := range tests { + t.Run(tt.name, func(t *testing.T) { + matcher := gomod.NewModulePathMatcher(tt.patterns) + got := matcher.IsPrivate(tt.modulePath) + if got != tt.want { + t.Errorf("IsPrivate() = %v, want %v", got, tt.want) + } + }) + } +} diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go new file mode 100644 index 0000000..4d14fce --- /dev/null +++ b/internal/strategy/gomod/private_fetcher.go @@ -0,0 +1,292 @@ +package gomod + +import ( + "bytes" + "context" + "fmt" + "io" + "io/fs" + "log/slog" + "os/exec" + "sort" + "strings" + "time" + + "github.com/alecthomas/errors" + "golang.org/x/mod/semver" + + "github.com/block/cachew/internal/gitclone" +) + +type privateFetcher struct { + gomod *Strategy + cloneManager *gitclone.Manager +} + +func newPrivateFetcher(gomod *Strategy, cloneManager *gitclone.Manager) *privateFetcher { + return &privateFetcher{ + gomod: gomod, + cloneManager: cloneManager, + } +} + +func (p *privateFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { + logger := p.gomod.logger.With(slog.String("module", path), slog.String("query", query)) + logger.DebugContext(ctx, "Private fetcher: Query") + + gitURL := p.modulePathToGitURL(path) + + repo, err := p.cloneManager.GetOrCreate(ctx, gitURL) + if err != nil { + return "", time.Time{}, errors.Wrapf(err, "get or create clone for %s", path) + } + + // Verify repository is ready + if repo.State() != gitclone.StateReady { + return "", time.Time{}, errors.Errorf("repository %s not cloned yet", gitURL) + } + + resolvedVersion, commitTime, err := p.resolveVersionQuery(ctx, repo, query) + if err != nil { + return "", time.Time{}, errors.Wrapf(err, "resolve version query %s", query) + } + + return resolvedVersion, commitTime, nil +} + +func (p *privateFetcher) List(ctx context.Context, path string) (versions []string, err error) { + logger := p.gomod.logger.With(slog.String("module", path)) + logger.DebugContext(ctx, "Private fetcher: List") + + gitURL := p.modulePathToGitURL(path) + repo, err := p.cloneManager.GetOrCreate(ctx, gitURL) + if err != nil { + return nil, errors.Wrapf(err, "get or create clone for %s", path) + } + + // Verify repository is ready + if repo.State() != gitclone.StateReady { + return nil, errors.Errorf("repository %s not cloned yet", gitURL) + } + + versions, err = p.listVersions(ctx, repo) + if err != nil { + return nil, errors.Wrap(err, "list versions") + } + + return versions, nil +} + +func (p *privateFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { + logger := p.gomod.logger.With(slog.String("module", path), slog.String("version", version)) + logger.DebugContext(ctx, "Private fetcher: Download") + + gitURL := p.modulePathToGitURL(path) + repo, err := p.cloneManager.GetOrCreate(ctx, gitURL) + if err != nil { + return nil, nil, nil, errors.Wrapf(err, "get or create clone for %s", path) + } + + // Verify repository is ready + if repo.State() != gitclone.StateReady { + return nil, nil, nil, errors.Errorf("repository %s not cloned yet", gitURL) + } + + // Verify the specific version/commit exists + if err := p.verifyCommitExists(ctx, repo, version); err != nil { + return nil, nil, nil, err + } + + infoReader, err := p.generateInfo(ctx, repo, version) + if err != nil { + return nil, nil, nil, errors.Wrap(err, "generate info") + } + + modReader := p.generateMod(ctx, repo, path, version) + + zipReader, err := p.generateZip(ctx, repo, path, version) + if err != nil { + _ = infoReader.Close() + _ = modReader.Close() + return nil, nil, nil, errors.Wrap(err, "generate zip") + } + + return infoReader, modReader, zipReader, nil +} + +func (p *privateFetcher) modulePathToGitURL(modulePath string) string { + return "https://" + modulePath +} + +// verifyCommitExists checks if a specific commit exists in the repo. +// Uses HasCommit which acquires a read lock and runs git cat-file -e. +// Returns an error if commit doesn't exist. +func (p *privateFetcher) verifyCommitExists(ctx context.Context, repo *gitclone.Repository, ref string) error { + if !repo.HasCommit(ctx, ref) { + return errors.Errorf("commit %s not found in repository %s", ref, repo.UpstreamURL()) + } + return nil +} + +// resolveVersionQuery resolves a version query (like "latest" or "v1.2.3") to a specific version. +func (p *privateFetcher) resolveVersionQuery(ctx context.Context, repo *gitclone.Repository, query string) (string, time.Time, error) { + if query == "latest" { + versions, err := p.listVersions(ctx, repo) + if err != nil || len(versions) == 0 { + return p.getDefaultBranchVersion(ctx, repo) + } + + latestVersion := versions[len(versions)-1] + commitTime, err := p.getCommitTime(ctx, repo, latestVersion) + if err != nil { + return "", time.Time{}, err + } + return latestVersion, commitTime, nil + } + + if semver.IsValid(query) { + commitTime, err := p.getCommitTime(ctx, repo, query) + if err != nil { + return "", time.Time{}, fs.ErrNotExist + } + return query, commitTime, nil + } + + return p.getDefaultBranchVersion(ctx, repo) +} + +func (p *privateFetcher) listVersions(ctx context.Context, repo *gitclone.Repository) ([]string, error) { + var output []byte + var err error + + repo.WithReadLock(func() { + // #nosec G204 - repo.Path() is controlled by us + cmd := exec.CommandContext(ctx, "git", "-C", repo.Path(), "tag", "-l", "v*") + output, err = cmd.CombinedOutput() + }) + + if err != nil { + return nil, errors.Wrapf(err, "git tag failed: %s", string(output)) + } + + var versions []string + for line := range strings.Lines(string(output)) { + line = strings.TrimSpace(line) + if line != "" && semver.IsValid(line) { + versions = append(versions, line) + } + } + + sort.Slice(versions, func(i, j int) bool { + return semver.Compare(versions[i], versions[j]) < 0 + }) + + return versions, nil +} + +func (p *privateFetcher) getCommitTime(ctx context.Context, repo *gitclone.Repository, ref string) (time.Time, error) { + var output []byte + var err error + + repo.WithReadLock(func() { + // #nosec G204 - repo.Path() and ref are controlled by us + cmd := exec.CommandContext(ctx, "git", "-C", repo.Path(), "log", "-1", "--format=%cI", ref) + output, err = cmd.CombinedOutput() + }) + + if err != nil { + return time.Time{}, errors.Wrapf(err, "git log failed: %s", string(output)) + } + + timeStr := strings.TrimSpace(string(output)) + t, err := time.Parse(time.RFC3339, timeStr) + return t, errors.Wrap(err, "parse commit time") +} + +func (p *privateFetcher) getDefaultBranchVersion(ctx context.Context, repo *gitclone.Repository) (string, time.Time, error) { + var output []byte + var err error + + repo.WithReadLock(func() { + // #nosec G204 - repo.Path() is controlled by us + cmd := exec.CommandContext(ctx, "git", "-C", repo.Path(), "rev-parse", "HEAD") + output, err = cmd.CombinedOutput() + }) + + if err != nil { + return "", time.Time{}, errors.Wrapf(err, "git rev-parse failed: %s", string(output)) + } + + commitHash := strings.TrimSpace(string(output)) + commitTime, err := p.getCommitTime(ctx, repo, "HEAD") + if err != nil { + return "", time.Time{}, err + } + + pseudoVersion := fmt.Sprintf("v0.0.0-%s-%s", + commitTime.UTC().Format("20060102150405"), + commitHash[:12]) + + return pseudoVersion, commitTime, nil +} + +func (p *privateFetcher) generateInfo(ctx context.Context, repo *gitclone.Repository, version string) (io.ReadSeekCloser, error) { + commitTime, err := p.getCommitTime(ctx, repo, version) + if err != nil { + return nil, err + } + + info := fmt.Sprintf(`{"Version":"%s","Time":"%s"}`, version, commitTime.Format(time.RFC3339)) + return newReadSeekCloser(bytes.NewReader([]byte(info))), nil +} + +func (p *privateFetcher) generateMod(ctx context.Context, repo *gitclone.Repository, modulePath, version string) io.ReadSeekCloser { + var output []byte + var err error + + repo.WithReadLock(func() { + // #nosec G204 - version and repo.Path() are controlled by this package, not user input + cmd := exec.CommandContext(ctx, "git", "-C", repo.Path(), "show", fmt.Sprintf("%s:go.mod", version)) + output, err = cmd.CombinedOutput() + }) + + if err != nil { + minimal := fmt.Sprintf("module %s\n\ngo 1.21\n", modulePath) + return newReadSeekCloser(bytes.NewReader([]byte(minimal))) + } + + return newReadSeekCloser(bytes.NewReader(output)) +} + +func (p *privateFetcher) generateZip(ctx context.Context, repo *gitclone.Repository, modulePath, version string) (io.ReadSeekCloser, error) { + prefix := fmt.Sprintf("%s@%s/", modulePath, version) + var output []byte + var err error + + repo.WithReadLock(func() { + // #nosec G204 - version and repo.Path() are controlled by this package, not user input + cmd := exec.CommandContext(ctx, "git", "-C", repo.Path(), "archive", + "--format=zip", + fmt.Sprintf("--prefix=%s", prefix), + version) + output, err = cmd.CombinedOutput() + }) + + if err != nil { + return nil, errors.Wrapf(err, "git archive failed: %s", string(output)) + } + + return newReadSeekCloser(bytes.NewReader(output)), nil +} + +type readSeekCloser struct { + *bytes.Reader +} + +func newReadSeekCloser(r *bytes.Reader) io.ReadSeekCloser { + return &readSeekCloser{Reader: r} +} + +func (r *readSeekCloser) Close() error { + return nil +} From 0a1e0d5fa271431409475112c187595a41898b54 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Tue, 3 Feb 2026 20:08:41 +1100 Subject: [PATCH 02/12] fix: Cleanup a little --- go.mod | 2 +- internal/gitclone/manager.go | 3 --- internal/strategy/git/git.go | 1 - internal/strategy/gomod/gomod.go | 2 -- internal/strategy/gomod/matcher.go | 3 --- internal/strategy/gomod/private_fetcher.go | 8 -------- 6 files changed, 1 insertion(+), 18 deletions(-) diff --git a/go.mod b/go.mod index ca8d1a0..ee59a97 100644 --- a/go.mod +++ b/go.mod @@ -9,6 +9,7 @@ require ( github.com/lmittmann/tint v1.1.2 github.com/minio/minio-go/v7 v7.0.97 go.etcd.io/bbolt v1.4.3 + golang.org/x/mod v0.31.0 ) require ( @@ -30,7 +31,6 @@ require ( github.com/stretchr/testify v1.11.1 // indirect github.com/tinylib/msgp v1.3.0 // indirect golang.org/x/crypto v0.44.0 // indirect - golang.org/x/mod v0.31.0 // indirect golang.org/x/net v0.47.0 // indirect golang.org/x/sys v0.38.0 // indirect golang.org/x/text v0.32.0 // indirect diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index ed2804b..16a6430 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -451,9 +451,6 @@ func (r *Repository) GetUpstreamRefs(ctx context.Context) (map[string]string, er return ParseGitRefs(output), nil } -// HasCommit checks if a specific commit/ref exists in the repository. -// Uses git cat-file -e to efficiently check object existence. -// Thread-safe - acquires read lock. func (r *Repository) HasCommit(ctx context.Context, ref string) bool { r.mu.RLock() defer r.mu.RUnlock() diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index a5cfc88..787bff3 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -68,7 +68,6 @@ func New(ctx context.Context, config Config, scheduler jobscheduler.Scheduler, c return nil, errors.Wrap(err, "create clone manager") } - // Register as shared Manager for use by other strategies (e.g., gomod) gitclone.SetShared(cloneManager) s := &Strategy{ diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index 4a42d38..bc9784d 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -59,9 +59,7 @@ func New(ctx context.Context, config Config, _ jobscheduler.Scheduler, cache cac var fetcher goproxy.Fetcher = publicFetcher - // Configure private repository support if private-paths is specified if len(config.PrivatePaths) > 0 { - // Get shared Manager from git strategy cloneManager := gitclone.GetShared() if cloneManager == nil { return nil, errors.New("private-paths configured but git strategy not initialized - git strategy with mirror-root is required for private module support") diff --git a/internal/strategy/gomod/matcher.go b/internal/strategy/gomod/matcher.go index 3a163f9..eebaa2d 100644 --- a/internal/strategy/gomod/matcher.go +++ b/internal/strategy/gomod/matcher.go @@ -5,17 +5,14 @@ import ( "strings" ) -// ModulePathMatcher matches module paths against patterns. type ModulePathMatcher struct { patterns []string } -// NewModulePathMatcher creates a new matcher with the given patterns. func NewModulePathMatcher(patterns []string) *ModulePathMatcher { return &ModulePathMatcher{patterns: patterns} } -// IsPrivate checks if a module path matches any private pattern. func (m *ModulePathMatcher) IsPrivate(modulePath string) bool { for _, pattern := range m.patterns { matched, err := path.Match(pattern, modulePath) diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 4d14fce..6bda7ae 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -41,7 +41,6 @@ func (p *privateFetcher) Query(ctx context.Context, path, query string) (version return "", time.Time{}, errors.Wrapf(err, "get or create clone for %s", path) } - // Verify repository is ready if repo.State() != gitclone.StateReady { return "", time.Time{}, errors.Errorf("repository %s not cloned yet", gitURL) } @@ -64,7 +63,6 @@ func (p *privateFetcher) List(ctx context.Context, path string) (versions []stri return nil, errors.Wrapf(err, "get or create clone for %s", path) } - // Verify repository is ready if repo.State() != gitclone.StateReady { return nil, errors.Errorf("repository %s not cloned yet", gitURL) } @@ -87,12 +85,10 @@ func (p *privateFetcher) Download(ctx context.Context, path, version string) (in return nil, nil, nil, errors.Wrapf(err, "get or create clone for %s", path) } - // Verify repository is ready if repo.State() != gitclone.StateReady { return nil, nil, nil, errors.Errorf("repository %s not cloned yet", gitURL) } - // Verify the specific version/commit exists if err := p.verifyCommitExists(ctx, repo, version); err != nil { return nil, nil, nil, err } @@ -118,9 +114,6 @@ func (p *privateFetcher) modulePathToGitURL(modulePath string) string { return "https://" + modulePath } -// verifyCommitExists checks if a specific commit exists in the repo. -// Uses HasCommit which acquires a read lock and runs git cat-file -e. -// Returns an error if commit doesn't exist. func (p *privateFetcher) verifyCommitExists(ctx context.Context, repo *gitclone.Repository, ref string) error { if !repo.HasCommit(ctx, ref) { return errors.Errorf("commit %s not found in repository %s", ref, repo.UpstreamURL()) @@ -128,7 +121,6 @@ func (p *privateFetcher) verifyCommitExists(ctx context.Context, repo *gitclone. return nil } -// resolveVersionQuery resolves a version query (like "latest" or "v1.2.3") to a specific version. func (p *privateFetcher) resolveVersionQuery(ctx context.Context, repo *gitclone.Repository, query string) (string, time.Time, error) { if query == "latest" { versions, err := p.listVersions(ctx, repo) From 7d9cf4cc5e59ce68f9024f6d52724b5d84cd0ab4 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Tue, 3 Feb 2026 20:12:16 +1100 Subject: [PATCH 03/12] fix: More cleanup --- internal/gitclone/manager.go | 4 ---- internal/gitclone/manager_test.go | 8 -------- 2 files changed, 12 deletions(-) diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 16a6430..0b79155 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -20,16 +20,12 @@ var ( sharedManagerMu sync.RWMutex ) -// SetShared stores a Manager instance to be shared across strategies. -// This should be called by the git strategy after creating its Manager. func SetShared(m *Manager) { sharedManagerMu.Lock() defer sharedManagerMu.Unlock() sharedManager = m } -// GetShared retrieves the shared Manager instance if one exists. -// Returns nil if no shared Manager has been set. func GetShared() *Manager { sharedManagerMu.RLock() defer sharedManagerMu.RUnlock() diff --git a/internal/gitclone/manager_test.go b/internal/gitclone/manager_test.go index ec60e26..9dc86b7 100644 --- a/internal/gitclone/manager_test.go +++ b/internal/gitclone/manager_test.go @@ -221,20 +221,16 @@ func TestRepository_HasCommit(t *testing.T) { tmpDir := t.TempDir() repoPath := filepath.Join(tmpDir, "test-repo") - // Create a real git repo with a commit assert.NoError(t, os.MkdirAll(repoPath, 0o755)) - // Initialize git repo cmd := exec.Command("git", "-C", repoPath, "init") assert.NoError(t, cmd.Run()) - // Configure git user cmd = exec.Command("git", "-C", repoPath, "config", "user.email", "test@example.com") assert.NoError(t, cmd.Run()) cmd = exec.Command("git", "-C", repoPath, "config", "user.name", "Test User") assert.NoError(t, cmd.Run()) - // Create a file and commit testFile := filepath.Join(repoPath, "test.txt") assert.NoError(t, os.WriteFile(testFile, []byte("test content"), 0o644)) cmd = exec.Command("git", "-C", repoPath, "add", "test.txt") @@ -242,11 +238,9 @@ func TestRepository_HasCommit(t *testing.T) { cmd = exec.Command("git", "-C", repoPath, "commit", "-m", "Initial commit") assert.NoError(t, cmd.Run()) - // Create a tag cmd = exec.Command("git", "-C", repoPath, "tag", "v1.0.0") assert.NoError(t, cmd.Run()) - // Create Repository instance repo := &Repository{ state: StateReady, path: repoPath, @@ -255,11 +249,9 @@ func TestRepository_HasCommit(t *testing.T) { } repo.fetchSem <- struct{}{} - // Test HasCommit with existing ref assert.True(t, repo.HasCommit(ctx, "HEAD")) assert.True(t, repo.HasCommit(ctx, "v1.0.0")) - // Test HasCommit with non-existent ref assert.False(t, repo.HasCommit(ctx, "nonexistent")) assert.False(t, repo.HasCommit(ctx, "v9.9.9")) } From 56692579af4e2de08b1531e80c2b058c0305d009 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 16:04:52 +1100 Subject: [PATCH 04/12] fix: Comment for fetcher behaviour --- internal/strategy/gomod/fetcher.go | 1 + 1 file changed, 1 insertion(+) diff --git a/internal/strategy/gomod/fetcher.go b/internal/strategy/gomod/fetcher.go index a67020b..5ff844c 100644 --- a/internal/strategy/gomod/fetcher.go +++ b/internal/strategy/gomod/fetcher.go @@ -9,6 +9,7 @@ import ( "github.com/goproxy/goproxy" ) +// compositeFetcher routes module requests to either public or private fetchers based on module path patterns. type compositeFetcher struct { publicFetcher goproxy.Fetcher privateFetcher goproxy.Fetcher From c1ba40cd8e029310c8a5a0d531ab10b424191bad Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 16:22:42 +1100 Subject: [PATCH 05/12] fix: Move matcher to fetcher module --- go.mod | 2 +- internal/strategy/gomod/fetcher.go | 27 +++++++++++++---- .../{matcher_test.go => fetcher_test.go} | 12 ++++---- internal/strategy/gomod/matcher.go | 29 ------------------- 4 files changed, 28 insertions(+), 42 deletions(-) rename internal/strategy/gomod/{matcher_test.go => fetcher_test.go} (91%) delete mode 100644 internal/strategy/gomod/matcher.go diff --git a/go.mod b/go.mod index d3bce86..cb0659f 100644 --- a/go.mod +++ b/go.mod @@ -17,6 +17,7 @@ require ( go.opentelemetry.io/otel/metric v1.40.0 go.opentelemetry.io/otel/sdk v1.40.0 go.opentelemetry.io/otel/sdk/metric v1.40.0 + golang.org/x/mod v0.31.0 ) require ( @@ -50,7 +51,6 @@ require ( go.opentelemetry.io/proto/otlp v1.9.0 // indirect go.yaml.in/yaml/v2 v2.4.2 // indirect golang.org/x/crypto v0.46.0 // indirect - golang.org/x/mod v0.31.0 // indirect golang.org/x/net v0.48.0 // indirect golang.org/x/sys v0.40.0 // indirect golang.org/x/text v0.32.0 // indirect diff --git a/internal/strategy/gomod/fetcher.go b/internal/strategy/gomod/fetcher.go index 5ff844c..557e5b0 100644 --- a/internal/strategy/gomod/fetcher.go +++ b/internal/strategy/gomod/fetcher.go @@ -3,6 +3,8 @@ package gomod import ( "context" "io" + "path" + "strings" "time" "github.com/alecthomas/errors" @@ -13,7 +15,7 @@ import ( type compositeFetcher struct { publicFetcher goproxy.Fetcher privateFetcher goproxy.Fetcher - matcher *ModulePathMatcher + patterns []string } func newCompositeFetcher( @@ -24,12 +26,27 @@ func newCompositeFetcher( return &compositeFetcher{ publicFetcher: publicFetcher, privateFetcher: privateFetcher, - matcher: NewModulePathMatcher(patterns), + patterns: patterns, } } +func (c *compositeFetcher) isPrivate(modulePath string) bool { + for _, pattern := range c.patterns { + matched, err := path.Match(pattern, modulePath) + if err == nil && matched { + return true + } + + if strings.HasPrefix(modulePath, pattern+"/") || modulePath == pattern { + return true + } + } + + return false +} + func (c *compositeFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { - if c.matcher.IsPrivate(path) { + if c.isPrivate(path) { v, tm, err := c.privateFetcher.Query(ctx, path, query) return v, tm, errors.Wrap(err, "private fetcher query") } @@ -38,7 +55,7 @@ func (c *compositeFetcher) Query(ctx context.Context, path, query string) (versi } func (c *compositeFetcher) List(ctx context.Context, path string) (versions []string, err error) { - if c.matcher.IsPrivate(path) { + if c.isPrivate(path) { v, err := c.privateFetcher.List(ctx, path) return v, errors.Wrap(err, "private fetcher list") } @@ -47,7 +64,7 @@ func (c *compositeFetcher) List(ctx context.Context, path string) (versions []st } func (c *compositeFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { - if c.matcher.IsPrivate(path) { + if c.isPrivate(path) { i, m, z, err := c.privateFetcher.Download(ctx, path, version) return i, m, z, errors.Wrap(err, "private fetcher download") } diff --git a/internal/strategy/gomod/matcher_test.go b/internal/strategy/gomod/fetcher_test.go similarity index 91% rename from internal/strategy/gomod/matcher_test.go rename to internal/strategy/gomod/fetcher_test.go index d1ce7b0..8cfc82c 100644 --- a/internal/strategy/gomod/matcher_test.go +++ b/internal/strategy/gomod/fetcher_test.go @@ -1,12 +1,10 @@ -package gomod_test +package gomod import ( "testing" - - "github.com/block/cachew/internal/strategy/gomod" ) -func TestModulePathMatcher(t *testing.T) { +func TestCompositeFetcher_isPrivate(t *testing.T) { tests := []struct { name string patterns []string @@ -113,10 +111,10 @@ func TestModulePathMatcher(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - matcher := gomod.NewModulePathMatcher(tt.patterns) - got := matcher.IsPrivate(tt.modulePath) + fetcher := newCompositeFetcher(nil, nil, tt.patterns) + got := fetcher.isPrivate(tt.modulePath) if got != tt.want { - t.Errorf("IsPrivate() = %v, want %v", got, tt.want) + t.Errorf("isPrivate() = %v, want %v", got, tt.want) } }) } diff --git a/internal/strategy/gomod/matcher.go b/internal/strategy/gomod/matcher.go deleted file mode 100644 index eebaa2d..0000000 --- a/internal/strategy/gomod/matcher.go +++ /dev/null @@ -1,29 +0,0 @@ -package gomod - -import ( - "path" - "strings" -) - -type ModulePathMatcher struct { - patterns []string -} - -func NewModulePathMatcher(patterns []string) *ModulePathMatcher { - return &ModulePathMatcher{patterns: patterns} -} - -func (m *ModulePathMatcher) IsPrivate(modulePath string) bool { - for _, pattern := range m.patterns { - matched, err := path.Match(pattern, modulePath) - if err == nil && matched { - return true - } - - if strings.HasPrefix(modulePath, pattern+"/") || modulePath == pattern { - return true - } - } - - return false -} From ea3cbd8b81f9f972eea9fed353d920d2a463db40 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 16:31:01 +1100 Subject: [PATCH 06/12] fix: Linting issues --- internal/strategy/gomod/fetcher.go | 24 ++++++++++++------------ internal/strategy/gomod/fetcher_test.go | 10 ++++++---- internal/strategy/gomod/gomod.go | 2 +- 3 files changed, 19 insertions(+), 17 deletions(-) diff --git a/internal/strategy/gomod/fetcher.go b/internal/strategy/gomod/fetcher.go index 557e5b0..c3d98c8 100644 --- a/internal/strategy/gomod/fetcher.go +++ b/internal/strategy/gomod/fetcher.go @@ -11,26 +11,26 @@ import ( "github.com/goproxy/goproxy" ) -// compositeFetcher routes module requests to either public or private fetchers based on module path patterns. -type compositeFetcher struct { +// CompositeFetcher routes module requests to either public or private fetchers based on module path patterns. +type CompositeFetcher struct { publicFetcher goproxy.Fetcher privateFetcher goproxy.Fetcher patterns []string } -func newCompositeFetcher( +func NewCompositeFetcher( publicFetcher goproxy.Fetcher, privateFetcher goproxy.Fetcher, patterns []string, -) *compositeFetcher { - return &compositeFetcher{ +) *CompositeFetcher { + return &CompositeFetcher{ publicFetcher: publicFetcher, privateFetcher: privateFetcher, patterns: patterns, } } -func (c *compositeFetcher) isPrivate(modulePath string) bool { +func (c *CompositeFetcher) IsPrivate(modulePath string) bool { for _, pattern := range c.patterns { matched, err := path.Match(pattern, modulePath) if err == nil && matched { @@ -45,8 +45,8 @@ func (c *compositeFetcher) isPrivate(modulePath string) bool { return false } -func (c *compositeFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { - if c.isPrivate(path) { +func (c *CompositeFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { + if c.IsPrivate(path) { v, tm, err := c.privateFetcher.Query(ctx, path, query) return v, tm, errors.Wrap(err, "private fetcher query") } @@ -54,8 +54,8 @@ func (c *compositeFetcher) Query(ctx context.Context, path, query string) (versi return v, tm, errors.Wrap(err, "public fetcher query") } -func (c *compositeFetcher) List(ctx context.Context, path string) (versions []string, err error) { - if c.isPrivate(path) { +func (c *CompositeFetcher) List(ctx context.Context, path string) (versions []string, err error) { + if c.IsPrivate(path) { v, err := c.privateFetcher.List(ctx, path) return v, errors.Wrap(err, "private fetcher list") } @@ -63,8 +63,8 @@ func (c *compositeFetcher) List(ctx context.Context, path string) (versions []st return v, errors.Wrap(err, "public fetcher list") } -func (c *compositeFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { - if c.isPrivate(path) { +func (c *CompositeFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { + if c.IsPrivate(path) { i, m, z, err := c.privateFetcher.Download(ctx, path, version) return i, m, z, errors.Wrap(err, "private fetcher download") } diff --git a/internal/strategy/gomod/fetcher_test.go b/internal/strategy/gomod/fetcher_test.go index 8cfc82c..7f365cb 100644 --- a/internal/strategy/gomod/fetcher_test.go +++ b/internal/strategy/gomod/fetcher_test.go @@ -1,7 +1,9 @@ -package gomod +package gomod_test import ( "testing" + + "github.com/block/cachew/internal/strategy/gomod" ) func TestCompositeFetcher_isPrivate(t *testing.T) { @@ -111,10 +113,10 @@ func TestCompositeFetcher_isPrivate(t *testing.T) { for _, tt := range tests { t.Run(tt.name, func(t *testing.T) { - fetcher := newCompositeFetcher(nil, nil, tt.patterns) - got := fetcher.isPrivate(tt.modulePath) + fetcher := gomod.NewCompositeFetcher(nil, nil, tt.patterns) + got := fetcher.IsPrivate(tt.modulePath) if got != tt.want { - t.Errorf("isPrivate() = %v, want %v", got, tt.want) + t.Errorf("IsPrivate() = %v, want %v", got, tt.want) } }) } diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index bc9784d..8e730fc 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -67,7 +67,7 @@ func New(ctx context.Context, config Config, _ jobscheduler.Scheduler, cache cac s.cloneManager = cloneManager privateFetcher := newPrivateFetcher(s, cloneManager) - fetcher = newCompositeFetcher(publicFetcher, privateFetcher, config.PrivatePaths) + fetcher = NewCompositeFetcher(publicFetcher, privateFetcher, config.PrivatePaths) s.logger.InfoContext(ctx, "Configured private module support", slog.Any("private_paths", config.PrivatePaths)) From d0ab47bbed9a56e211752f4f33ddd47882295f57 Mon Sep 17 00:00:00 2001 From: js-murph <7096301+js-murph@users.noreply.github.com> Date: Thu, 5 Feb 2026 19:40:32 +1100 Subject: [PATCH 07/12] Update internal/strategy/gomod/gomod.go Co-authored-by: Alec Thomas --- internal/strategy/gomod/gomod.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index 8e730fc..5f30410 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -70,7 +70,7 @@ func New(ctx context.Context, config Config, _ jobscheduler.Scheduler, cache cac fetcher = NewCompositeFetcher(publicFetcher, privateFetcher, config.PrivatePaths) s.logger.InfoContext(ctx, "Configured private module support", - slog.Any("private_paths", config.PrivatePaths)) + slog.Any("private-paths", config.PrivatePaths)) } s.goproxy = &goproxy.Goproxy{ From e2b01e0f90c5289d93a5dcac2683187e4410f6e7 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 19:56:27 +1100 Subject: [PATCH 08/12] fix: getting logger from a different object --- internal/strategy/gomod/gomod.go | 2 +- internal/strategy/gomod/private_fetcher.go | 12 ++++++------ 2 files changed, 7 insertions(+), 7 deletions(-) diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index 8e730fc..fa3c4eb 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -66,7 +66,7 @@ func New(ctx context.Context, config Config, _ jobscheduler.Scheduler, cache cac } s.cloneManager = cloneManager - privateFetcher := newPrivateFetcher(s, cloneManager) + privateFetcher := newPrivateFetcher(s.logger, cloneManager) fetcher = NewCompositeFetcher(publicFetcher, privateFetcher, config.PrivatePaths) s.logger.InfoContext(ctx, "Configured private module support", diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 6bda7ae..198a866 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -19,19 +19,19 @@ import ( ) type privateFetcher struct { - gomod *Strategy + logger *slog.Logger cloneManager *gitclone.Manager } -func newPrivateFetcher(gomod *Strategy, cloneManager *gitclone.Manager) *privateFetcher { +func newPrivateFetcher(logger *slog.Logger, cloneManager *gitclone.Manager) *privateFetcher { return &privateFetcher{ - gomod: gomod, + logger: logger, cloneManager: cloneManager, } } func (p *privateFetcher) Query(ctx context.Context, path, query string) (version string, t time.Time, err error) { - logger := p.gomod.logger.With(slog.String("module", path), slog.String("query", query)) + logger := p.logger.With(slog.String("module", path), slog.String("query", query)) logger.DebugContext(ctx, "Private fetcher: Query") gitURL := p.modulePathToGitURL(path) @@ -54,7 +54,7 @@ func (p *privateFetcher) Query(ctx context.Context, path, query string) (version } func (p *privateFetcher) List(ctx context.Context, path string) (versions []string, err error) { - logger := p.gomod.logger.With(slog.String("module", path)) + logger := p.logger.With(slog.String("module", path)) logger.DebugContext(ctx, "Private fetcher: List") gitURL := p.modulePathToGitURL(path) @@ -76,7 +76,7 @@ func (p *privateFetcher) List(ctx context.Context, path string) (versions []stri } func (p *privateFetcher) Download(ctx context.Context, path, version string) (info, mod, zip io.ReadSeekCloser, err error) { - logger := p.gomod.logger.With(slog.String("module", path), slog.String("version", version)) + logger := p.logger.With(slog.String("module", path), slog.String("version", version)) logger.DebugContext(ctx, "Private fetcher: Download") gitURL := p.modulePathToGitURL(path) From f351f16c6a5573835eaa2b45911c1f60323c92e8 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 20:01:33 +1100 Subject: [PATCH 09/12] fix: Use json marshalling instead of constructing as a string --- internal/strategy/gomod/private_fetcher.go | 19 +++++++++++++++++-- 1 file changed, 17 insertions(+), 2 deletions(-) diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 198a866..0aa64f3 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -3,6 +3,7 @@ package gomod import ( "bytes" "context" + "encoding/json" "fmt" "io" "io/fs" @@ -23,6 +24,11 @@ type privateFetcher struct { cloneManager *gitclone.Manager } +type moduleInfo struct { + Version string + Time string +} + func newPrivateFetcher(logger *slog.Logger, cloneManager *gitclone.Manager) *privateFetcher { return &privateFetcher{ logger: logger, @@ -228,8 +234,17 @@ func (p *privateFetcher) generateInfo(ctx context.Context, repo *gitclone.Reposi return nil, err } - info := fmt.Sprintf(`{"Version":"%s","Time":"%s"}`, version, commitTime.Format(time.RFC3339)) - return newReadSeekCloser(bytes.NewReader([]byte(info))), nil + info := moduleInfo{ + Version: version, + Time: commitTime.Format(time.RFC3339), + } + + data, err := json.Marshal(info) + if err != nil { + return nil, errors.Wrap(err, "marshal module info") + } + + return newReadSeekCloser(bytes.NewReader(data)), nil } func (p *privateFetcher) generateMod(ctx context.Context, repo *gitclone.Repository, modulePath, version string) io.ReadSeekCloser { From af37b6ff894063c3535df9f1e6ce6eae45c96969 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 20:06:22 +1100 Subject: [PATCH 10/12] fix: linting --- internal/strategy/gomod/private_fetcher.go | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 0aa64f3..4846f94 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -25,8 +25,8 @@ type privateFetcher struct { } type moduleInfo struct { - Version string - Time string + Version string `json:"Version"` + Time string `json:"Time"` } func newPrivateFetcher(logger *slog.Logger, cloneManager *gitclone.Manager) *privateFetcher { From a96270136bd1f6e60b3ec3b203e3baf6537d2d8a Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 21:38:10 +1100 Subject: [PATCH 11/12] fix: Missing imports --- internal/strategy/gomod/gomod.go | 4 ---- 1 file changed, 4 deletions(-) diff --git a/internal/strategy/gomod/gomod.go b/internal/strategy/gomod/gomod.go index 314b5b4..36ff279 100644 --- a/internal/strategy/gomod/gomod.go +++ b/internal/strategy/gomod/gomod.go @@ -11,11 +11,7 @@ import ( "github.com/goproxy/goproxy" "github.com/block/cachew/internal/cache" -<<<<<<< johnm/gomod-private-attempt-three "github.com/block/cachew/internal/gitclone" - "github.com/block/cachew/internal/jobscheduler" -======= ->>>>>>> main "github.com/block/cachew/internal/logging" "github.com/block/cachew/internal/strategy" ) From e3dc51e78054ec42f7260117de46ba12afb6d2a4 Mon Sep 17 00:00:00 2001 From: John Murphy Date: Thu, 5 Feb 2026 21:59:43 +1100 Subject: [PATCH 12/12] fix: Bug with empty cache --- internal/gitclone/manager.go | 4 ++ internal/strategy/gomod/private_fetcher.go | 43 +++++++++++++++++++--- 2 files changed, 41 insertions(+), 6 deletions(-) diff --git a/internal/gitclone/manager.go b/internal/gitclone/manager.go index 0b79155..d0b8ac7 100644 --- a/internal/gitclone/manager.go +++ b/internal/gitclone/manager.go @@ -149,6 +149,10 @@ func (m *Manager) Get(upstreamURL string) *Repository { return m.clones[upstreamURL] } +func (m *Manager) Config() Config { + return m.config +} + func (m *Manager) DiscoverExisting(_ context.Context) error { err := filepath.Walk(m.config.RootDir, func(path string, info os.FileInfo, err error) error { if err != nil { diff --git a/internal/strategy/gomod/private_fetcher.go b/internal/strategy/gomod/private_fetcher.go index 4846f94..c89f867 100644 --- a/internal/strategy/gomod/private_fetcher.go +++ b/internal/strategy/gomod/private_fetcher.go @@ -47,8 +47,8 @@ func (p *privateFetcher) Query(ctx context.Context, path, query string) (version return "", time.Time{}, errors.Wrapf(err, "get or create clone for %s", path) } - if repo.State() != gitclone.StateReady { - return "", time.Time{}, errors.Errorf("repository %s not cloned yet", gitURL) + if err := p.ensureReady(ctx, repo); err != nil { + return "", time.Time{}, errors.Wrapf(err, "ensure repository ready for %s", gitURL) } resolvedVersion, commitTime, err := p.resolveVersionQuery(ctx, repo, query) @@ -69,8 +69,8 @@ func (p *privateFetcher) List(ctx context.Context, path string) (versions []stri return nil, errors.Wrapf(err, "get or create clone for %s", path) } - if repo.State() != gitclone.StateReady { - return nil, errors.Errorf("repository %s not cloned yet", gitURL) + if err := p.ensureReady(ctx, repo); err != nil { + return nil, errors.Wrapf(err, "ensure repository ready for %s", gitURL) } versions, err = p.listVersions(ctx, repo) @@ -91,8 +91,8 @@ func (p *privateFetcher) Download(ctx context.Context, path, version string) (in return nil, nil, nil, errors.Wrapf(err, "get or create clone for %s", path) } - if repo.State() != gitclone.StateReady { - return nil, nil, nil, errors.Errorf("repository %s not cloned yet", gitURL) + if err := p.ensureReady(ctx, repo); err != nil { + return nil, nil, nil, errors.Wrapf(err, "ensure repository ready for %s", gitURL) } if err := p.verifyCommitExists(ctx, repo, version); err != nil { @@ -116,6 +116,37 @@ func (p *privateFetcher) Download(ctx context.Context, path, version string) (in return infoReader, modReader, zipReader, nil } +func (p *privateFetcher) ensureReady(ctx context.Context, repo *gitclone.Repository) error { + if repo.State() == gitclone.StateReady { + return nil + } + + config := p.cloneManager.Config() + if err := repo.Clone(ctx, config); err != nil { + return errors.Wrap(err, "clone repository") + } + + ticker := time.NewTicker(100 * time.Millisecond) + defer ticker.Stop() + + timeout := time.After(30 * time.Minute) // reasonable timeout for cloning + + for { + if repo.State() == gitclone.StateReady { + return nil + } + + select { + case <-ticker.C: + // Continue polling + case <-timeout: + return errors.Errorf("timeout waiting for repository %s to be ready", repo.UpstreamURL()) + case <-ctx.Done(): + return errors.Wrap(ctx.Err(), "context cancelled while waiting for clone") + } + } +} + func (p *privateFetcher) modulePathToGitURL(modulePath string) string { return "https://" + modulePath }