Use git.Export as the base of git.Checkout

To ease the way for distinguishing between cases where we want a
read-only view of the files, and those where we need a clone to
prepare commits in, this commit lines git.Export and git.Checkout up,
by embedding the former in the latter. This removes a long-running
duplication of a couple of methods.

There's also a modest move towards letting manifests be generated from
an Export rather than needing a checkout. In theory, all you need are
the files, but to get there in practice we have to break down some
assumptions (e.g., instead of requiring a git.Checkout, require an
interface from which you can get the directories of interest).

daemon/daemon.go

@@ -74,11 +74,16 @@ func (d *Daemon) Export(ctx context.Context) ([]byte, error) {
 	return d.Cluster.Export(ctx)
 }
 
-func (d *Daemon) getManifestStore(checkout *git.Checkout) (manifests.Store, error) {
+type repo interface {
+	Dir() string
+	AbsolutePaths() []string
+}
+
+func (d *Daemon) getManifestStore(r repo) (manifests.Store, error) {
 	if d.ManifestGenerationEnabled {
-		return manifests.NewConfigAware(checkout.Dir(), checkout.ManifestDirs(), d.Manifests)
+		return manifests.NewConfigAware(r.Dir(), r.AbsolutePaths(), d.Manifests)
 	}
-	return manifests.NewRawFiles(checkout.Dir(), checkout.ManifestDirs(), d.Manifests), nil
+	return manifests.NewRawFiles(r.Dir(), r.AbsolutePaths(), d.Manifests), nil
 }
 
 func (d *Daemon) getResources(ctx context.Context) (map[string]resource.Resource, v6.ReadOnlyReason, error) {
@@ -680,13 +685,13 @@ func containers2containers(cs []resource.Container) []v6.Container {
 // cost, we cache the result of sorting, so that other uses of the
 // image can reuse it (if they are also sorted by timestamp).
 
-type repo struct {
+type sortedImageRepo struct {
 	images                []image.Info
 	imagesByTag           map[string]image.Info
 	imagesSortedByCreated update.SortedImageInfos
 }
 
-func (r *repo) SortedImages(p policy.Pattern) update.SortedImageInfos {
+func (r *sortedImageRepo) SortedImages(p policy.Pattern) update.SortedImageInfos {
 	// RequiresTimestamp means "ordered by timestamp" (it's required
 	// because no comparison to see which image is newer can be made
 	// if a timestamp is missing)
@@ -699,16 +704,16 @@ func (r *repo) SortedImages(p policy.Pattern) update.SortedImageInfos {
 	return update.SortImages(r.images, p)
 }
 
-func (r *repo) Images() []image.Info {
+func (r *sortedImageRepo) Images() []image.Info {
 	return r.images
 }
 
-func (r *repo) ImageByTag(tag string) image.Info {
+func (r *sortedImageRepo) ImageByTag(tag string) image.Info {
 	return r.imagesByTag[tag]
 }
 
 func getWorkloadContainers(workload cluster.Workload, imageRepos update.ImageRepos, resource resource.Resource, fields []string) (res []v6.Container, err error) {
-	repos := map[image.Name]*repo{}
+	repos := map[image.Name]*sortedImageRepo{}
 
 	for _, c := range workload.ContainersOrNil() {
 		imageName := c.Image.Name
@@ -732,7 +737,7 @@ func getWorkloadContainers(workload cluster.Workload, imageRepos update.ImageRep
 				}
 				images = append(images, info)
 			}
-			imageRepo = &repo{images: images, imagesByTag: repoMetadata.Images}
+			imageRepo = &sortedImageRepo{images: images, imagesByTag: repoMetadata.Images}
 			repos[imageName] = imageRepo
 		}
 

daemon/daemon_test.go

@@ -472,7 +472,7 @@ func TestDaemon_Release(t *testing.T) {
 		}
 		defer co.Clean()
 		// open a file
-		dirs := co.ManifestDirs()
+		dirs := co.AbsolutePaths()
 		if file, err := os.Open(filepath.Join(dirs[0], "helloworld-deploy.yaml")); err == nil {
 
 			// make sure it gets closed
@@ -517,7 +517,7 @@ func TestDaemon_PolicyUpdate(t *testing.T) {
 			return false
 		}
 		defer co.Clean()
-		cm := manifests.NewRawFiles(co.Dir(), co.ManifestDirs(), d.Manifests)
+		cm := manifests.NewRawFiles(co.Dir(), co.AbsolutePaths(), d.Manifests)
 		m, err := cm.GetAllResourcesByID(context.TODO())
 		if err != nil {
 			t.Fatalf("Error: %s", err.Error())
@@ -861,7 +861,7 @@ func (w *wait) ForImageTag(t *testing.T, d *Daemon, workload, container, tag str
 			return false
 		}
 		defer co.Clean()
-		cm := manifests.NewRawFiles(co.Dir(), co.ManifestDirs(), d.Manifests)
+		cm := manifests.NewRawFiles(co.Dir(), co.AbsolutePaths(), d.Manifests)
 		resources, err := cm.GetAllResourcesByID(context.TODO())
 		assert.NoError(t, err)
 

daemon/sync_test.go

@@ -258,7 +258,7 @@ func TestDoSync_WithNewCommit(t *testing.T) {
 			return err
 		}
 		// Push some new changes
-		cm := manifests.NewRawFiles(checkout.Dir(), checkout.ManifestDirs(), d.Manifests)
+		cm := manifests.NewRawFiles(checkout.Dir(), checkout.AbsolutePaths(), d.Manifests)
 		resourcesByID, err := cm.GetAllResourcesByID(context.TODO())
 		if err != nil {
 			return err

git/gittest/repo_test.go

@@ -28,7 +28,7 @@ func TestCommit(t *testing.T) {
 	defer cleanup()
 
 	for file, _ := range testfiles.Files {
-		dirs := checkout.ManifestDirs()
+		dirs := checkout.AbsolutePaths()
 		path := filepath.Join(dirs[0], file)
 		if err := ioutil.WriteFile(path, []byte("FIRST CHANGE"), 0666); err != nil {
 			t.Fatal(err)
@@ -84,7 +84,7 @@ func TestSignedCommit(t *testing.T) {
 	defer cleanup()
 
 	for file, _ := range testfiles.Files {
-		dirs := checkout.ManifestDirs()
+		dirs := checkout.AbsolutePaths()
 		path := filepath.Join(dirs[0], file)
 		if err := ioutil.WriteFile(path, []byte("FIRST CHANGE"), 0666); err != nil {
 			t.Fatal(err)
@@ -199,7 +199,7 @@ func TestCheckout(t *testing.T) {
 	}
 
 	changedFile := ""
-	dirs := checkout.ManifestDirs()
+	dirs := checkout.AbsolutePaths()
 	for file, _ := range testfiles.Files {
 		path := filepath.Join(dirs[0], file)
 		if err := ioutil.WriteFile(path, []byte("FIRST CHANGE"), 0666); err != nil {

git/working.go

@@ -29,7 +29,7 @@ type Config struct {
 // intended to be used for one-off "transactions", e.g,. committing
 // changes then pushing upstream. It has no locking.
 type Checkout struct {
-	dir          string
+	*Export
 	config       Config
 	upstream     Remote
 	realNotesRef string // cache the notes ref, since we use it to push as well
@@ -105,36 +105,24 @@ func (r *Repo) Clone(ctx context.Context, conf Config) (*Checkout, error) {
 	}
 
 	return &Checkout{
-		dir:          repoDir,
+		Export:       &Export{dir: repoDir},
 		upstream:     upstream,
 		realNotesRef: realNotesRef,
 		config:       conf,
 	}, nil
 }
 
-// Clean a Checkout up (remove the clone)
-func (c *Checkout) Clean() {
-	if c.dir != "" {
-		os.RemoveAll(c.dir)
-	}
-}
-
-// Dir returns the path to the repo
-func (c *Checkout) Dir() string {
-	return c.dir
-}
-
-// ManifestDirs returns the paths to the manifests files. It ensures
+// AbsolutePaths returns the absolute paths as configured. It ensures
 // that at least one path is returned, so that it can be used with
 // `Manifest.LoadManifests`.
-func (c *Checkout) ManifestDirs() []string {
+func (c *Checkout) AbsolutePaths() []string {
 	if len(c.config.Paths) == 0 {
-		return []string{c.dir}
+		return []string{c.Dir()}
 	}
 
 	paths := make([]string, len(c.config.Paths), len(c.config.Paths))
 	for i, p := range c.config.Paths {
-		paths[i] = filepath.Join(c.dir, p)
+		paths[i] = filepath.Join(c.Dir(), p)
 	}
 	return paths
 }
@@ -143,12 +131,12 @@ func (c *Checkout) ManifestDirs() []string {
 // extra data as a note, and pushes the commit and note to the remote repo.
 func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction, note interface{}, addUntracked bool) error {
 	if addUntracked {
-		if err := add(ctx, c.dir, "."); err != nil {
+		if err := add(ctx, c.Dir(), "."); err != nil {
 			return err
 		}
 	}
 
-	if !check(ctx, c.dir, c.config.Paths, addUntracked) {
+	if !check(ctx, c.Dir(), c.config.Paths, addUntracked) {
 		return ErrNoChanges
 	}
 
@@ -157,7 +145,7 @@ func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction,
 		commitAction.SigningKey = c.config.SigningKey
 	}
 
-	if err := commit(ctx, c.dir, commitAction); err != nil {
+	if err := commit(ctx, c.Dir(), commitAction); err != nil {
 		return err
 	}
 
@@ -166,60 +154,60 @@ func (c *Checkout) CommitAndPush(ctx context.Context, commitAction CommitAction,
 		if err != nil {
 			return err
 		}
-		if err := addNote(ctx, c.dir, rev, c.config.NotesRef, note); err != nil {
+		if err := addNote(ctx, c.Dir(), rev, c.config.NotesRef, note); err != nil {
 			return err
 		}
 	}
 
 	refs := []string{c.config.Branch}
-	ok, err := refExists(ctx, c.dir, c.realNotesRef)
+	ok, err := refExists(ctx, c.Dir(), c.realNotesRef)
 	if ok {
 		refs = append(refs, c.realNotesRef)
 	} else if err != nil {
 		return err
 	}
 
-	if err := push(ctx, c.dir, c.upstream.URL, refs); err != nil {
+	if err := push(ctx, c.Dir(), c.upstream.URL, refs); err != nil {
 		return PushError(c.upstream.URL, err)
 	}
 	return nil
 }
 
 // GetNote gets a note for the revision specified, or nil if there is no such note.
 func (c *Checkout) GetNote(ctx context.Context, rev string, note interface{}) (bool, error) {
-	return getNote(ctx, c.dir, c.realNotesRef, rev, note)
+	return getNote(ctx, c.Dir(), c.realNotesRef, rev, note)
 }
 
 func (c *Checkout) HeadRevision(ctx context.Context) (string, error) {
-	return refRevision(ctx, c.dir, "HEAD")
+	return refRevision(ctx, c.Dir(), "HEAD")
 }
 
 func (c *Checkout) MoveTagAndPush(ctx context.Context, tagAction TagAction) error {
 	if tagAction.SigningKey == "" {
 		tagAction.SigningKey = c.config.SigningKey
 	}
-	return moveTagAndPush(ctx, c.dir, c.upstream.URL, tagAction)
+	return moveTagAndPush(ctx, c.Dir(), c.upstream.URL, tagAction)
 }
 
 // ChangedFiles does a git diff listing changed files
 func (c *Checkout) ChangedFiles(ctx context.Context, ref string) ([]string, error) {
-	list, err := changed(ctx, c.dir, ref, c.config.Paths)
+	list, err := changed(ctx, c.Dir(), ref, c.config.Paths)
 	if err == nil {
 		for i, file := range list {
-			list[i] = filepath.Join(c.dir, file)
+			list[i] = filepath.Join(c.Dir(), file)
 		}
 	}
 	return list, err
 }
 
 func (c *Checkout) NoteRevList(ctx context.Context) (map[string]struct{}, error) {
-	return noteRevList(ctx, c.dir, c.realNotesRef)
+	return noteRevList(ctx, c.Dir(), c.realNotesRef)
 }
 
 func (c *Checkout) Checkout(ctx context.Context, rev string) error {
-	return checkout(ctx, c.dir, rev)
+	return checkout(ctx, c.Dir(), rev)
 }
 
 func (c *Checkout) Add(ctx context.Context, path string) error {
-	return add(ctx, c.dir, path)
+	return add(ctx, c.Dir(), path)
 }

release/releaser_test.go

@@ -162,7 +162,7 @@ func mockCluster(running ...cluster.Workload) *mock.Mock {
 }
 
 func NewManifestStoreOrFail(t *testing.T, parser manifests.Manifests, checkout *git.Checkout) manifests.Store {
-	cm := manifests.NewRawFiles(checkout.Dir(), checkout.ManifestDirs(), parser)
+	cm := manifests.NewRawFiles(checkout.Dir(), checkout.AbsolutePaths(), parser)
 	return cm
 }
 

sync/sync_test.go

@@ -26,8 +26,8 @@ func TestSync(t *testing.T) {
 	parser := kubernetes.NewManifests(kubernetes.ConstNamespacer("default"), log.NewLogfmtLogger(os.Stdout))
 	clus := &syncCluster{map[string]string{}}
 
-	dirs := checkout.ManifestDirs()
-	rs := manifests.NewRawFiles(checkout.Dir(), checkout.ManifestDirs(), parser)
+	dirs := checkout.AbsolutePaths()
+	rs := manifests.NewRawFiles(checkout.Dir(), dirs, parser)
 	resources, err := rs.GetAllResourcesByID(context.TODO())
 	if err != nil {
 		t.Fatal(err)