Skip to content
This repository has been archived by the owner on Sep 9, 2020. It is now read-only.

gps: Adaptively clean dirty git repos #1279

Merged
merged 5 commits into from
Oct 29, 2017
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -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:

Expand Down
6 changes: 0 additions & 6 deletions internal/gps/manager_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
10 changes: 9 additions & 1 deletion internal/gps/maybe_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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
}

Expand Down
1 change: 1 addition & 0 deletions internal/gps/source_manager.go
Original file line number Diff line number Diff line change
Expand Up @@ -738,6 +738,7 @@ const (
ctSourceInit
ctSourceFetch
ctExportTree
ctValidateLocal
)

func (ct callType) String() string {
Expand Down
68 changes: 68 additions & 0 deletions internal/gps/vcs_source.go
Original file line number Diff line number Diff line change
Expand Up @@ -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

Expand Down
95 changes: 95 additions & 0 deletions internal/gps/vcs_source_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ package gps
import (
"context"
"io/ioutil"
"log"
"net/url"
"os"
"os/exec"
Expand Down Expand Up @@ -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.com-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()

Expand Down