From 4304d5f4fde635ad2bb3f6bd9ca94ad2a6cc29de Mon Sep 17 00:00:00 2001
From: Ian Campbell <ijc@docker.com>
Date: Thu, 12 Oct 2017 10:07:09 +0100
Subject: [PATCH] linuxkit pkg: correct behaviour when user explictly specifies
 a hash.
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

With the old package.mk arrangements we would only append -dirty and -$arch to
the hash if the user didn't specify a HASH= directly. So the following would
work
    make ORG=ijc HASH=dev tag
and produce an image `ijc/«image»:dev`.

However the new equivalent:
    linuxkit pkg build -org=ijc --hash-=dev
instead produces an image `ijc/«image»:dev-dirty-$arch`. Which is undesirable.

This commit refactors the code in two main ways:

Firstly directly append `-dirty` to the hash as we extract it from git (rather
than on use in the `Tag()` method), and only do this if the user has not
specified an explicit tag. Note that we still track dirtiness in the `Pkg`
object and so will not allow a push (or release) from a dirty tree (the
makefile version would have tried this with unpredictable results), nor will we
apply the `org.opencontainers.image.revision` label to a dirty build.

Secondly if we are not pushing the image+manifest then we retag the -$arch
suffixed image without the the -$arch. This differs from the Makefile version
which would simply have built without the -$arch in the first place, I think
this is an improvement. If we are pushing the manifest-tool remains responsible
for creating the non -$arch image.

Signed-off-by: Ian Campbell <ijc@docker.com>
---
 src/cmd/linuxkit/pkglib/build.go  |  4 ++++
 src/cmd/linuxkit/pkglib/docker.go |  3 +++
 src/cmd/linuxkit/pkglib/pkglib.go | 22 +++++++++++-----------
 3 files changed, 18 insertions(+), 11 deletions(-)

diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go
index c570b4863..e2a1d50e4 100644
--- a/src/cmd/linuxkit/pkglib/build.go
+++ b/src/cmd/linuxkit/pkglib/build.go
@@ -117,6 +117,10 @@ func (p Pkg) Build(bos ...BuildOpt) error {
 	}
 
 	if !bo.push {
+		if err := d.tag(p.Tag()+suffix, p.Tag()); err != nil {
+			return err
+		}
+
 		fmt.Printf("Build complete, not pushing, all done.\n")
 		return nil
 	}
diff --git a/src/cmd/linuxkit/pkglib/docker.go b/src/cmd/linuxkit/pkglib/docker.go
index f3d377ad3..3f9daecc8 100644
--- a/src/cmd/linuxkit/pkglib/docker.go
+++ b/src/cmd/linuxkit/pkglib/docker.go
@@ -71,6 +71,7 @@ func (dr dockerRunner) push(img string) error {
 }
 
 func (dr dockerRunner) pushWithManifest(img, suffix string) error {
+	fmt.Printf("Pushing %s\n", img+suffix)
 	if err := dr.push(img + suffix); err != nil {
 		return err
 	}
@@ -80,6 +81,7 @@ func (dr dockerRunner) pushWithManifest(img, suffix string) error {
 		dctArg = "1"
 	}
 
+	fmt.Printf("Pushing %s to manifest %s\n", img+suffix, img)
 	cmd := exec.Command("/bin/sh", "-c", manifestPushScript, "manifest-push-script", img, dctArg)
 	cmd.Stdout = os.Stdout
 	cmd.Stderr = os.Stderr
@@ -90,6 +92,7 @@ func (dr dockerRunner) pushWithManifest(img, suffix string) error {
 }
 
 func (dr dockerRunner) tag(ref, tag string) error {
+	fmt.Printf("Tagging %s as %s\n", ref, tag)
 	return dr.command("tag", ref, tag)
 }
 
diff --git a/src/cmd/linuxkit/pkglib/pkglib.go b/src/cmd/linuxkit/pkglib/pkglib.go
index ee5a584a5..27e401f77 100644
--- a/src/cmd/linuxkit/pkglib/pkglib.go
+++ b/src/cmd/linuxkit/pkglib/pkglib.go
@@ -138,12 +138,6 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) {
 		}
 	})
 
-	if hash == "" {
-		if hash, err = gitTreeHash(hashPath, hashCommit); err != nil {
-			return Pkg{}, err
-		}
-	}
-
 	gitDirty, err := gitIsDirty(hashPath, hashCommit)
 	if err != nil {
 		return Pkg{}, err
@@ -151,6 +145,16 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) {
 
 	dirty = dirty || gitDirty
 
+	if hash == "" {
+		if hash, err = gitTreeHash(hashPath, hashCommit); err != nil {
+			return Pkg{}, err
+		}
+
+		if dirty {
+			hash += "-dirty"
+		}
+	}
+
 	return Pkg{
 		image:      pi.Image,
 		org:        pi.Org,
@@ -185,11 +189,7 @@ func (p Pkg) ReleaseTag(release string) (string, error) {
 
 // Tag returns the tag to use for the package
 func (p Pkg) Tag() string {
-	tag := p.org + "/" + p.image + ":" + p.hash
-	if p.dirty {
-		tag += "-dirty"
-	}
-	return tag
+	return p.org + "/" + p.image + ":" + p.hash
 }
 
 func (p Pkg) archSupported(want string) bool {