From e42e8ceed792e40ac2874a8a32155545ec1e3e20 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 20 Oct 2017 16:57:33 +0100 Subject: [PATCH 1/4] linuxkit pkg: improve handling of git working directory Ensure that all git commands are run as if from the package directory using the `-C` option. Otherwise the various attempts to use git fail if `linuxkit pkg` is invoked from outside the git repo. Signed-off-by: Ian Campbell --- src/cmd/linuxkit/pkglib/build.go | 4 ++-- src/cmd/linuxkit/pkglib/git.go | 38 ++++++++++++++++++++----------- src/cmd/linuxkit/pkglib/pkglib.go | 8 +++++-- 3 files changed, 33 insertions(+), 17 deletions(-) diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go index e2a1d50e4..f32c723be 100644 --- a/src/cmd/linuxkit/pkglib/build.go +++ b/src/cmd/linuxkit/pkglib/build.go @@ -71,7 +71,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { } if bo.release == "" { - r, err := gitCommitTag("HEAD") + r, err := p.git.commitTag("HEAD") if err != nil { return err } @@ -101,7 +101,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { args = append(args, "--label", "org.opencontainers.image.source="+p.gitRepo) } if !p.dirty { - commit, err := gitCommitHash("HEAD") + commit, err := p.git.commitHash("HEAD") if err != nil { return err } diff --git a/src/cmd/linuxkit/pkglib/git.go b/src/cmd/linuxkit/pkglib/git.go index ce2f48ce7..de60b5d33 100644 --- a/src/cmd/linuxkit/pkglib/git.go +++ b/src/cmd/linuxkit/pkglib/git.go @@ -19,8 +19,20 @@ func init() { treeHashRe = regexp.MustCompile("^[0-7]{6} [^ ]+ ([0-9a-f]{40})\t.+\n$") } -func gitCommandStdout(args ...string) (string, error) { - cmd := exec.Command("git", args...) +type git struct { + dir string +} + +func newGit(dir string) *git { + return &git{dir} +} + +func (g git) mkCmd(args ...string) *exec.Cmd { + return exec.Command("git", append([]string{"-C", g.dir}, args...)...) +} + +func (g git) commandStdout(args ...string) (string, error) { + cmd := g.mkCmd(args...) cmd.Stderr = os.Stderr if debugGitCommands { @@ -33,8 +45,8 @@ func gitCommandStdout(args ...string) (string, error) { return string(out), nil } -func gitCommand(args ...string) error { - cmd := exec.Command("git", args...) +func (g git) command(args ...string) error { + cmd := g.mkCmd(args...) cmd.Stdout = os.Stdout cmd.Stderr = os.Stderr if debugGitCommands { @@ -43,8 +55,8 @@ func gitCommand(args ...string) error { return cmd.Run() } -func gitTreeHash(pkg, commit string) (string, error) { - out, err := gitCommandStdout("ls-tree", "--full-tree", commit, "--", pkg) +func (g git) treeHash(pkg, commit string) (string, error) { + out, err := g.commandStdout("ls-tree", "--full-tree", commit, "--", pkg) if err != nil { return "", err } @@ -57,8 +69,8 @@ func gitTreeHash(pkg, commit string) (string, error) { return matches[1], nil } -func gitCommitHash(commit string) (string, error) { - out, err := gitCommandStdout("rev-parse", commit) +func (g git) commitHash(commit string) (string, error) { + out, err := g.commandStdout("rev-parse", commit) if err != nil { return "", err } @@ -66,8 +78,8 @@ func gitCommitHash(commit string) (string, error) { return strings.TrimSpace(out), nil } -func gitCommitTag(commit string) (string, error) { - out, err := gitCommandStdout("tag", "-l", "--points-at", commit) +func (g git) commitTag(commit string) (string, error) { + out, err := g.commandStdout("tag", "-l", "--points-at", commit) if err != nil { return "", err } @@ -75,7 +87,7 @@ func gitCommitTag(commit string) (string, error) { return strings.TrimSpace(out), nil } -func gitIsDirty(pkg, commit string) (bool, error) { +func (g git) isDirty(pkg, commit string) (bool, error) { // If it isn't HEAD it can't be dirty if commit != "HEAD" { return false, nil @@ -86,11 +98,11 @@ func gitIsDirty(pkg, commit string) (bool, error) { // because `git diff-index` only uses the `lstat` result and // not the actual file contents. Running `git update-index // --refresh` updates the cache. - if err := gitCommand("update-index", "-q", "--refresh"); err != nil { + if err := g.command("update-index", "-q", "--refresh"); err != nil { return false, err } - err := gitCommand("diff-index", "--quiet", commit, "--", pkg) + err := g.command("diff-index", "--quiet", commit, "--", pkg) if err == nil { return false, nil } diff --git a/src/cmd/linuxkit/pkglib/pkglib.go b/src/cmd/linuxkit/pkglib/pkglib.go index a5578d7f9..1b44f003a 100644 --- a/src/cmd/linuxkit/pkglib/pkglib.go +++ b/src/cmd/linuxkit/pkglib/pkglib.go @@ -36,6 +36,7 @@ type Pkg struct { hash string dirty bool commitHash string + git *git } // NewFromCLI creates a Pkg from a set of CLI arguments. Calls fs.Parse() @@ -138,7 +139,9 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) { } }) - gitDirty, err := gitIsDirty(hashPath, hashCommit) + git := newGit(pkgPath) + + gitDirty, err := git.isDirty(hashPath, hashCommit) if err != nil { return Pkg{}, err } @@ -146,7 +149,7 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) { dirty = dirty || gitDirty if hash == "" { - if hash, err = gitTreeHash(hashPath, hashCommit); err != nil { + if hash, err = git.treeHash(hashPath, hashCommit); err != nil { return Pkg{}, err } @@ -167,6 +170,7 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) { cache: !pi.DisableCache, dirty: dirty, pkgPath: pkgPath, + git: git, }, nil } From 0e31d8d1a9f49e4b5d3d9a26cec95a997bc87b13 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 20 Oct 2017 17:05:37 +0100 Subject: [PATCH 2/4] linuxkit pkg: allow caller of git to specify what happens to stderr Currently all forward it to os.Stderr, but in my next patch I will want to direct to /dev/null for one command. Signed-off-by: Ian Campbell --- src/cmd/linuxkit/pkglib/git.go | 11 ++++++----- 1 file changed, 6 insertions(+), 5 deletions(-) diff --git a/src/cmd/linuxkit/pkglib/git.go b/src/cmd/linuxkit/pkglib/git.go index de60b5d33..ee7498b3f 100644 --- a/src/cmd/linuxkit/pkglib/git.go +++ b/src/cmd/linuxkit/pkglib/git.go @@ -4,6 +4,7 @@ package pkglib import ( "fmt" + "io" "os" "os/exec" "regexp" @@ -31,9 +32,9 @@ func (g git) mkCmd(args ...string) *exec.Cmd { return exec.Command("git", append([]string{"-C", g.dir}, args...)...) } -func (g git) commandStdout(args ...string) (string, error) { +func (g git) commandStdout(stderr io.Writer, args ...string) (string, error) { cmd := g.mkCmd(args...) - cmd.Stderr = os.Stderr + cmd.Stderr = stderr if debugGitCommands { fmt.Fprintf(os.Stderr, "+ %v\n", cmd.Args) @@ -56,7 +57,7 @@ func (g git) command(args ...string) error { } func (g git) treeHash(pkg, commit string) (string, error) { - out, err := g.commandStdout("ls-tree", "--full-tree", commit, "--", pkg) + out, err := g.commandStdout(os.Stderr, "ls-tree", "--full-tree", commit, "--", pkg) if err != nil { return "", err } @@ -70,7 +71,7 @@ func (g git) treeHash(pkg, commit string) (string, error) { } func (g git) commitHash(commit string) (string, error) { - out, err := g.commandStdout("rev-parse", commit) + out, err := g.commandStdout(os.Stderr, "rev-parse", commit) if err != nil { return "", err } @@ -79,7 +80,7 @@ func (g git) commitHash(commit string) (string, error) { } func (g git) commitTag(commit string) (string, error) { - out, err := g.commandStdout("tag", "-l", "--points-at", commit) + out, err := g.commandStdout(os.Stderr, "tag", "-l", "--points-at", commit) if err != nil { return "", err } From 991bfd27940fffd50a89853d7336db0faa729766 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 20 Oct 2017 17:07:08 +0100 Subject: [PATCH 3/4] linuxkit pkg: handle packages which are not in git Detect if this is the case by checking if the given path is not in git and conditionalising anything which would touch git. Images built from outside git will, in the absence of options to force otherwise, get tagged "latest". Fixes: #2613 Signed-off-by: Ian Campbell --- src/cmd/linuxkit/pkglib/build.go | 6 +++--- src/cmd/linuxkit/pkglib/git.go | 34 +++++++++++++++++++++++++++++-- src/cmd/linuxkit/pkglib/pkglib.go | 29 +++++++++++++++++--------- 3 files changed, 54 insertions(+), 15 deletions(-) diff --git a/src/cmd/linuxkit/pkglib/build.go b/src/cmd/linuxkit/pkglib/build.go index f32c723be..a52a92997 100644 --- a/src/cmd/linuxkit/pkglib/build.go +++ b/src/cmd/linuxkit/pkglib/build.go @@ -70,7 +70,7 @@ func (p Pkg) Build(bos ...BuildOpt) error { return fmt.Errorf("Unknown arch %q", arch) } - if bo.release == "" { + if p.git != nil && bo.release == "" { r, err := p.git.commitTag("HEAD") if err != nil { return err @@ -97,10 +97,10 @@ func (p Pkg) Build(bos ...BuildOpt) error { var args []string - if p.gitRepo != "" { + if p.git != nil && p.gitRepo != "" { args = append(args, "--label", "org.opencontainers.image.source="+p.gitRepo) } - if !p.dirty { + if p.git != nil && !p.dirty { commit, err := p.git.commitHash("HEAD") if err != nil { return err diff --git a/src/cmd/linuxkit/pkglib/git.go b/src/cmd/linuxkit/pkglib/git.go index ee7498b3f..41a8c990d 100644 --- a/src/cmd/linuxkit/pkglib/git.go +++ b/src/cmd/linuxkit/pkglib/git.go @@ -24,8 +24,19 @@ type git struct { dir string } -func newGit(dir string) *git { - return &git{dir} +// Returns git==nil and no error if the path is not within a git repository +func newGit(dir string) (*git, error) { + g := &git{dir} + + // Check if dir really is within a git directory + ok, err := g.isWorkTree(dir) + if err != nil { + return nil, err + } + if !ok { + return nil, nil + } + return g, nil } func (g git) mkCmd(args ...string) *exec.Cmd { @@ -56,6 +67,25 @@ func (g git) command(args ...string) error { return cmd.Run() } +func (g git) isWorkTree(pkg string) (bool, error) { + tf, err := g.commandStdout(nil, "rev-parse", "--is-inside-work-tree") + if err != nil { + // If we executed git ok but it errored then that's because this isn't a git repo + if _, ok := err.(*exec.ExitError); ok { + return false, nil + } + return false, err + } + + tf = strings.TrimSpace(tf) + + if tf == "true" { + return true, nil + } + + return false, fmt.Errorf("unexpected output from git rev-parse --is-inside-work-tree: %s", tf) +} + func (g git) treeHash(pkg, commit string) (string, error) { out, err := g.commandStdout(os.Stderr, "ls-tree", "--full-tree", commit, "--", pkg) if err != nil { diff --git a/src/cmd/linuxkit/pkglib/pkglib.go b/src/cmd/linuxkit/pkglib/pkglib.go index 1b44f003a..aab1e05c9 100644 --- a/src/cmd/linuxkit/pkglib/pkglib.go +++ b/src/cmd/linuxkit/pkglib/pkglib.go @@ -139,22 +139,27 @@ func NewFromCLI(fs *flag.FlagSet, args ...string) (Pkg, error) { } }) - git := newGit(pkgPath) - - gitDirty, err := git.isDirty(hashPath, hashCommit) + git, err := newGit(pkgPath) if err != nil { return Pkg{}, err } - dirty = dirty || gitDirty - - if hash == "" { - if hash, err = git.treeHash(hashPath, hashCommit); err != nil { + if git != nil { + gitDirty, err := git.isDirty(hashPath, hashCommit) + if err != nil { return Pkg{}, err } - if dirty { - hash += "-dirty" + dirty = dirty || gitDirty + + if hash == "" { + if hash, err = git.treeHash(hashPath, hashCommit); err != nil { + return Pkg{}, err + } + + if dirty { + hash += "-dirty" + } } } @@ -193,7 +198,11 @@ func (p Pkg) ReleaseTag(release string) (string, error) { // Tag returns the tag to use for the package func (p Pkg) Tag() string { - return p.org + "/" + p.image + ":" + p.hash + r := p.org + "/" + p.image + if p.hash != "" { + r += ":" + p.hash + } + return r } // TrustEnabled returns true if trust is enabled From ba3cc2fc6d9857883cb147be0a43fdf69fbdd8c0 Mon Sep 17 00:00:00 2001 From: Ian Campbell Date: Fri, 20 Oct 2017 17:18:00 +0100 Subject: [PATCH 4/4] linuxkit pkg: make ":latest" for non-git packages explicit Signed-off-by: Ian Campbell --- src/cmd/linuxkit/pkglib/pkglib.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/src/cmd/linuxkit/pkglib/pkglib.go b/src/cmd/linuxkit/pkglib/pkglib.go index aab1e05c9..5e7ac0a8f 100644 --- a/src/cmd/linuxkit/pkglib/pkglib.go +++ b/src/cmd/linuxkit/pkglib/pkglib.go @@ -198,11 +198,11 @@ func (p Pkg) ReleaseTag(release string) (string, error) { // Tag returns the tag to use for the package func (p Pkg) Tag() string { - r := p.org + "/" + p.image - if p.hash != "" { - r += ":" + p.hash + t := p.hash + if t == "" { + t = "latest" } - return r + return p.org + "/" + p.image + ":" + t } // TrustEnabled returns true if trust is enabled