Merge pull request #1279 from sdboyer/git-autorecover

gps: Adaptively clean dirty git repos

Author: sdboyer

CHANGELOG.md

@@ -3,6 +3,7 @@
 BUG FIXES:
 
 * Releases targeting Windows now have a `.exe` suffix (#1291).
+* Adaptively recover from dirty and corrupted git repositories in cache (#1279).
 
 IMPROVEMENTS:
 

internal/gps/manager_test.go

@@ -259,12 +259,6 @@ func TestSourceInit(t *testing.T) {
 
 	os.Stat(filepath.Join(cpath, "metadata", "github.com", "sdboyer", "gpkt", "cache.json"))
 
-	// TODO(sdboyer) disabled until we get caching working
-	//_, err = os.Stat(filepath.Join(cpath, "metadata", "github.com", "sdboyer", "gpkt", "cache.json"))
-	//if err != nil {
-	//t.Error("Metadata cache json file does not exist in expected location")
-	//}
-
 	// Ensure source existence values are what we expect
 	var exists bool
 	exists, err = sm.SourceExists(id)

internal/gps/maybe_source.go

@@ -100,13 +100,21 @@ func (m maybeGitSource) try(ctx context.Context, cachedir string, c singleSource
 		return nil, 0, err
 	}
 
-	c.setVersionMap(vl)
 	state := sourceIsSetUp | sourceExistsUpstream | sourceHasLatestVersionList
 
 	if r.CheckLocal() {
 		state |= sourceExistsLocally
+
+		if err := superv.do(ctx, "git", ctValidateLocal, func(ctx context.Context) error {
+			// If repository already exists on disk, make a pass to be sure
+			// everything's clean.
+			return src.ensureClean(ctx)
+		}); err != nil {
+			return nil, 0, err
+		}
 	}
 
+	c.setVersionMap(vl)
 	return src, state, nil
 }
 

internal/gps/source_manager.go

@@ -738,6 +738,7 @@ const (
 	ctSourceInit
 	ctSourceFetch
 	ctExportTree
+	ctValidateLocal
 )
 
 func (ct callType) String() string {

internal/gps/vcs_source.go

@@ -123,6 +123,74 @@ type gitSource struct {
 	baseVCSSource
 }
 
+// ensureClean sees to it that a git repository is clean and in working order,
+// or returns an error if the adaptive recovery attempts fail.
+func (s *gitSource) ensureClean(ctx context.Context) error {
+	r := s.repo.(*gitRepo)
+	cmd := commandContext(
+		ctx,
+		"git",
+		"status",
+		"--porcelain",
+	)
+	cmd.SetDir(r.LocalPath())
+
+	out, err := cmd.CombinedOutput()
+	if err != nil {
+		// An error on simple git status indicates some aggressive repository
+		// corruption, outside of the purview that we can deal with here.
+		return err
+	}
+
+	if len(bytes.TrimSpace(out)) == 0 {
+		// No output from status indicates a clean tree, without any modified or
+		// untracked files - we're in good shape.
+		return nil
+	}
+
+	// We could be more parsimonious about this, but it's probably not worth it
+	// - it's a rare case to have to do any cleanup anyway, so when we do, we
+	// might as well just throw the kitchen sink at it.
+	cmd = commandContext(
+		ctx,
+		"git",
+		"reset",
+		"--hard",
+	)
+	cmd.SetDir(r.LocalPath())
+	_, err = cmd.CombinedOutput()
+	if err != nil {
+		return err
+	}
+
+	// We also need to git clean -df; just reuse defendAgainstSubmodules here,
+	// even though it's a bit layer-breaky.
+	err = r.defendAgainstSubmodules(ctx)
+	if err != nil {
+		return err
+	}
+
+	// Check status one last time. If it's still not clean, give up.
+	cmd = commandContext(
+		ctx,
+		"git",
+		"status",
+		"--porcelain",
+	)
+	cmd.SetDir(r.LocalPath())
+
+	out, err = cmd.CombinedOutput()
+	if err != nil {
+		return err
+	}
+
+	if len(bytes.TrimSpace(out)) != 0 {
+		return errors.Errorf("failed to clean up git repository at %s - dirty? corrupted? status output: \n%s", r.LocalPath(), string(out))
+	}
+
+	return nil
+}
+
 func (s *gitSource) exportRevisionTo(ctx context.Context, rev Revision, to string) error {
 	r := s.repo
 

internal/gps/vcs_source_test.go

@@ -7,6 +7,7 @@ package gps
 import (
 	"context"
 	"io/ioutil"
+	"log"
 	"net/url"
 	"os"
 	"os/exec"
@@ -647,6 +648,100 @@ func TestGitSourceListVersionsNoDupes(t *testing.T) {
 	}
 }
 
+func TestGitSourceAdaptiveCleanup(t *testing.T) {
+	t.Parallel()
+
+	// This test is slowish, skip it on -short
+	if testing.Short() {
+		t.Skip("Skipping git adaptive failure recovery test in short mode")
+	}
+	requiresBins(t, "git")
+
+	cpath, err := ioutil.TempDir("", "smcache")
+	if err != nil {
+		t.Fatalf("Failed to create temp dir: %s", err)
+	}
+
+	var sm *SourceMgr
+	mkSM := func() {
+		// If sm is already set, make sure it's released, then create a new one.
+		if sm != nil {
+			sm.Release()
+		}
+
+		var err error
+		sm, err = NewSourceManager(SourceManagerConfig{
+			Cachedir: cpath,
+			Logger:   log.New(test.Writer{TB: t}, "", 0),
+		})
+		if err != nil {
+			t.Fatalf("Unexpected error on SourceManager creation: %s", err)
+		}
+	}
+
+	mkSM()
+	id := mkPI("github.com/sdboyer/gpkt")
+	err = sm.SyncSourceFor(id)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	repodir := filepath.Join(sm.cachedir, "sources", "https---github.1485827954.workers.dev-sdboyer-gpkt")
+	if _, err := os.Stat(repodir); err != nil {
+		if os.IsNotExist(err) {
+			t.Fatalf("expected location for repodir did not exist: %q", repodir)
+		} else {
+			t.Fatal(err)
+		}
+	}
+
+	// Create a file that git will see as untracked.
+	untrackedPath := filepath.Join(repodir, "untrackedfile")
+	err = ioutil.WriteFile(untrackedPath, []byte("foo"), 0644)
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	mkSM()
+	err = sm.SyncSourceFor(id)
+	if err != nil {
+		t.Fatalf("choked after adding dummy file: %q", err)
+	}
+
+	if _, err := os.Stat(untrackedPath); err == nil {
+		t.Fatal("untracked file still existed after cleanup should've been triggered")
+	}
+
+	// Remove a file that we know exists, which `git status` checks should catch.
+	readmePath := filepath.Join(repodir, "README.md")
+	os.Remove(readmePath)
+
+	mkSM()
+	err = sm.SyncSourceFor(id)
+	if err != nil {
+		t.Fatalf("choked after removing known file: %q", err)
+	}
+
+	if _, err := os.Stat(readmePath); err != nil {
+		t.Fatal("README was still absent after cleanup should've been triggered")
+	}
+
+	// Remove .git/objects directory, which should make git bite it.
+	err = os.RemoveAll(filepath.Join(repodir, ".git", "objects"))
+	if err != nil {
+		t.Fatal(err)
+	}
+
+	mkSM()
+	err = sm.SyncSourceFor(id)
+	if err != nil {
+		t.Fatalf("choked after removing .git/objects directory: %q", err)
+	}
+
+	sm.Release()
+	os.RemoveAll(cpath)
+}
+
 func Test_bzrSource_exportRevisionTo_removeVcsFiles(t *testing.T) {
 	t.Parallel()