From a8d4b58f7fba5a0915115de937202e0baf33b625 Mon Sep 17 00:00:00 2001 From: Alec Thomas Date: Mon, 19 Jan 2026 11:25:59 +1100 Subject: [PATCH] docs: cleanup a lot of pointless comments --- AGENTS.md | 4 ++ internal/strategy/git/backend.go | 47 ++++------------------ internal/strategy/git/bundle.go | 15 +------ internal/strategy/git/command.go | 22 +++-------- internal/strategy/git/git.go | 67 +++----------------------------- internal/strategy/git/proxy.go | 1 - 6 files changed, 24 insertions(+), 132 deletions(-) diff --git a/AGENTS.md b/AGENTS.md index 525fee8..fdacc57 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -15,3 +15,7 @@ cache and restore data. Strategies are not limited to using the Cache though; fo keep a local bare checkout of upstream Git repositories and serve packs from the repo directly. The codebase uses Hermit to manage toolchains. It is written in Go, and uses Just for running common tasks. + +Only add comments for relatively large blocks of code, 20+ lines or more, and ONLY if it is not obvious what the code is +doing. ALWAYS add Go-style documentation comments for public variables/types/functions. If you do add comments, the +comments should explain WHY something is happening, not WHAT is happening. diff --git a/internal/strategy/git/backend.go b/internal/strategy/git/backend.go index 07091fd..82325a0 100644 --- a/internal/strategy/git/backend.go +++ b/internal/strategy/git/backend.go @@ -20,7 +20,6 @@ import ( "github.com/block/cachew/internal/logging" ) -// serveFromBackend serves a Git request using git http-backend. func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *clone) { ctx := r.Context() logger := logging.FromContext(ctx) @@ -37,12 +36,10 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *c return } - // Build the path that git http-backend expects host := r.PathValue("host") pathValue := r.PathValue("path") - // For regular clones, we need to insert /.git before the git protocol paths - // Find where the git operation starts (e.g., /info/refs, /git-upload-pack) + // Insert /.git before the git protocol paths to match the filesystem layout var gitOperation string var repoPathWithSuffix string @@ -54,10 +51,7 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *c } } - // Remove .git suffix from repo path for the filesystem path repoPath := strings.TrimSuffix(repoPathWithSuffix, ".git") - - // Construct backend path with .git directory: /host/repo/.git/info/refs backendPath := "/" + host + "/" + repoPath + "/.git" + gitOperation logger.DebugContext(r.Context(), "Serving with git http-backend", @@ -65,7 +59,6 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *c slog.String("backend_path", backendPath), slog.String("clone_path", c.path)) - // Capture stderr from git http-backend to log errors var stderrBuf bytes.Buffer handler := &cgi.Handler{ @@ -79,13 +72,11 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *c }, } - // Modify request for http-backend r2 := r.Clone(r.Context()) r2.URL.Path = backendPath handler.ServeHTTP(w, r2) - // Log stderr if there was any output (indicates an error) if stderrBuf.Len() > 0 { logger.ErrorContext(r.Context(), "git http-backend error", slog.String("stderr", stderrBuf.String()), @@ -93,7 +84,6 @@ func (s *Strategy) serveFromBackend(w http.ResponseWriter, r *http.Request, c *c } } -// executeClone performs a git clone operation. func (s *Strategy) executeClone(ctx context.Context, c *clone) error { logger := logging.FromContext(ctx) @@ -101,8 +91,8 @@ func (s *Strategy) executeClone(ctx context.Context, c *clone) error { return errors.Wrap(err, "create clone directory") } - // #nosec G204 - c.upstreamURL and c.path are controlled by us // Configure git for large repositories to avoid network buffer issues + // #nosec G204 - c.upstreamURL and c.path are controlled by us args := []string{"clone"} if s.config.CloneDepth > 0 { args = append(args, "--depth", strconv.Itoa(s.config.CloneDepth)) @@ -124,9 +114,7 @@ func (s *Strategy) executeClone(ctx context.Context, c *clone) error { return errors.Wrap(err, "git clone") } - // Configure remote to fetch all branches, not just the default branch - // git clone sets fetch = +refs/heads/master:refs/remotes/origin/master by default - // We need to change it to fetch all branches + // git clone only sets up fetching for the default branch, change it to fetch all branches // #nosec G204 - c.path is controlled by us cmd = exec.CommandContext(ctx, "git", "-C", c.path, "config", "remote.origin.fetch", "+refs/heads/*:refs/remotes/origin/*") output, err = cmd.CombinedOutput() @@ -137,7 +125,6 @@ func (s *Strategy) executeClone(ctx context.Context, c *clone) error { return errors.Wrap(err, "configure fetch refspec") } - // Fetch all branches now that the refspec is configured cmd, err = gitCommand(ctx, c.upstreamURL, "-C", c.path, "-c", "http.postBuffer=524288000", "-c", "http.lowSpeedLimit=1000", @@ -157,26 +144,20 @@ func (s *Strategy) executeClone(ctx context.Context, c *clone) error { return nil } -// executeFetch performs a git remote update operation. func (s *Strategy) executeFetch(ctx context.Context, c *clone) error { logger := logging.FromContext(ctx) - // Try to acquire the semaphore select { case <-c.fetchSem: - // We acquired the semaphore, perform the fetch defer func() { - // Release the semaphore c.fetchSem <- struct{}{} }() case <-ctx.Done(): return errors.Wrap(ctx.Err(), "context cancelled before acquiring fetch semaphore") default: - // Semaphore is held by another goroutine, wait for it logger.DebugContext(ctx, "Fetch already in progress, waiting") select { case <-c.fetchSem: - // Fetch completed by another goroutine, release and return c.fetchSem <- struct{}{} return nil case <-ctx.Done(): @@ -184,9 +165,8 @@ func (s *Strategy) executeFetch(ctx context.Context, c *clone) error { } } - // #nosec G204 - c.path is controlled by us // Configure git for large repositories to avoid network buffer issues - // Use 'remote update' to properly handle ref updates and pruning + // #nosec G204 - c.path is controlled by us cmd, err := gitCommand(ctx, c.upstreamURL, "-C", c.path, "-c", "http.postBuffer=524288000", // 500MB buffer "-c", "http.lowSpeedLimit=1000", // 1KB/s minimum speed @@ -211,12 +191,11 @@ func (s *Strategy) executeFetch(ctx context.Context, c *clone) error { } // ensureRefsUpToDate checks if upstream has refs we don't have and fetches if needed. -// Uses a short-lived cache to avoid excessive ls-remote calls. +// Short-lived cache avoids excessive ls-remote calls. func (s *Strategy) ensureRefsUpToDate(ctx context.Context, c *clone) error { logger := logging.FromContext(ctx) c.mu.Lock() - // Check if we've done a recent ref check if c.refCheckValid && time.Since(c.lastRefCheck) < s.config.RefCheckInterval { c.mu.Unlock() logger.DebugContext(ctx, "Skipping ref check, recently checked", @@ -230,32 +209,25 @@ func (s *Strategy) ensureRefsUpToDate(ctx context.Context, c *clone) error { logger.DebugContext(ctx, "Checking upstream for new refs", slog.String("upstream", c.upstreamURL)) - // Get local refs localRefs, err := s.getLocalRefs(ctx, c) if err != nil { return errors.Wrap(err, "get local refs") } - // Get upstream refs upstreamRefs, err := s.getUpstreamRefs(ctx, c) if err != nil { return errors.Wrap(err, "get upstream refs") } - // Check if upstream has any refs we don't have or refs that have been updated - // Skip peeled refs (refs ending in ^{}) as they're not real refs needsFetch := false for ref, upstreamSHA := range upstreamRefs { - // Skip peeled tag refs like refs/tags/v1.0.0^{} if strings.HasSuffix(ref, "^{}") { continue } - // Only check refs/heads/* from upstream since those are what we fetch - // (GitHub exposes refs/pull/* and other refs we don't fetch) + // Only check refs/heads/* since GitHub exposes refs/pull/* and other refs we don't fetch if !strings.HasPrefix(ref, "refs/heads/") { continue } - // Convert refs/heads/X to refs/remotes/origin/X for local lookup localRef := "refs/remotes/origin/" + strings.TrimPrefix(ref, "refs/heads/") localSHA, exists := localRefs[localRef] if !exists || localSHA != upstreamSHA { @@ -287,11 +259,8 @@ func (s *Strategy) ensureRefsUpToDate(ctx context.Context, c *clone) error { return err } -// getLocalRefs returns a map of ref names to SHAs for the local clone. func (s *Strategy) getLocalRefs(ctx context.Context, c *clone) (map[string]string, error) { // #nosec G204 - c.path is controlled by us - // Use for-each-ref to get all refs including remote refs - // No need for insteadOf protection since this is purely local cmd := exec.CommandContext(ctx, "git", "-C", c.path, "for-each-ref", "--format=%(objectname) %(refname)") output, err := cmd.CombinedOutput() if err != nil { @@ -301,7 +270,6 @@ func (s *Strategy) getLocalRefs(ctx context.Context, c *clone) (map[string]strin return ParseGitRefs(output), nil } -// getUpstreamRefs returns a map of ref names to SHAs for the upstream repository. func (s *Strategy) getUpstreamRefs(ctx context.Context, c *clone) (map[string]string, error) { // #nosec G204 - c.upstreamURL is controlled by us cmd, err := gitCommand(ctx, c.upstreamURL, "ls-remote", c.upstreamURL) @@ -316,8 +284,7 @@ func (s *Strategy) getUpstreamRefs(ctx context.Context, c *clone) (map[string]st return ParseGitRefs(output), nil } -// ParseGitRefs parses the output of git show-ref or git ls-remote. -// Format: . +// ParseGitRefs parses the output of git show-ref or git ls-remote (format: ). func ParseGitRefs(output []byte) map[string]string { refs := make(map[string]string) scanner := bufio.NewScanner(strings.NewReader(string(output))) diff --git a/internal/strategy/git/bundle.go b/internal/strategy/git/bundle.go index c99cb69..faae6d8 100644 --- a/internal/strategy/git/bundle.go +++ b/internal/strategy/git/bundle.go @@ -15,11 +15,9 @@ import ( "github.com/block/cachew/internal/logging" ) -// cloneBundleLoop generates bundles periodically for a single clone. func (s *Strategy) cloneBundleLoop(ctx context.Context, c *clone) { logger := logging.FromContext(ctx) - // Generate bundle immediately on start if one doesn't exist s.generateAndUploadBundleIfMissing(ctx, c) ticker := time.NewTicker(s.config.BundleInterval) @@ -38,23 +36,19 @@ func (s *Strategy) cloneBundleLoop(ctx context.Context, c *clone) { } } -// generateAndUploadBundleIfMissing generates a bundle only if one doesn't exist in cache. func (s *Strategy) generateAndUploadBundleIfMissing(ctx context.Context, c *clone) { logger := logging.FromContext(ctx) - // Check if bundle already exists in cache cacheKey := cache.NewKey(c.upstreamURL + ".bundle") reader, _, err := s.cache.Open(ctx, cacheKey) if err == nil { - // Bundle exists, close and skip generation _ = reader.Close() logger.DebugContext(ctx, "Bundle already exists in cache, skipping generation", slog.String("upstream", c.upstreamURL)) return } - // Only generate if the error is that the bundle doesn't exist if !errors.Is(err, os.ErrNotExist) { logger.ErrorContext(ctx, "Failed to check for existing bundle", slog.String("upstream", c.upstreamURL), @@ -62,11 +56,9 @@ func (s *Strategy) generateAndUploadBundleIfMissing(ctx context.Context, c *clon return } - // Bundle doesn't exist, generate it s.generateAndUploadBundle(ctx, c) } -// generateAndUploadBundle generates a bundle and streams it directly to cache. func (s *Strategy) generateAndUploadBundle(ctx context.Context, c *clone) { logger := logging.FromContext(ctx) @@ -75,7 +67,6 @@ func (s *Strategy) generateAndUploadBundle(ctx context.Context, c *clone) { cacheKey := cache.NewKey(c.upstreamURL + ".bundle") - // Create cache writer headers := textproto.MIMEHeader{ "Content-Type": []string{"application/x-git-bundle"}, } @@ -89,9 +80,8 @@ func (s *Strategy) generateAndUploadBundle(ctx context.Context, c *clone) { } defer w.Close() - // Stream bundle directly to cache - // #nosec G204 - c.path is controlled by us // Use --branches --remotes to include all branches but exclude tags (which can be massive) + // #nosec G204 - c.path is controlled by us args := []string{"-C", c.path, "bundle", "create", "-", "--branches", "--remotes"} cmd, err := gitCommand(ctx, "", args...) if err != nil { @@ -102,7 +92,6 @@ func (s *Strategy) generateAndUploadBundle(ctx context.Context, c *clone) { } cmd.Stdout = w - // Capture stderr for error reporting stderrPipe, err := cmd.StderrPipe() if err != nil { logger.ErrorContext(ctx, "Failed to create stderr pipe", @@ -122,7 +111,7 @@ func (s *Strategy) generateAndUploadBundle(ctx context.Context, c *clone) { return } - stderr, _ := io.ReadAll(stderrPipe) //nolint:errcheck // Only used for logging + stderr, _ := io.ReadAll(stderrPipe) //nolint:errcheck if err := cmd.Wait(); err != nil { logger.ErrorContext(ctx, "Failed to generate bundle", diff --git a/internal/strategy/git/command.go b/internal/strategy/git/command.go index f172a54..bbdd6a6 100644 --- a/internal/strategy/git/command.go +++ b/internal/strategy/git/command.go @@ -9,18 +9,14 @@ import ( "github.com/alecthomas/errors" ) -// gitCommand creates a git command with insteadOf URL rewriting disabled for the given URL. -// This prevents git config rules like "url.X.insteadOf=Y" from rewriting the specific URL -// to point back through the proxy, which would cause infinite loops. -// Other insteadOf rules and all auth configuration are preserved. +// gitCommand creates a git command with insteadOf URL rewriting disabled for the given URL +// to prevent infinite loops where git config rules rewrite URLs to point back through the proxy. func gitCommand(ctx context.Context, url string, args ...string) (*exec.Cmd, error) { - // Query for insteadOf rules that would affect this URL and build -c flags to disable them configArgs, err := getInsteadOfDisableArgsForURL(ctx, url) if err != nil { return nil, errors.Wrap(err, "get insteadOf disable args") } - // Prepend disable args to the git command arguments var allArgs []string if len(configArgs) > 0 { allArgs = append(allArgs, configArgs...) @@ -31,37 +27,29 @@ func gitCommand(ctx context.Context, url string, args ...string) (*exec.Cmd, err return cmd, nil } -// getInsteadOfDisableArgsForURL queries git config for insteadOf rules that would affect -// the given URL and returns arguments to disable only those specific rules. +// getInsteadOfDisableArgsForURL returns arguments to disable insteadOf rules that would affect the given URL. func getInsteadOfDisableArgsForURL(ctx context.Context, targetURL string) ([]string, error) { if targetURL == "" { return nil, nil } - // Query git config for all url.*.insteadOf and url.*.pushInsteadOf settings cmd := exec.CommandContext(ctx, "git", "config", "--get-regexp", "^url\\..*\\.(insteadof|pushinsteadof)$") output, err := cmd.CombinedOutput() if err != nil { - // No insteadOf rules found (exit code 1) is expected and not an error - // Return empty args to continue without disabling any rules - return []string{}, nil //nolint:nilerr // Exit code 1 is expected when no rules exist + // Exit code 1 when no insteadOf rules exist is expected, not an error + return []string{}, nil //nolint:nilerr } - // Parse output and check which rules would match our URL - // Output format: url..insteadof or url..pushinsteadof var args []string scanner := bufio.NewScanner(strings.NewReader(string(output))) for scanner.Scan() { line := scanner.Text() - // Split into config key and value parts := strings.Fields(line) if len(parts) >= 2 { configKey := parts[0] pattern := parts[1] - // Check if our target URL would match this insteadOf pattern if strings.HasPrefix(targetURL, pattern) { - // This rule would affect our URL, so disable it args = append(args, "-c", configKey+"=") } } diff --git a/internal/strategy/git/git.go b/internal/strategy/git/git.go index bf1f90d..f4bbcd3 100644 --- a/internal/strategy/git/git.go +++ b/internal/strategy/git/git.go @@ -25,7 +25,6 @@ func init() { strategy.Register("git", New) } -// Config for the Git strategy. type Config struct { MirrorRoot string `hcl:"mirror-root" help:"Directory to store git clones." required:""` FetchInterval time.Duration `hcl:"fetch-interval,optional" help:"How often to fetch from upstream in minutes." default:"15m"` @@ -34,16 +33,14 @@ type Config struct { CloneDepth int `hcl:"clone-depth,optional" help:"Depth for shallow clones. 0 means full clone." default:"0"` } -// cloneState represents the current state of a clone. type cloneState int const ( - stateEmpty cloneState = iota // Clone doesn't exist yet - stateCloning // Clone is in progress - stateReady // Clone is ready to serve + stateEmpty cloneState = iota + stateCloning + stateReady ) -// clone represents a checked out clone of an upstream repository. type clone struct { mu sync.RWMutex state cloneState @@ -52,10 +49,9 @@ type clone struct { lastFetch time.Time lastRefCheck time.Time refCheckValid bool - fetchSem chan struct{} // Semaphore to coordinate fetch operations + fetchSem chan struct{} } -// Strategy implements a protocol-aware Git caching proxy. type Strategy struct { config Config cache cache.Cache @@ -63,10 +59,9 @@ type Strategy struct { clonesMu sync.RWMutex httpClient *http.Client proxy *httputil.ReverseProxy - ctx context.Context // Strategy lifecycle context + ctx context.Context } -// New creates a new Git caching strategy. func New(ctx context.Context, config Config, cache cache.Cache, mux strategy.Mux) (*Strategy, error) { logger := logging.FromContext(ctx) @@ -94,7 +89,6 @@ func New(ctx context.Context, config Config, cache cache.Cache, mux strategy.Mux ctx: ctx, } - // Scan for existing clones on disk and start bundle loops for them if err := s.discoverExistingClones(ctx); err != nil { logger.WarnContext(ctx, "Failed to discover existing clones", slog.String("error", err.Error())) @@ -130,7 +124,6 @@ var _ strategy.Strategy = (*Strategy)(nil) func (s *Strategy) String() string { return "git" } -// handleRequest routes Git HTTP requests based on operation type. func (s *Strategy) handleRequest(w http.ResponseWriter, r *http.Request) { ctx := r.Context() logger := logging.FromContext(ctx) @@ -143,24 +136,20 @@ func (s *Strategy) handleRequest(w http.ResponseWriter, r *http.Request) { slog.String("host", host), slog.String("path", pathValue)) - // Check if this is a bundle request if strings.HasSuffix(pathValue, "/bundle") { s.handleBundleRequest(w, r, host, pathValue) return } - // Determine the service type from query param or path service := r.URL.Query().Get("service") isReceivePack := service == "git-receive-pack" || strings.HasSuffix(pathValue, "/git-receive-pack") - // Write operations always forward to upstream if isReceivePack { logger.DebugContext(ctx, "Forwarding write operation to upstream") s.forwardToUpstream(w, r, host, pathValue) return } - // Read operations: serve from local clone if ready, otherwise forward repoPath := ExtractRepoPath(pathValue) upstreamURL := "https://" + host + "/" + repoPath @@ -170,38 +159,30 @@ func (s *Strategy) handleRequest(w http.ResponseWriter, r *http.Request) { state := c.state c.mu.RUnlock() - // Check if this is an info/refs request (ref discovery) isInfoRefs := strings.HasSuffix(pathValue, "/info/refs") switch state { case stateReady: - // For info/refs requests, ensure we have the latest refs from upstream if isInfoRefs { if err := s.ensureRefsUpToDate(ctx, c); err != nil { logger.WarnContext(ctx, "Failed to ensure refs up to date", slog.String("error", err.Error())) - // Continue serving even if ref check fails } } - // Also do background fetch if interval has passed s.maybeBackgroundFetch(ctx, c) s.serveFromBackend(w, r, c) case stateCloning: - // Clone in progress, forward to upstream logger.DebugContext(ctx, "Clone in progress, forwarding to upstream") s.forwardToUpstream(w, r, host, pathValue) case stateEmpty: - // Start cloning in background, forward this request to upstream logger.DebugContext(ctx, "Starting background clone, forwarding to upstream") go s.startClone(context.WithoutCancel(ctx), c) s.forwardToUpstream(w, r, host, pathValue) } } -// ExtractRepoPath extracts the repository path from the request path, -// removing git-specific suffixes. func ExtractRepoPath(pathValue string) string { repoPath := pathValue repoPath = strings.TrimSuffix(repoPath, "/info/refs") @@ -211,7 +192,6 @@ func ExtractRepoPath(pathValue string) string { return repoPath } -// handleBundleRequest serves a git bundle from the cache. func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, host, pathValue string) { ctx := r.Context() logger := logging.FromContext(ctx) @@ -220,17 +200,11 @@ func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, h slog.String("host", host), slog.String("path", pathValue)) - // Remove /bundle suffix to get repo path pathValue = strings.TrimSuffix(pathValue, "/bundle") - - // Extract repo path and construct upstream URL repoPath := ExtractRepoPath(pathValue) upstreamURL := "https://" + host + "/" + repoPath - - // Generate cache key cacheKey := cache.NewKey(upstreamURL + ".bundle") - // Open bundle from cache reader, headers, err := s.cache.Open(ctx, cacheKey) if err != nil { if errors.Is(err, os.ErrNotExist) { @@ -247,14 +221,12 @@ func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, h } defer reader.Close() - // Set headers for key, values := range headers { for _, value := range values { w.Header().Add(key, value) } } - // Stream bundle to client _, err = io.Copy(w, reader) if err != nil { logger.ErrorContext(ctx, "Failed to stream bundle", @@ -263,7 +235,6 @@ func (s *Strategy) handleBundleRequest(w http.ResponseWriter, r *http.Request, h } } -// getOrCreateClone returns an existing clone or creates a new one in empty state. func (s *Strategy) getOrCreateClone(ctx context.Context, upstreamURL string) *clone { s.clonesMu.RLock() c, exists := s.clones[upstreamURL] @@ -276,12 +247,10 @@ func (s *Strategy) getOrCreateClone(ctx context.Context, upstreamURL string) *cl s.clonesMu.Lock() defer s.clonesMu.Unlock() - // Double-check after acquiring write lock if c, exists = s.clones[upstreamURL]; exists { return c } - // Create new clone entry clonePath := s.clonePathForURL(upstreamURL) c = &clone{ @@ -291,76 +260,60 @@ func (s *Strategy) getOrCreateClone(ctx context.Context, upstreamURL string) *cl fetchSem: make(chan struct{}, 1), } - // Check if clone already exists on disk (from previous run) - // Verify it has a .git directory to ensure it's a valid clone gitDir := filepath.Join(clonePath, ".git") if _, err := os.Stat(gitDir); err == nil { c.state = stateReady logging.FromContext(ctx).DebugContext(ctx, "Found existing clone on disk", slog.String("path", clonePath)) - // Start bundle generation loop for existing clone if s.config.BundleInterval > 0 { go s.cloneBundleLoop(s.ctx, c) } } - // Initialize semaphore as available c.fetchSem <- struct{}{} s.clones[upstreamURL] = c return c } -// clonePathForURL returns the filesystem path for a clone given its upstream URL. func (s *Strategy) clonePathForURL(upstreamURL string) string { parsed, err := url.Parse(upstreamURL) if err != nil { - // Fallback to simple hash if URL parsing fails return filepath.Join(s.config.MirrorRoot, "unknown") } - // Create path: {mirror_root}/{host}/{path} repoPath := strings.TrimSuffix(parsed.Path, ".git") return filepath.Join(s.config.MirrorRoot, parsed.Host, repoPath) } -// discoverExistingClones scans the mirror root for existing clones and starts bundle loops. func (s *Strategy) discoverExistingClones(ctx context.Context) error { logger := logging.FromContext(ctx) - // Walk the mirror root directory err := filepath.Walk(s.config.MirrorRoot, func(path string, info os.FileInfo, err error) error { if err != nil { return err } - // Skip non-directories if !info.IsDir() { return nil } - // Check if this directory is a git repository by looking for .git directory or HEAD file gitDir := filepath.Join(path, ".git") headPath := filepath.Join(path, ".git", "HEAD") if _, statErr := os.Stat(gitDir); statErr != nil { - // Skip if .git doesn't exist (not a git repo) if errors.Is(statErr, os.ErrNotExist) { return nil } - // Return other errors return errors.Wrap(statErr, "stat .git directory") } if _, statErr := os.Stat(headPath); statErr != nil { - // Skip if HEAD doesn't exist (not a valid git repo) if errors.Is(statErr, os.ErrNotExist) { return nil } - // Return other errors return errors.Wrap(statErr, "stat HEAD file") } - // Extract upstream URL from path relPath, err := filepath.Rel(s.config.MirrorRoot, path) if err != nil { logger.WarnContext(ctx, "Failed to get relative path", @@ -369,7 +322,6 @@ func (s *Strategy) discoverExistingClones(ctx context.Context) error { return nil } - // Convert path to upstream URL: {host}/{path}.git -> https://{host}/{path} parts := strings.Split(filepath.ToSlash(relPath), "/") if len(parts) < 2 { return nil @@ -379,7 +331,6 @@ func (s *Strategy) discoverExistingClones(ctx context.Context) error { repoPath := strings.Join(parts[1:], "/") upstreamURL := "https://" + host + "/" + repoPath - // Create clone entry c := &clone{ state: stateReady, path: path, @@ -396,7 +347,6 @@ func (s *Strategy) discoverExistingClones(ctx context.Context) error { slog.String("path", path), slog.String("upstream", upstreamURL)) - // Start bundle generation loop if s.config.BundleInterval > 0 { go s.cloneBundleLoop(s.ctx, c) } @@ -411,7 +361,6 @@ func (s *Strategy) discoverExistingClones(ctx context.Context) error { return nil } -// startClone initiates a git clone operation. func (s *Strategy) startClone(ctx context.Context, c *clone) { logger := logging.FromContext(ctx) @@ -446,13 +395,11 @@ func (s *Strategy) startClone(ctx context.Context, c *clone) { slog.String("upstream", c.upstreamURL), slog.String("path", c.path)) - // Start bundle generation loop for new clone if s.config.BundleInterval > 0 { go s.cloneBundleLoop(context.WithoutCancel(ctx), c) } } -// maybeBackgroundFetch triggers a background fetch if enough time has passed. func (s *Strategy) maybeBackgroundFetch(ctx context.Context, c *clone) { c.mu.RLock() lastFetch := c.lastFetch @@ -465,17 +412,15 @@ func (s *Strategy) maybeBackgroundFetch(ctx context.Context, c *clone) { go s.backgroundFetch(context.WithoutCancel(ctx), c) } -// backgroundFetch fetches updates from upstream. func (s *Strategy) backgroundFetch(ctx context.Context, c *clone) { logger := logging.FromContext(ctx) c.mu.Lock() - // Double-check timing after acquiring lock if time.Since(c.lastFetch) < s.config.FetchInterval { c.mu.Unlock() return } - c.lastFetch = time.Now() // Update immediately to prevent concurrent fetches + c.lastFetch = time.Now() c.mu.Unlock() logger.DebugContext(ctx, "Fetching updates", diff --git a/internal/strategy/git/proxy.go b/internal/strategy/git/proxy.go index 2db0b96..ecacbc4 100644 --- a/internal/strategy/git/proxy.go +++ b/internal/strategy/git/proxy.go @@ -7,7 +7,6 @@ import ( "github.com/block/cachew/internal/logging" ) -// forwardToUpstream forwards a request to the upstream Git server. func (s *Strategy) forwardToUpstream(w http.ResponseWriter, r *http.Request, host, pathValue string) { logger := logging.FromContext(r.Context())