From 45712c94cd1b45bda348376ccf07d1d8349735ab Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Mon, 25 Apr 2016 22:13:29 +1000 Subject: [PATCH 01/15] Added System-scoped (git config --system) versions of Find/Set/Unset/UnsetSection commands. These commands operate on config stored at system scope e.g. /etc/gitconfig. --- git/git.go | 24 +++++++++++++++++++++++- 1 file changed, 23 insertions(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index 5964b4cd..a37967fb 100644 --- a/git/git.go +++ b/git/git.go @@ -285,12 +285,18 @@ func (c *gitConfig) Find(val string) string { return output } -// Find returns the git config value for the key +// FindGlobal returns the git config value global scope for the key func (c *gitConfig) FindGlobal(val string) string { output, _ := subprocess.SimpleExec("git", "config", "--global", val) return output } +// FindSystem returns the git config value in system scope for the key +func (c *gitConfig) FindSystem(val string) string { + output, _ := subprocess.SimpleExec("git", "config", "--system", val) + return output +} + // Find returns the git config value for the key func (c *gitConfig) FindLocal(val string) string { output, _ := subprocess.SimpleExec("git", "config", "--local", val) @@ -302,15 +308,31 @@ func (c *gitConfig) SetGlobal(key, val string) { subprocess.SimpleExec("git", "config", "--global", key, val) } +// SetSystem sets the git config value for the key in the system config +func (c *gitConfig) SetSystem(key, val string) { + subprocess.SimpleExec("git", "config", "--system", key, val) +} + // UnsetGlobal removes the git config value for the key from the global config func (c *gitConfig) UnsetGlobal(key string) { subprocess.SimpleExec("git", "config", "--global", "--unset", key) } +// UnsetSystem removes the git config value for the key from the system config +func (c *gitConfig) UnsetSystem(key string) { + subprocess.SimpleExec("git", "config", "--system", "--unset", key) +} + +// UnsetGlobalSection removes the entire named section from the global config func (c *gitConfig) UnsetGlobalSection(key string) { subprocess.SimpleExec("git", "config", "--global", "--remove-section", key) } +// UnsetGlobalSection removes the entire named section from the system config +func (c *gitConfig) UnsetSystemSection(key string) { + subprocess.SimpleExec("git", "config", "--system", "--remove-section", key) +} + // SetLocal sets the git config value for the key in the specified config file func (c *gitConfig) SetLocal(file, key, val string) { args := make([]string, 1, 5) From fb1e26ceb811755b0b8cf4db0a4a5b878cc9daa9 Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Mon, 25 Apr 2016 22:14:41 +1000 Subject: [PATCH 02/15] Config attribute operations now auto-elevate to system scope from global if running as the root user (unless --local is specified), using the new System-level config commands. --- lfs/attribute.go | 22 ++++++++++++++++++---- 1 file changed, 18 insertions(+), 4 deletions(-) diff --git a/lfs/attribute.go b/lfs/attribute.go index e8fb53ce..fbe0e2bd 100644 --- a/lfs/attribute.go +++ b/lfs/attribute.go @@ -2,6 +2,7 @@ package lfs import ( "fmt" + "os" "regexp" "strings" @@ -67,7 +68,11 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { if opt.Local { currentValue = git.Config.FindLocal(key) } else { - currentValue = git.Config.FindGlobal(key) + if os.Geteuid() == 0 { + currentValue = git.Config.FindSystem(key) + } else { + currentValue = git.Config.FindGlobal(key) + } } if opt.Force || shouldReset(currentValue) { @@ -75,8 +80,13 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { git.Config.UnsetLocalKey("", key) git.Config.SetLocal("", key, value) } else { - git.Config.UnsetGlobal(key) - git.Config.SetGlobal(key, value) + if os.Geteuid() == 0 { + git.Config.UnsetSystem(key) + git.Config.SetSystem(key, value) + } else { + git.Config.UnsetGlobal(key) + git.Config.SetGlobal(key, value) + } } return nil @@ -90,7 +100,11 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { // Uninstall removes all properties in the path of this property. func (a *Attribute) Uninstall() { - git.Config.UnsetGlobalSection(a.Section) + if os.Geteuid() == 0 { + git.Config.UnsetSystemSection(a.Section) + } else { + git.Config.UnsetGlobalSection(a.Section) + } } // shouldReset determines whether or not a value is resettable given its current From 7c4db5710c79d767dc34394637d8e91b3e254cd1 Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Mon, 25 Apr 2016 17:31:12 +1000 Subject: [PATCH 03/15] Added postinst (git lfs install) and prerm (git lfs uninstall) scripts to Debian deb. These will run as root and so will operate at the new --system scope. --- debian/postinst | 2 ++ debian/prerm | 2 ++ 2 files changed, 4 insertions(+) create mode 100644 debian/postinst create mode 100644 debian/prerm diff --git a/debian/postinst b/debian/postinst new file mode 100644 index 00000000..1d50c203 --- /dev/null +++ b/debian/postinst @@ -0,0 +1,2 @@ +#!/bin/sh +git lfs install diff --git a/debian/prerm b/debian/prerm new file mode 100644 index 00000000..200089fa --- /dev/null +++ b/debian/prerm @@ -0,0 +1,2 @@ +#!/bin/sh +git lfs uninstall From 82f860abfcc35aa88303274c447cbcfb2c7de71e Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Mon, 25 Apr 2016 20:10:53 +1000 Subject: [PATCH 04/15] Added rpm %post (git lfs install) and %prerm (git lfs uninstall) scripts. These will run as root and so will operate at the new --system scope. --- rpm/SPECS/git-lfs.spec | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/rpm/SPECS/git-lfs.spec b/rpm/SPECS/git-lfs.spec index c7998f1e..8e05aee1 100644 --- a/rpm/SPECS/git-lfs.spec +++ b/rpm/SPECS/git-lfs.spec @@ -46,6 +46,12 @@ mkdir -p -m 755 ${RPM_BUILD_ROOT}/usr/share/man/man5 install -D man/*.1 ${RPM_BUILD_ROOT}/usr/share/man/man1 install -D man/*.5 ${RPM_BUILD_ROOT}/usr/share/man/man5 +%post +git lfs install + +%preun +git lfs uninstall + %check export GOPATH=`pwd` export GIT_LFS_TEST_DIR=$(mktemp -d) From 648d8c78a824abcda823791c66f9bc5cf7b7cc9b Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Tue, 26 Apr 2016 16:26:02 +1000 Subject: [PATCH 05/15] Added --system option to install command. This option is invoked from post-install scripts in rpm/deb packages. --- commands/command_install.go | 14 +++++++++++++- debian/postinst | 2 +- docs/man/git-lfs-install.1.ronn | 5 ++++- lfs/attribute.go | 34 ++++++++++++++------------------- rpm/SPECS/git-lfs.spec | 2 +- 5 files changed, 33 insertions(+), 24 deletions(-) diff --git a/commands/command_install.go b/commands/command_install.go index 692b7124..4cd35d0b 100644 --- a/commands/command_install.go +++ b/commands/command_install.go @@ -3,6 +3,7 @@ package commands import ( "github.com/github/git-lfs/lfs" "github.com/spf13/cobra" + "os" ) var ( @@ -18,6 +19,7 @@ var ( forceInstall = false localInstall = false + systemInstall = false skipSmudgeInstall = false ) @@ -26,7 +28,15 @@ func installCommand(cmd *cobra.Command, args []string) { requireInRepo() } - opt := lfs.InstallOptions{Force: forceInstall, Local: localInstall} + if systemInstall && os.Geteuid() != 0 { + Print("WARNING: current user is not root/admin, system install is likely to fail.") + } + + if localInstall && systemInstall { + Exit("Only one of --local and --system options can be specified.") + } + + opt := lfs.InstallOptions{Force: forceInstall, Local: localInstall, System: systemInstall} if skipSmudgeInstall { // assume the user is changing their smudge mode, so enable force implicitly opt.Force = true @@ -52,6 +62,8 @@ func installHooksCommand(cmd *cobra.Command, args []string) { func init() { installCmd.Flags().BoolVarP(&forceInstall, "force", "f", false, "Set the Git LFS global config, overwriting previous values.") installCmd.Flags().BoolVarP(&localInstall, "local", "l", false, "Set the Git LFS config for the local Git repository only.") + // TODO - -s was already taken by --skip-smudge, what should --system be shortened-to? + installCmd.Flags().BoolVarP(&systemInstall, "system", "", false, "Set the Git LFS config in system-wide scope.") installCmd.Flags().BoolVarP(&skipSmudgeInstall, "skip-smudge", "s", false, "Skip automatic downloading of objects on clone or pull.") installCmd.AddCommand(installHooksCmd) RootCmd.AddCommand(installCmd) diff --git a/debian/postinst b/debian/postinst index 1d50c203..3c547bab 100644 --- a/debian/postinst +++ b/debian/postinst @@ -1,2 +1,2 @@ #!/bin/sh -git lfs install +git lfs install --system diff --git a/docs/man/git-lfs-install.1.ronn b/docs/man/git-lfs-install.1.ronn index 3b682d8b..685fcf92 100644 --- a/docs/man/git-lfs-install.1.ronn +++ b/docs/man/git-lfs-install.1.ronn @@ -23,7 +23,10 @@ filters if they are not already set. Sets the "lfs" smudge and clean filters, overwriting existing values. * `--local`: Sets the "lfs" smudge and clean filters in the local repository's git - config, instead of the global git config. + config, instead of the global git config (~/.gitconfig). +* `--system`: + Sets the "lfs" smudge and clean filters in the system git config, e.g. /etc/gitconfig + instead of the global git config (~/.gitconfig). * `--skip-smudge`: Skips automatic downloading of objects on clone or pull. This requires a manual "git lfs pull" every time a new commit is checked out on your diff --git a/lfs/attribute.go b/lfs/attribute.go index fbe0e2bd..5a589c40 100644 --- a/lfs/attribute.go +++ b/lfs/attribute.go @@ -2,7 +2,6 @@ package lfs import ( "fmt" - "os" "regexp" "strings" @@ -32,8 +31,9 @@ type Attribute struct { // InstallOptions serves as an argument to Install(). type InstallOptions struct { - Force bool - Local bool + Force bool + Local bool + System bool } // Install instructs Git to set all keys and values relative to the root @@ -67,26 +67,22 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { var currentValue string if opt.Local { currentValue = git.Config.FindLocal(key) + } else if opt.System { + currentValue = git.Config.FindSystem(key) } else { - if os.Geteuid() == 0 { - currentValue = git.Config.FindSystem(key) - } else { - currentValue = git.Config.FindGlobal(key) - } + currentValue = git.Config.FindGlobal(key) } if opt.Force || shouldReset(currentValue) { if opt.Local { git.Config.UnsetLocalKey("", key) git.Config.SetLocal("", key, value) + } else if opt.System { + git.Config.UnsetSystem(key) + git.Config.SetSystem(key, value) } else { - if os.Geteuid() == 0 { - git.Config.UnsetSystem(key) - git.Config.SetSystem(key, value) - } else { - git.Config.UnsetGlobal(key) - git.Config.SetGlobal(key, value) - } + git.Config.UnsetGlobal(key) + git.Config.SetGlobal(key, value) } return nil @@ -100,11 +96,9 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { // Uninstall removes all properties in the path of this property. func (a *Attribute) Uninstall() { - if os.Geteuid() == 0 { - git.Config.UnsetSystemSection(a.Section) - } else { - git.Config.UnsetGlobalSection(a.Section) - } + // uninstall from both system and global + git.Config.UnsetSystemSection(a.Section) + git.Config.UnsetGlobalSection(a.Section) } // shouldReset determines whether or not a value is resettable given its current diff --git a/rpm/SPECS/git-lfs.spec b/rpm/SPECS/git-lfs.spec index 8e05aee1..24e468cb 100644 --- a/rpm/SPECS/git-lfs.spec +++ b/rpm/SPECS/git-lfs.spec @@ -47,7 +47,7 @@ install -D man/*.1 ${RPM_BUILD_ROOT}/usr/share/man/man1 install -D man/*.5 ${RPM_BUILD_ROOT}/usr/share/man/man5 %post -git lfs install +git lfs install --system %preun git lfs uninstall From 77734205b82e409224858eb44aba458df143a5cc Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Wed, 27 Apr 2016 09:24:00 +1000 Subject: [PATCH 06/15] Stop SimpleExec swallowing errors. Fixed #1183. ExitError conditions now return a plain error with message like: Error running git [config --system filter.lfs.clean git-lfs clean -- %f]: 'error: could not lock config file /etc/gitconfig: Permission denied' 'exit status 255' Also: * Made Set/Unset Config commands return errors when they occur. * Made install attribute set fail properly with an error message e.g. if permission-denied. Previously this failed silently and reported success. ***NOTE*** this commit contains Go 1.6-specific APIs. The following commit will remove those, and that following commit can be reverted when Go >= 1.6 is required. --- git/git.go | 32 ++++++++++++++++---------------- lfs/attribute.go | 13 ++++++++----- subprocess/subprocess.go | 18 ++++++++++++------ 3 files changed, 36 insertions(+), 27 deletions(-) diff --git a/git/git.go b/git/git.go index a37967fb..e32e1ff6 100644 --- a/git/git.go +++ b/git/git.go @@ -304,55 +304,55 @@ func (c *gitConfig) FindLocal(val string) string { } // SetGlobal sets the git config value for the key in the global config -func (c *gitConfig) SetGlobal(key, val string) { - subprocess.SimpleExec("git", "config", "--global", key, val) +func (c *gitConfig) SetGlobal(key, val string) (string, error) { + return subprocess.SimpleExec("git", "config", "--global", key, val) } // SetSystem sets the git config value for the key in the system config -func (c *gitConfig) SetSystem(key, val string) { - subprocess.SimpleExec("git", "config", "--system", key, val) +func (c *gitConfig) SetSystem(key, val string) (string, error) { + return subprocess.SimpleExec("git", "config", "--system", key, val) } // UnsetGlobal removes the git config value for the key from the global config -func (c *gitConfig) UnsetGlobal(key string) { - subprocess.SimpleExec("git", "config", "--global", "--unset", key) +func (c *gitConfig) UnsetGlobal(key string) (string, error) { + return subprocess.SimpleExec("git", "config", "--global", "--unset", key) } // UnsetSystem removes the git config value for the key from the system config -func (c *gitConfig) UnsetSystem(key string) { - subprocess.SimpleExec("git", "config", "--system", "--unset", key) +func (c *gitConfig) UnsetSystem(key string) (string, error) { + return subprocess.SimpleExec("git", "config", "--system", "--unset", key) } // UnsetGlobalSection removes the entire named section from the global config -func (c *gitConfig) UnsetGlobalSection(key string) { - subprocess.SimpleExec("git", "config", "--global", "--remove-section", key) +func (c *gitConfig) UnsetGlobalSection(key string) (string, error) { + return subprocess.SimpleExec("git", "config", "--global", "--remove-section", key) } // UnsetGlobalSection removes the entire named section from the system config -func (c *gitConfig) UnsetSystemSection(key string) { - subprocess.SimpleExec("git", "config", "--system", "--remove-section", key) +func (c *gitConfig) UnsetSystemSection(key string) (string, error) { + return subprocess.SimpleExec("git", "config", "--system", "--remove-section", key) } // SetLocal sets the git config value for the key in the specified config file -func (c *gitConfig) SetLocal(file, key, val string) { +func (c *gitConfig) SetLocal(file, key, val string) (string, error) { args := make([]string, 1, 5) args[0] = "config" if len(file) > 0 { args = append(args, "--file", file) } args = append(args, key, val) - subprocess.SimpleExec("git", args...) + return subprocess.SimpleExec("git", args...) } // UnsetLocalKey removes the git config value for the key from the specified config file -func (c *gitConfig) UnsetLocalKey(file, key string) { +func (c *gitConfig) UnsetLocalKey(file, key string) (string, error) { args := make([]string, 1, 5) args[0] = "config" if len(file) > 0 { args = append(args, "--file", file) } args = append(args, "--unset", key) - subprocess.SimpleExec("git", args...) + return subprocess.SimpleExec("git", args...) } // List lists all of the git config values diff --git a/lfs/attribute.go b/lfs/attribute.go index 5a589c40..cef8d946 100644 --- a/lfs/attribute.go +++ b/lfs/attribute.go @@ -74,18 +74,21 @@ func (a *Attribute) set(key, value string, opt InstallOptions) error { } if opt.Force || shouldReset(currentValue) { + var err error if opt.Local { + // ignore error for unset, git returns non-zero if missing git.Config.UnsetLocalKey("", key) - git.Config.SetLocal("", key, value) + _, err = git.Config.SetLocal("", key, value) } else if opt.System { + // ignore error for unset, git returns non-zero if missing git.Config.UnsetSystem(key) - git.Config.SetSystem(key, value) + _, err = git.Config.SetSystem(key, value) } else { + // ignore error for unset, git returns non-zero if missing git.Config.UnsetGlobal(key) - git.Config.SetGlobal(key, value) + _, err = git.Config.SetGlobal(key, value) } - - return nil + return err } else if currentValue != value { return fmt.Errorf("The %s attribute should be \"%s\" but is \"%s\"", key, value, currentValue) diff --git a/subprocess/subprocess.go b/subprocess/subprocess.go index 826a3471..1f1e508a 100644 --- a/subprocess/subprocess.go +++ b/subprocess/subprocess.go @@ -17,14 +17,20 @@ func SimpleExec(name string, args ...string) (string, error) { cmd := ExecCommand(name, args...) output, err := cmd.Output() - if _, ok := err.(*exec.ExitError); ok { - return "", nil - } - if err != nil { - return fmt.Sprintf("Error running %s %s", name, args), err + if exitError, ok := err.(*exec.ExitError); ok { + errorOutput := strings.TrimSpace(string(exitError.Stderr)) + if errorOutput == "" { + // some commands might write nothing to stderr but something to stdout in error-conditions, in which case, we'll use that + // in the error string + errorOutput = strings.TrimSpace(string(output)) + } + formattedErr := fmt.Errorf("Error running %s %s: '%s' '%s'", name, args, errorOutput, strings.TrimSpace(exitError.Error())) + + // return "" as output in error case, for callers that don't care about errors but rely on "" returned, in-case stdout != "" + return "", formattedErr } - return strings.Trim(string(output), " \n"), nil + return strings.Trim(string(output), " \n"), err } // An env for an exec.Command without GIT_TRACE From d06e97b37db762847c34dc3f7dfdd212088084ca Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Fri, 29 Apr 2016 07:12:33 +1000 Subject: [PATCH 07/15] Removed Go 1.6-specific API changes introduced in the previous commit. This commit can be reverted as soon as Go >= 1.6 is required. This commit copies-down some utility methods introduced as part of the API change in Go 1.6. These changes are convenient to use here, since they auto-truncate stderr error output. Revert this commit when moving to Go 1.6. --- subprocess/subprocess.go | 96 +++++++++++++++++++++++++++++++++++++++- 1 file changed, 95 insertions(+), 1 deletion(-) diff --git a/subprocess/subprocess.go b/subprocess/subprocess.go index 1f1e508a..00ffe3c7 100644 --- a/subprocess/subprocess.go +++ b/subprocess/subprocess.go @@ -3,9 +3,11 @@ package subprocess import ( + "bytes" "fmt" "os" "os/exec" + "strconv" "strings" "github.com/rubyist/tracerx" @@ -16,9 +18,18 @@ func SimpleExec(name string, args ...string) (string, error) { tracerx.Printf("run_command: '%s' %s", name, strings.Join(args, " ")) cmd := ExecCommand(name, args...) + //start copied from Go 1.6 exec.go + captureErr := cmd.Stderr == nil + if captureErr { + cmd.Stderr = &prefixSuffixSaver{N: 32 << 10} + } + //end copied from Go 1.6 exec.go + output, err := cmd.Output() + if exitError, ok := err.(*exec.ExitError); ok { - errorOutput := strings.TrimSpace(string(exitError.Stderr)) + // TODO for min Go 1.6+, replace with ExitError.Stderr + errorOutput := strings.TrimSpace(string(cmd.Stderr.(*prefixSuffixSaver).Bytes())) if errorOutput == "" { // some commands might write nothing to stderr but something to stdout in error-conditions, in which case, we'll use that // in the error string @@ -48,3 +59,86 @@ func init() { env = append(env, kv) } } + +// remaining code in file copied from Go 1.6 (c4fa25f4fc8f4419d0b0707bcdae9199a745face) exec.go and can be removed if moving to Go 1.6 minimum. +// go 1.6 adds ExitError.Stderr with nice prefix/suffix trimming, which could replace cmd.Stderr above + +//start copied from Go 1.6 exec.go +// prefixSuffixSaver is an io.Writer which retains the first N bytes +// and the last N bytes written to it. The Bytes() methods reconstructs +// it with a pretty error message. +type prefixSuffixSaver struct { + N int // max size of prefix or suffix + prefix []byte + suffix []byte // ring buffer once len(suffix) == N + suffixOff int // offset to write into suffix + skipped int64 + + // TODO(bradfitz): we could keep one large []byte and use part of it for + // the prefix, reserve space for the '... Omitting N bytes ...' message, + // then the ring buffer suffix, and just rearrange the ring buffer + // suffix when Bytes() is called, but it doesn't seem worth it for + // now just for error messages. It's only ~64KB anyway. +} + +func (w *prefixSuffixSaver) Write(p []byte) (n int, err error) { + lenp := len(p) + p = w.fill(&w.prefix, p) + + // Only keep the last w.N bytes of suffix data. + if overage := len(p) - w.N; overage > 0 { + p = p[overage:] + w.skipped += int64(overage) + } + p = w.fill(&w.suffix, p) + + // w.suffix is full now if p is non-empty. Overwrite it in a circle. + for len(p) > 0 { // 0, 1, or 2 iterations. + n := copy(w.suffix[w.suffixOff:], p) + p = p[n:] + w.skipped += int64(n) + w.suffixOff += n + if w.suffixOff == w.N { + w.suffixOff = 0 + } + } + return lenp, nil +} + +// fill appends up to len(p) bytes of p to *dst, such that *dst does not +// grow larger than w.N. It returns the un-appended suffix of p. +func (w *prefixSuffixSaver) fill(dst *[]byte, p []byte) (pRemain []byte) { + if remain := w.N - len(*dst); remain > 0 { + add := minInt(len(p), remain) + *dst = append(*dst, p[:add]...) + p = p[add:] + } + return p +} + +func (w *prefixSuffixSaver) Bytes() []byte { + if w.suffix == nil { + return w.prefix + } + if w.skipped == 0 { + return append(w.prefix, w.suffix...) + } + var buf bytes.Buffer + buf.Grow(len(w.prefix) + len(w.suffix) + 50) + buf.Write(w.prefix) + buf.WriteString("\n... omitting ") + buf.WriteString(strconv.FormatInt(w.skipped, 10)) + buf.WriteString(" bytes ...\n") + buf.Write(w.suffix[w.suffixOff:]) + buf.Write(w.suffix[:w.suffixOff]) + return buf.Bytes() +} + +func minInt(a, b int) int { + if a < b { + return a + } + return b +} + +//end copied from Go 1.6 exec.go From df22c0aaeef7bffa856036f7114743faace44df3 Mon Sep 17 00:00:00 2001 From: Brett Randall Date: Fri, 29 Apr 2016 06:48:44 +1000 Subject: [PATCH 08/15] Improved error-reporting in ResolveRef. This is required when SimpleExec stops swallowing the error return. Related test "ls-files: with zero files". --- git/git.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git/git.go b/git/git.go index e32e1ff6..38730d2d 100644 --- a/git/git.go +++ b/git/git.go @@ -65,7 +65,7 @@ func LsRemote(remote, remoteRef string) (string, error) { func ResolveRef(ref string) (*Ref, error) { outp, err := subprocess.SimpleExec("git", "rev-parse", ref, "--symbolic-full-name", ref) if err != nil { - return nil, err + return nil, fmt.Errorf("Git can't resolve ref: %q", ref) } if outp == "" { return nil, fmt.Errorf("Git can't resolve ref: %q", ref) From d382f1f836d7eee7c6b63d0737bf825c5fab4a6c Mon Sep 17 00:00:00 2001 From: David Pursehouse Date: Sat, 13 Aug 2016 13:20:52 +0900 Subject: [PATCH 09/15] Fixes #1356: Don't show same error message twice Only show the error message if it's different from the inner message that was already shown. Change-Id: Iff591518853b1c5c229b676d08d8f914f1eb6f62 Signed-off-by: David Pursehouse --- commands/uploader.go | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/commands/uploader.go b/commands/uploader.go index 598d4603..b6a4f6ce 100644 --- a/commands/uploader.go +++ b/commands/uploader.go @@ -146,10 +146,15 @@ func upload(c *uploadContext, unfiltered []*lfs.WrappedPointer) { if Debugging || errutil.IsFatalError(err) { LoggedError(err, err.Error()) } else { + var innermsg string = "" if inner := errutil.GetInnerError(err); inner != nil { - Error(inner.Error()) + innermsg = inner.Error() + Error(innermsg) + } + var msg string = err.Error() + if msg != innermsg { + Error(msg) } - Error(err.Error()) } } From 334572329c898855ceb6ed5a4ca39da7e4fec8f0 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Tue, 16 Aug 2016 12:03:37 -0600 Subject: [PATCH 10/15] clean up duplicate error handling code --- commands/command_fetch.go | 2 +- commands/commands.go | 35 +++++++++++++++++++++++++++-------- commands/uploader.go | 14 +------------- 3 files changed, 29 insertions(+), 22 deletions(-) diff --git a/commands/command_fetch.go b/commands/command_fetch.go index 06b510f7..98007416 100644 --- a/commands/command_fetch.go +++ b/commands/command_fetch.go @@ -313,7 +313,7 @@ func fetchAndReportToChan(pointers []*lfs.WrappedPointer, include, exclude []str ok := true for _, err := range q.Errors() { ok = false - ExitWithError(err) + FullError(err) } return ok } diff --git a/commands/commands.go b/commands/commands.go index 626db30a..fe7bb070 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -105,17 +105,36 @@ func Exit(format string, args ...interface{}) { os.Exit(2) } +// ExitWithError either panics with a full stack trace for fatal errors, or +// simply prints the error message and exits immediately. func ExitWithError(err error) { - if Debugging || errutil.IsFatalError(err) { - Panic(err, err.Error()) - } else { - if inner := errutil.GetInnerError(err); inner != nil { - Error(inner.Error()) - } - Exit(err.Error()) - } + errorWith(err, Panic, Exit) } +// FullError prints either a full stack trace for fatal errors, or just the +// error message. +func FullError(err error) { + errorWith(err, LoggedError, Error) +} + +func errorWith(err error, fatalErrFn func(error, string, ...interface{}), errFn func(string, ...interface{})) { + var innermsg string + if inner := errutil.GetInnerError(err); inner != nil { + innermsg = inner.Error() + } + + errmsg := err.Error() + if errmsg != innermsg { + Error(innermsg) + } + + if Debugging || errutil.IsFatalError(err) { + fatalErrFn(err, errmsg) + } else { + errFn(errmsg) + } + } + // Debug prints a formatted message if debugging is enabled. The formatted // message also shows up in the panic log, if created. func Debug(format string, args ...interface{}) { diff --git a/commands/uploader.go b/commands/uploader.go index b6a4f6ce..da1e1f73 100644 --- a/commands/uploader.go +++ b/commands/uploader.go @@ -143,19 +143,7 @@ func upload(c *uploadContext, unfiltered []*lfs.WrappedPointer) { q.Wait() for _, err := range q.Errors() { - if Debugging || errutil.IsFatalError(err) { - LoggedError(err, err.Error()) - } else { - var innermsg string = "" - if inner := errutil.GetInnerError(err); inner != nil { - innermsg = inner.Error() - Error(innermsg) - } - var msg string = err.Error() - if msg != innermsg { - Error(msg) - } - } + FullError(err) } if len(q.Errors()) > 0 { From 46c17a501b9efd4426fe1c23aede259ae8c28e2a Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 17 Aug 2016 11:22:53 -0600 Subject: [PATCH 11/15] vendor: add github.com/pkg/errs --- glide.lock | 19 +- glide.yaml | 3 +- script/test | 9 +- script/vendor | 2 + vendor/github.com/pkg/errors/.gitignore | 24 ++ vendor/github.com/pkg/errors/.travis.yml | 10 + vendor/github.com/pkg/errors/LICENSE | 24 ++ vendor/github.com/pkg/errors/README.md | 50 ++++ vendor/github.com/pkg/errors/appveyor.yml | 32 ++ vendor/github.com/pkg/errors/errors.go | 211 +++++++++++++ vendor/github.com/pkg/errors/errors_test.go | 159 ++++++++++ vendor/github.com/pkg/errors/example_test.go | 152 ++++++++++ vendor/github.com/pkg/errors/format_test.go | 141 +++++++++ vendor/github.com/pkg/errors/stack.go | 165 +++++++++++ vendor/github.com/pkg/errors/stack_test.go | 295 +++++++++++++++++++ 15 files changed, 1288 insertions(+), 8 deletions(-) create mode 100644 vendor/github.com/pkg/errors/.gitignore create mode 100644 vendor/github.com/pkg/errors/.travis.yml create mode 100644 vendor/github.com/pkg/errors/LICENSE create mode 100644 vendor/github.com/pkg/errors/README.md create mode 100644 vendor/github.com/pkg/errors/appveyor.yml create mode 100644 vendor/github.com/pkg/errors/errors.go create mode 100644 vendor/github.com/pkg/errors/errors_test.go create mode 100644 vendor/github.com/pkg/errors/example_test.go create mode 100644 vendor/github.com/pkg/errors/format_test.go create mode 100644 vendor/github.com/pkg/errors/stack.go create mode 100644 vendor/github.com/pkg/errors/stack_test.go diff --git a/glide.lock b/glide.lock index 4eaaa5a9..157502c0 100644 --- a/glide.lock +++ b/glide.lock @@ -1,5 +1,5 @@ -hash: 275cba8ff30fe108a79618e8ad803f600136572260dc78a7d879bd248b3bdbdb -updated: 2016-05-25T10:47:36.829238548-06:00 +hash: 8200892207101d0f4b474cd994ebb3da744f5e27037e11eaee1db6d8c2b0e559 +updated: 2016-08-17T11:25:21.691448231-06:00 imports: - name: github.com/bgentry/go-netrc version: 9fd32a8b3d3d3f9d43c341bfe098430e07609480 @@ -17,6 +17,8 @@ imports: version: 6807e777504f54ad073ecef66747de158294b639 - name: github.com/olekukonko/ts version: ecf753e7c962639ab5a1fb46f7da627d4c0a04b8 +- name: github.com/pkg/errors + version: 01fa4104b9c248c8945d14d9f128454d5b28d595 - name: github.com/rubyist/tracerx version: d7bcc0bc315bed2a841841bee5dbecc8d7d7582f - name: github.com/spf13/cobra @@ -25,16 +27,27 @@ imports: version: 580b9be06c33d8ba9dcc8757ea56b7642472c2f5 - name: github.com/stretchr/testify version: 6cb3b85ef5a0efef77caef88363ec4d4b5c0976d + subpackages: + - assert - name: github.com/technoweenie/go-contentaddressable version: 38171def3cd15e3b76eb156219b3d48704643899 - name: github.com/ThomsonReutersEikon/go-ntlm version: b00ec39bbdd04f845950f4dbb4fd0a2c3155e830 subpackages: - ntlm + - ntlm/md4 - name: github.com/xeipuuv/gojsonpointer version: e0fe6f68307607d540ed8eac07a342c33fa1b54a - name: github.com/xeipuuv/gojsonreference version: e02fc20de94c78484cd5ffb007f8af96be030a45 - name: github.com/xeipuuv/gojsonschema version: d5336c75940ef31c9ceeb0ae64cf92944bccb4ee -devImports: [] +testImports: +- name: github.com/davecgh/go-spew + version: 5215b55f46b2b919f50a1df0eaa5886afe4e3b3d + subpackages: + - spew +- name: github.com/pmezard/go-difflib + version: d8ed2627bdf02c080bf22230dbb337003b7aba2d + subpackages: + - difflib diff --git a/glide.yaml b/glide.yaml index 671f084c..c6d77c15 100644 --- a/glide.yaml +++ b/glide.yaml @@ -26,7 +26,6 @@ import: version: 6cb3b85ef5a0efef77caef88363ec4d4b5c0976d - package: github.com/ThomsonReutersEikon/go-ntlm version: b00ec39bbdd04f845950f4dbb4fd0a2c3155e830 - # go-ntlm includes a util/ directory, which was removed subpackages: - ntlm - package: github.com/technoweenie/go-contentaddressable @@ -37,3 +36,5 @@ import: version: e02fc20de94c78484cd5ffb007f8af96be030a45 - package: github.com/xeipuuv/gojsonschema version: d5336c75940ef31c9ceeb0ae64cf92944bccb4ee +- package: github.com/pkg/errors + version: ^0.7.0 diff --git a/script/test b/script/test index beaddbcb..55a8aed1 100755 --- a/script/test +++ b/script/test @@ -10,11 +10,12 @@ else # debian/rules variable DH_GOLANG_EXCLUDES, so update those when adding here. GO15VENDOREXPERIMENT=1 go test \ $(GO15VENDOREXPERIMENT=1 go list ./... \ - | grep -v "github.com/olekukonko/ts" \ - | grep -v "github.com/xeipuuv/gojsonreference" \ - | grep -v "github.com/xeipuuv/gojsonschema" \ - | grep -v "github.com/technoweenie/go-contentaddressable" \ | grep -v "github.com/kr/pty" \ | grep -v "github.com/kr/text" \ + | grep -v "github.com/olekukonko/ts" \ + | grep -v "github.com/stretchr/testify" \ + | grep -v "github.com/technoweenie/go-contentaddressable" \ + | grep -v "github.com/xeipuuv/gojsonreference" \ + | grep -v "github.com/xeipuuv/gojsonschema" \ ) fi diff --git a/script/vendor b/script/vendor index 6c946802..8e13269d 100755 --- a/script/vendor +++ b/script/vendor @@ -3,3 +3,5 @@ glide update -s -u glide install -s -u rm -rf vendor/github.com/ThomsonReutersEikon/go-ntlm/utils +rm -rf vendor/github.com/davecgh/go-spew +rm -rf vendor/github.com/pmezard/go-difflib diff --git a/vendor/github.com/pkg/errors/.gitignore b/vendor/github.com/pkg/errors/.gitignore new file mode 100644 index 00000000..daf913b1 --- /dev/null +++ b/vendor/github.com/pkg/errors/.gitignore @@ -0,0 +1,24 @@ +# Compiled Object files, Static and Dynamic libs (Shared Objects) +*.o +*.a +*.so + +# Folders +_obj +_test + +# Architecture specific extensions/prefixes +*.[568vq] +[568vq].out + +*.cgo1.go +*.cgo2.c +_cgo_defun.c +_cgo_gotypes.go +_cgo_export.* + +_testmain.go + +*.exe +*.test +*.prof diff --git a/vendor/github.com/pkg/errors/.travis.yml b/vendor/github.com/pkg/errors/.travis.yml new file mode 100644 index 00000000..024e2846 --- /dev/null +++ b/vendor/github.com/pkg/errors/.travis.yml @@ -0,0 +1,10 @@ +language: go +go_import_path: github.com/pkg/errors +go: + - 1.4.3 + - 1.5.4 + - 1.6.2 + - tip + +script: + - go test -v ./... diff --git a/vendor/github.com/pkg/errors/LICENSE b/vendor/github.com/pkg/errors/LICENSE new file mode 100644 index 00000000..fafcaafd --- /dev/null +++ b/vendor/github.com/pkg/errors/LICENSE @@ -0,0 +1,24 @@ +Copyright (c) 2015, Dave Cheney +All rights reserved. + +Redistribution and use in source and binary forms, with or without +modification, are permitted provided that the following conditions are met: + +* Redistributions of source code must retain the above copyright notice, this + list of conditions and the following disclaimer. + +* Redistributions in binary form must reproduce the above copyright notice, + this list of conditions and the following disclaimer in the documentation + and/or other materials provided with the distribution. + +THIS SOFTWARE IS PROVIDED BY THE COPYRIGHT HOLDERS AND CONTRIBUTORS "AS IS" +AND ANY EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE +IMPLIED WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE +DISCLAIMED. IN NO EVENT SHALL THE COPYRIGHT HOLDER OR CONTRIBUTORS BE LIABLE +FOR ANY DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL +DAMAGES (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR +SERVICES; LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER +CAUSED AND ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, +OR TORT (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE +OF THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE. + diff --git a/vendor/github.com/pkg/errors/README.md b/vendor/github.com/pkg/errors/README.md new file mode 100644 index 00000000..6ea6422e --- /dev/null +++ b/vendor/github.com/pkg/errors/README.md @@ -0,0 +1,50 @@ +# errors [![Travis-CI](https://travis-ci.org/pkg/errors.svg)](https://travis-ci.org/pkg/errors) [![AppVeyor](https://ci.appveyor.com/api/projects/status/b98mptawhudj53ep/branch/master?svg=true)](https://ci.appveyor.com/project/davecheney/errors/branch/master) [![GoDoc](https://godoc.org/github.com/pkg/errors?status.svg)](http://godoc.org/github.com/pkg/errors) [![Report card](https://goreportcard.com/badge/github.com/pkg/errors)](https://goreportcard.com/report/github.com/pkg/errors) + +Package errors provides simple error handling primitives. + +The traditional error handling idiom in Go is roughly akin to +```go +if err != nil { + return err +} +``` +which applied recursively up the call stack results in error reports without context or debugging information. The errors package allows programmers to add context to the failure path in their code in a way that does not destroy the original value of the error. + +## Adding context to an error + +The errors.Wrap function returns a new error that adds context to the original error. For example +```go +_, err := ioutil.ReadAll(r) +if err != nil { + return errors.Wrap(err, "read failed") +} +``` +## Retrieving the cause of an error + +Using `errors.Wrap` constructs a stack of errors, adding context to the preceding error. Depending on the nature of the error it may be necessary to reverse the operation of errors.Wrap to retrieve the original error for inspection. Any error value which implements this interface can be inspected by `errors.Cause`. +```go +type causer interface { + Cause() error +} +``` +`errors.Cause` will recursively retrieve the topmost error which does not implement `causer`, which is assumed to be the original cause. For example: +```go +switch err := errors.Cause(err).(type) { +case *MyError: + // handle specifically +default: + // unknown error +} +``` + +[Read the package documentation for more information](https://godoc.org/github.com/pkg/errors). + +## Contributing + +We welcome pull requests, bug fixes and issue reports. With that said, the bar for adding new symbols to this package is intentionally set high. + +Before proposing a change, please discuss your change by raising an issue. + +## Licence + +BSD-2-Clause diff --git a/vendor/github.com/pkg/errors/appveyor.yml b/vendor/github.com/pkg/errors/appveyor.yml new file mode 100644 index 00000000..a932eade --- /dev/null +++ b/vendor/github.com/pkg/errors/appveyor.yml @@ -0,0 +1,32 @@ +version: build-{build}.{branch} + +clone_folder: C:\gopath\src\github.com\pkg\errors +shallow_clone: true # for startup speed + +environment: + GOPATH: C:\gopath + +platform: + - x64 + +# http://www.appveyor.com/docs/installed-software +install: + # some helpful output for debugging builds + - go version + - go env + # pre-installed MinGW at C:\MinGW is 32bit only + # but MSYS2 at C:\msys64 has mingw64 + - set PATH=C:\msys64\mingw64\bin;%PATH% + - gcc --version + - g++ --version + +build_script: + - go install -v ./... + +test_script: + - set PATH=C:\gopath\bin;%PATH% + - go test -v ./... + +#artifacts: +# - path: '%GOPATH%\bin\*.exe' +deploy: off diff --git a/vendor/github.com/pkg/errors/errors.go b/vendor/github.com/pkg/errors/errors.go new file mode 100644 index 00000000..65bf7a0f --- /dev/null +++ b/vendor/github.com/pkg/errors/errors.go @@ -0,0 +1,211 @@ +// Package errors provides simple error handling primitives. +// +// The traditional error handling idiom in Go is roughly akin to +// +// if err != nil { +// return err +// } +// +// which applied recursively up the call stack results in error reports +// without context or debugging information. The errors package allows +// programmers to add context to the failure path in their code in a way +// that does not destroy the original value of the error. +// +// Adding context to an error +// +// The errors.Wrap function returns a new error that adds context to the +// original error. For example +// +// _, err := ioutil.ReadAll(r) +// if err != nil { +// return errors.Wrap(err, "read failed") +// } +// +// Retrieving the cause of an error +// +// Using errors.Wrap constructs a stack of errors, adding context to the +// preceding error. Depending on the nature of the error it may be necessary +// to reverse the operation of errors.Wrap to retrieve the original error +// for inspection. Any error value which implements this interface +// +// type Causer interface { +// Cause() error +// } +// +// can be inspected by errors.Cause. errors.Cause will recursively retrieve +// the topmost error which does not implement causer, which is assumed to be +// the original cause. For example: +// +// switch err := errors.Cause(err).(type) { +// case *MyError: +// // handle specifically +// default: +// // unknown error +// } +// +// Formatted printing of errors +// +// All error values returned from this package implement fmt.Formatter and can +// be formatted by the fmt package. The following verbs are supported +// +// %s print the error. If the error has a Cause it will be +// printed recursively +// %v see %s +// %+v extended format. Each Frame of the error's StackTrace will +// be printed in detail. +// +// Retrieving the stack trace of an error or wrapper +// +// New, Errorf, Wrap, and Wrapf record a stack trace at the point they are +// invoked. This information can be retrieved with the following interface. +// +// type StackTrace interface { +// StackTrace() errors.StackTrace +// } +// +// Where errors.StackTrace is defined as +// +// type StackTrace []Frame +// +// The Frame type represents a call site in the stacktrace. Frame supports +// the fmt.Formatter interface that can be used for printing information about +// the stacktrace of this error. For example: +// +// if err, ok := err.(StackTrace); ok { +// for _, f := range err.StackTrace() { +// fmt.Printf("%+s:%d", f) +// } +// } +// +// See the documentation for Frame.Format for more details. +package errors + +import ( + "fmt" + "io" +) + +// _error is an error implementation returned by New and Errorf +// that implements its own fmt.Formatter. +type _error struct { + msg string + *stack +} + +func (e _error) Error() string { return e.msg } + +func (e _error) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + io.WriteString(s, e.msg) + fmt.Fprintf(s, "%+v", e.StackTrace()) + return + } + fallthrough + case 's': + io.WriteString(s, e.msg) + } +} + +// New returns an error with the supplied message. +func New(message string) error { + return _error{ + message, + callers(), + } +} + +// Errorf formats according to a format specifier and returns the string +// as a value that satisfies error. +func Errorf(format string, args ...interface{}) error { + return _error{ + fmt.Sprintf(format, args...), + callers(), + } +} + +type cause struct { + cause error + msg string +} + +func (c cause) Error() string { return fmt.Sprintf("%s: %v", c.msg, c.Cause()) } +func (c cause) Cause() error { return c.cause } + +// wrapper is an error implementation returned by Wrap and Wrapf +// that implements its own fmt.Formatter. +type wrapper struct { + cause + *stack +} + +func (w wrapper) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + if s.Flag('+') { + fmt.Fprintf(s, "%+v\n", w.Cause()) + fmt.Fprintf(s, "%+v: %s", w.StackTrace()[0], w.msg) + return + } + fallthrough + case 's': + io.WriteString(s, w.Error()) + } +} + +// Wrap returns an error annotating err with message. +// If err is nil, Wrap returns nil. +func Wrap(err error, message string) error { + if err == nil { + return nil + } + return wrapper{ + cause: cause{ + cause: err, + msg: message, + }, + stack: callers(), + } +} + +// Wrapf returns an error annotating err with the format specifier. +// If err is nil, Wrapf returns nil. +func Wrapf(err error, format string, args ...interface{}) error { + if err == nil { + return nil + } + return wrapper{ + cause: cause{ + cause: err, + msg: fmt.Sprintf(format, args...), + }, + stack: callers(), + } +} + +// Cause returns the underlying cause of the error, if possible. +// An error value has a cause if it implements the following +// interface: +// +// type Causer interface { +// Cause() error +// } +// +// If the error does not implement Cause, the original error will +// be returned. If the error is nil, nil will be returned without further +// investigation. +func Cause(err error) error { + type causer interface { + Cause() error + } + + for err != nil { + cause, ok := err.(causer) + if !ok { + break + } + err = cause.Cause() + } + return err +} diff --git a/vendor/github.com/pkg/errors/errors_test.go b/vendor/github.com/pkg/errors/errors_test.go new file mode 100644 index 00000000..11d45554 --- /dev/null +++ b/vendor/github.com/pkg/errors/errors_test.go @@ -0,0 +1,159 @@ +package errors + +import ( + "errors" + "fmt" + "io" + "reflect" + "testing" +) + +func TestNew(t *testing.T) { + tests := []struct { + err string + want error + }{ + {"", fmt.Errorf("")}, + {"foo", fmt.Errorf("foo")}, + {"foo", New("foo")}, + {"string with format specifiers: %v", errors.New("string with format specifiers: %v")}, + } + + for _, tt := range tests { + got := New(tt.err) + if got.Error() != tt.want.Error() { + t.Errorf("New.Error(): got: %q, want %q", got, tt.want) + } + } +} + +func TestWrapNil(t *testing.T) { + got := Wrap(nil, "no error") + if got != nil { + t.Errorf("Wrap(nil, \"no error\"): got %#v, expected nil", got) + } +} + +func TestWrap(t *testing.T) { + tests := []struct { + err error + message string + want string + }{ + {io.EOF, "read error", "read error: EOF"}, + {Wrap(io.EOF, "read error"), "client error", "client error: read error: EOF"}, + } + + for _, tt := range tests { + got := Wrap(tt.err, tt.message).Error() + if got != tt.want { + t.Errorf("Wrap(%v, %q): got: %v, want %v", tt.err, tt.message, got, tt.want) + } + } +} + +type nilError struct{} + +func (nilError) Error() string { return "nil error" } + +func TestCause(t *testing.T) { + x := New("error") + tests := []struct { + err error + want error + }{{ + // nil error is nil + err: nil, + want: nil, + }, { + // explicit nil error is nil + err: (error)(nil), + want: nil, + }, { + // typed nil is nil + err: (*nilError)(nil), + want: (*nilError)(nil), + }, { + // uncaused error is unaffected + err: io.EOF, + want: io.EOF, + }, { + // caused error returns cause + err: Wrap(io.EOF, "ignored"), + want: io.EOF, + }, { + err: x, // return from errors.New + want: x, + }} + + for i, tt := range tests { + got := Cause(tt.err) + if !reflect.DeepEqual(got, tt.want) { + t.Errorf("test %d: got %#v, want %#v", i+1, got, tt.want) + } + } +} + +func TestWrapfNil(t *testing.T) { + got := Wrapf(nil, "no error") + if got != nil { + t.Errorf("Wrapf(nil, \"no error\"): got %#v, expected nil", got) + } +} + +func TestWrapf(t *testing.T) { + tests := []struct { + err error + message string + want string + }{ + {io.EOF, "read error", "read error: EOF"}, + {Wrapf(io.EOF, "read error without format specifiers"), "client error", "client error: read error without format specifiers: EOF"}, + {Wrapf(io.EOF, "read error with %d format specifier", 1), "client error", "client error: read error with 1 format specifier: EOF"}, + } + + for _, tt := range tests { + got := Wrapf(tt.err, tt.message).Error() + if got != tt.want { + t.Errorf("Wrapf(%v, %q): got: %v, want %v", tt.err, tt.message, got, tt.want) + } + } +} + +func TestErrorf(t *testing.T) { + tests := []struct { + err error + want string + }{ + {Errorf("read error without format specifiers"), "read error without format specifiers"}, + {Errorf("read error with %d format specifier", 1), "read error with 1 format specifier"}, + } + + for _, tt := range tests { + got := tt.err.Error() + if got != tt.want { + t.Errorf("Errorf(%v): got: %q, want %q", tt.err, got, tt.want) + } + } +} + +// errors.New, etc values are not expected to be compared by value +// but the change in errors#27 made them incomparable. Assert that +// various kinds of errors have a functional equality operator, even +// if the result of that equality is always false. +func TestErrorEquality(t *testing.T) { + tests := []struct { + err1, err2 error + }{ + {io.EOF, io.EOF}, + {io.EOF, nil}, + {io.EOF, errors.New("EOF")}, + {io.EOF, New("EOF")}, + {New("EOF"), New("EOF")}, + {New("EOF"), Errorf("EOF")}, + {New("EOF"), Wrap(io.EOF, "EOF")}, + } + for _, tt := range tests { + _ = tt.err1 == tt.err2 // mustn't panic + } +} diff --git a/vendor/github.com/pkg/errors/example_test.go b/vendor/github.com/pkg/errors/example_test.go new file mode 100644 index 00000000..f1df2340 --- /dev/null +++ b/vendor/github.com/pkg/errors/example_test.go @@ -0,0 +1,152 @@ +package errors_test + +import ( + "fmt" + + "github.com/pkg/errors" +) + +func ExampleNew() { + err := errors.New("whoops") + fmt.Println(err) + + // Output: whoops +} + +func ExampleNew_printf() { + err := errors.New("whoops") + fmt.Printf("%+v", err) + + // Example output: + // whoops + // github.com/pkg/errors_test.ExampleNew_printf + // /home/dfc/src/github.com/pkg/errors/example_test.go:17 + // testing.runExample + // /home/dfc/go/src/testing/example.go:114 + // testing.RunExamples + // /home/dfc/go/src/testing/example.go:38 + // testing.(*M).Run + // /home/dfc/go/src/testing/testing.go:744 + // main.main + // /github.com/pkg/errors/_test/_testmain.go:106 + // runtime.main + // /home/dfc/go/src/runtime/proc.go:183 + // runtime.goexit + // /home/dfc/go/src/runtime/asm_amd64.s:2059 +} + +func ExampleWrap() { + cause := errors.New("whoops") + err := errors.Wrap(cause, "oh noes") + fmt.Println(err) + + // Output: oh noes: whoops +} + +func fn() error { + e1 := errors.New("error") + e2 := errors.Wrap(e1, "inner") + e3 := errors.Wrap(e2, "middle") + return errors.Wrap(e3, "outer") +} + +func ExampleCause() { + err := fn() + fmt.Println(err) + fmt.Println(errors.Cause(err)) + + // Output: outer: middle: inner: error + // error +} + +func ExampleWrap_extended() { + err := fn() + fmt.Printf("%+v\n", err) + + // Example output: + // error + // github.com/pkg/errors_test.fn + // /home/dfc/src/github.com/pkg/errors/example_test.go:47 + // github.com/pkg/errors_test.ExampleCause_printf + // /home/dfc/src/github.com/pkg/errors/example_test.go:63 + // testing.runExample + // /home/dfc/go/src/testing/example.go:114 + // testing.RunExamples + // /home/dfc/go/src/testing/example.go:38 + // testing.(*M).Run + // /home/dfc/go/src/testing/testing.go:744 + // main.main + // /github.com/pkg/errors/_test/_testmain.go:104 + // runtime.main + // /home/dfc/go/src/runtime/proc.go:183 + // runtime.goexit + // /home/dfc/go/src/runtime/asm_amd64.s:2059 + // github.com/pkg/errors_test.fn + // /home/dfc/src/github.com/pkg/errors/example_test.go:48: inner + // github.com/pkg/errors_test.fn + // /home/dfc/src/github.com/pkg/errors/example_test.go:49: middle + // github.com/pkg/errors_test.fn + // /home/dfc/src/github.com/pkg/errors/example_test.go:50: outer +} + +func ExampleWrapf() { + cause := errors.New("whoops") + err := errors.Wrapf(cause, "oh noes #%d", 2) + fmt.Println(err) + + // Output: oh noes #2: whoops +} + +func ExampleErrorf_extended() { + err := errors.Errorf("whoops: %s", "foo") + fmt.Printf("%+v", err) + + // Example output: + // whoops: foo + // github.com/pkg/errors_test.ExampleErrorf + // /home/dfc/src/github.com/pkg/errors/example_test.go:101 + // testing.runExample + // /home/dfc/go/src/testing/example.go:114 + // testing.RunExamples + // /home/dfc/go/src/testing/example.go:38 + // testing.(*M).Run + // /home/dfc/go/src/testing/testing.go:744 + // main.main + // /github.com/pkg/errors/_test/_testmain.go:102 + // runtime.main + // /home/dfc/go/src/runtime/proc.go:183 + // runtime.goexit + // /home/dfc/go/src/runtime/asm_amd64.s:2059 +} + +func Example_stacktrace() { + type StackTrace interface { + StackTrace() errors.StackTrace + } + + err, ok := errors.Cause(fn()).(StackTrace) + if !ok { + panic("oops, err does not implement StackTrace") + } + + st := err.StackTrace() + fmt.Printf("%+v", st[0:2]) // top two frames + + // Example output: + // github.com/pkg/errors_test.fn + // /home/dfc/src/github.com/pkg/errors/example_test.go:47 + // github.com/pkg/errors_test.Example_stacktrace + // /home/dfc/src/github.com/pkg/errors/example_test.go:127 +} + +func ExampleCause_printf() { + err := errors.Wrap(func() error { + return func() error { + return errors.Errorf("hello %s", fmt.Sprintf("world")) + }() + }(), "failed") + + fmt.Printf("%v", err) + + // Output: failed: hello world +} diff --git a/vendor/github.com/pkg/errors/format_test.go b/vendor/github.com/pkg/errors/format_test.go new file mode 100644 index 00000000..60806ae8 --- /dev/null +++ b/vendor/github.com/pkg/errors/format_test.go @@ -0,0 +1,141 @@ +package errors + +import ( + "fmt" + "io" + "regexp" + "strings" + "testing" +) + +func TestFormatNew(t *testing.T) { + tests := []struct { + error + format string + want string + }{{ + New("error"), + "%s", + "error", + }, { + New("error"), + "%v", + "error", + }, { + New("error"), + "%+v", + "error\n" + + "github.com/pkg/errors.TestFormatNew\n" + + "\t.+/github.com/pkg/errors/format_test.go:25", + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.error, tt.format, tt.want) + } +} + +func TestFormatErrorf(t *testing.T) { + tests := []struct { + error + format string + want string + }{{ + Errorf("%s", "error"), + "%s", + "error", + }, { + Errorf("%s", "error"), + "%v", + "error", + }, { + Errorf("%s", "error"), + "%+v", + "error\n" + + "github.com/pkg/errors.TestFormatErrorf\n" + + "\t.+/github.com/pkg/errors/format_test.go:51", + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.error, tt.format, tt.want) + } +} + +func TestFormatWrap(t *testing.T) { + tests := []struct { + error + format string + want string + }{{ + Wrap(New("error"), "error2"), + "%s", + "error2: error", + }, { + Wrap(New("error"), "error2"), + "%v", + "error2: error", + }, { + Wrap(New("error"), "error2"), + "%+v", + "error\n" + + "github.com/pkg/errors.TestFormatWrap\n" + + "\t.+/github.com/pkg/errors/format_test.go:77", + }, { + Wrap(io.EOF, "error"), + "%s", + "error: EOF", + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.error, tt.format, tt.want) + } +} + +func TestFormatWrapf(t *testing.T) { + tests := []struct { + error + format string + want string + }{{ + Wrapf(New("error"), "error%d", 2), + "%s", + "error2: error", + }, { + Wrap(io.EOF, "error"), + "%v", + "error: EOF", + }, { + Wrap(io.EOF, "error"), + "%+v", + "EOF\n" + + "github.com/pkg/errors.TestFormatWrapf\n" + + "\t.+/github.com/pkg/errors/format_test.go:107: error", + }, { + Wrapf(New("error"), "error%d", 2), + "%v", + "error2: error", + }, { + Wrapf(New("error"), "error%d", 2), + "%+v", + "error\n" + + "github.com/pkg/errors.TestFormatWrapf\n" + + "\t.+/github.com/pkg/errors/format_test.go:117", + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.error, tt.format, tt.want) + } +} + +func testFormatRegexp(t *testing.T, arg interface{}, format, want string) { + got := fmt.Sprintf(format, arg) + lines := strings.SplitN(got, "\n", -1) + for i, w := range strings.SplitN(want, "\n", -1) { + match, err := regexp.MatchString(w, lines[i]) + if err != nil { + t.Fatal(err) + } + if !match { + t.Errorf("fmt.Sprintf(%q, err): got: %q, want: %q", format, got, want) + } + } +} diff --git a/vendor/github.com/pkg/errors/stack.go b/vendor/github.com/pkg/errors/stack.go new file mode 100644 index 00000000..243a64a2 --- /dev/null +++ b/vendor/github.com/pkg/errors/stack.go @@ -0,0 +1,165 @@ +package errors + +import ( + "fmt" + "io" + "path" + "runtime" + "strings" +) + +// Frame represents a program counter inside a stack frame. +type Frame uintptr + +// pc returns the program counter for this frame; +// multiple frames may have the same PC value. +func (f Frame) pc() uintptr { return uintptr(f) - 1 } + +// file returns the full path to the file that contains the +// function for this Frame's pc. +func (f Frame) file() string { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return "unknown" + } + file, _ := fn.FileLine(f.pc()) + return file +} + +// line returns the line number of source code of the +// function for this Frame's pc. +func (f Frame) line() int { + fn := runtime.FuncForPC(f.pc()) + if fn == nil { + return 0 + } + _, line := fn.FileLine(f.pc()) + return line +} + +// Format formats the frame according to the fmt.Formatter interface. +// +// %s source file +// %d source line +// %n function name +// %v equivalent to %s:%d +// +// Format accepts flags that alter the printing of some verbs, as follows: +// +// %+s path of source file relative to the compile time GOPATH +// %+v equivalent to %+s:%d +func (f Frame) Format(s fmt.State, verb rune) { + switch verb { + case 's': + switch { + case s.Flag('+'): + pc := f.pc() + fn := runtime.FuncForPC(pc) + if fn == nil { + io.WriteString(s, "unknown") + } else { + file, _ := fn.FileLine(pc) + fmt.Fprintf(s, "%s\n\t%s", fn.Name(), file) + } + default: + io.WriteString(s, path.Base(f.file())) + } + case 'd': + fmt.Fprintf(s, "%d", f.line()) + case 'n': + name := runtime.FuncForPC(f.pc()).Name() + io.WriteString(s, funcname(name)) + case 'v': + f.Format(s, 's') + io.WriteString(s, ":") + f.Format(s, 'd') + } +} + +// StackTrace is stack of Frames from innermost (newest) to outermost (oldest). +type StackTrace []Frame + +func (st StackTrace) Format(s fmt.State, verb rune) { + switch verb { + case 'v': + switch { + case s.Flag('+'): + for _, f := range st { + fmt.Fprintf(s, "\n%+v", f) + } + case s.Flag('#'): + fmt.Fprintf(s, "%#v", []Frame(st)) + default: + fmt.Fprintf(s, "%v", []Frame(st)) + } + case 's': + fmt.Fprintf(s, "%s", []Frame(st)) + } +} + +// stack represents a stack of program counters. +type stack []uintptr + +func (s *stack) StackTrace() StackTrace { + f := make([]Frame, len(*s)) + for i := 0; i < len(f); i++ { + f[i] = Frame((*s)[i]) + } + return f +} + +func callers() *stack { + const depth = 32 + var pcs [depth]uintptr + n := runtime.Callers(3, pcs[:]) + var st stack = pcs[0:n] + return &st +} + +// funcname removes the path prefix component of a function's name reported by func.Name(). +func funcname(name string) string { + i := strings.LastIndex(name, "/") + name = name[i+1:] + i = strings.Index(name, ".") + return name[i+1:] +} + +func trimGOPATH(name, file string) string { + // Here we want to get the source file path relative to the compile time + // GOPATH. As of Go 1.6.x there is no direct way to know the compiled + // GOPATH at runtime, but we can infer the number of path segments in the + // GOPATH. We note that fn.Name() returns the function name qualified by + // the import path, which does not include the GOPATH. Thus we can trim + // segments from the beginning of the file path until the number of path + // separators remaining is one more than the number of path separators in + // the function name. For example, given: + // + // GOPATH /home/user + // file /home/user/src/pkg/sub/file.go + // fn.Name() pkg/sub.Type.Method + // + // We want to produce: + // + // pkg/sub/file.go + // + // From this we can easily see that fn.Name() has one less path separator + // than our desired output. We count separators from the end of the file + // path until it finds two more than in the function name and then move + // one character forward to preserve the initial path segment without a + // leading separator. + const sep = "/" + goal := strings.Count(name, sep) + 2 + i := len(file) + for n := 0; n < goal; n++ { + i = strings.LastIndex(file[:i], sep) + if i == -1 { + // not enough separators found, set i so that the slice expression + // below leaves file unmodified + i = -len(sep) + break + } + } + // get back to 0 or trim the leading separator + file = file[i+len(sep):] + return file +} diff --git a/vendor/github.com/pkg/errors/stack_test.go b/vendor/github.com/pkg/errors/stack_test.go new file mode 100644 index 00000000..c0488d53 --- /dev/null +++ b/vendor/github.com/pkg/errors/stack_test.go @@ -0,0 +1,295 @@ +package errors + +import ( + "fmt" + "runtime" + "testing" +) + +var initpc, _, _, _ = runtime.Caller(0) + +func TestFrameLine(t *testing.T) { + var tests = []struct { + Frame + want int + }{{ + Frame(initpc), + 9, + }, { + func() Frame { + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) + }(), + 20, + }, { + func() Frame { + var pc, _, _, _ = runtime.Caller(1) + return Frame(pc) + }(), + 28, + }, { + Frame(0), // invalid PC + 0, + }} + + for _, tt := range tests { + got := tt.Frame.line() + want := tt.want + if want != got { + t.Errorf("Frame(%v): want: %v, got: %v", uintptr(tt.Frame), want, got) + } + } +} + +type X struct{} + +func (x X) val() Frame { + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) +} + +func (x *X) ptr() Frame { + var pc, _, _, _ = runtime.Caller(0) + return Frame(pc) +} + +func TestFrameFormat(t *testing.T) { + var tests = []struct { + Frame + format string + want string + }{{ + Frame(initpc), + "%s", + "stack_test.go", + }, { + Frame(initpc), + "%+s", + "github.com/pkg/errors.init\n" + + "\t.+/github.com/pkg/errors/stack_test.go", + }, { + Frame(0), + "%s", + "unknown", + }, { + Frame(0), + "%+s", + "unknown", + }, { + Frame(initpc), + "%d", + "9", + }, { + Frame(0), + "%d", + "0", + }, { + Frame(initpc), + "%n", + "init", + }, { + func() Frame { + var x X + return x.ptr() + }(), + "%n", + `\(\*X\).ptr`, + }, { + func() Frame { + var x X + return x.val() + }(), + "%n", + "X.val", + }, { + Frame(0), + "%n", + "", + }, { + Frame(initpc), + "%v", + "stack_test.go:9", + }, { + Frame(initpc), + "%+v", + "github.com/pkg/errors.init\n" + + "\t.+/github.com/pkg/errors/stack_test.go:9", + }, { + Frame(0), + "%v", + "unknown:0", + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.Frame, tt.format, tt.want) + } +} + +func TestFuncname(t *testing.T) { + tests := []struct { + name, want string + }{ + {"", ""}, + {"runtime.main", "main"}, + {"github.com/pkg/errors.funcname", "funcname"}, + {"funcname", "funcname"}, + {"io.copyBuffer", "copyBuffer"}, + {"main.(*R).Write", "(*R).Write"}, + } + + for _, tt := range tests { + got := funcname(tt.name) + want := tt.want + if got != want { + t.Errorf("funcname(%q): want: %q, got %q", tt.name, want, got) + } + } +} + +func TestTrimGOPATH(t *testing.T) { + var tests = []struct { + Frame + want string + }{{ + Frame(initpc), + "github.com/pkg/errors/stack_test.go", + }} + + for _, tt := range tests { + pc := tt.Frame.pc() + fn := runtime.FuncForPC(pc) + file, _ := fn.FileLine(pc) + got := trimGOPATH(fn.Name(), file) + want := tt.want + if want != got { + t.Errorf("%v: want %q, got %q", tt.Frame, want, got) + } + } +} + +func TestStackTrace(t *testing.T) { + tests := []struct { + err error + want []string + }{{ + New("ooh"), []string{ + "github.com/pkg/errors.TestStackTrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:175", + }, + }, { + Wrap(New("ooh"), "ahh"), []string{ + "github.com/pkg/errors.TestStackTrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:180", // this is the stack of Wrap, not New + }, + }, { + Cause(Wrap(New("ooh"), "ahh")), []string{ + "github.com/pkg/errors.TestStackTrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:185", // this is the stack of New + }, + }, { + func() error { return New("ooh") }(), []string{ + `github.com/pkg/errors.(func·005|TestStackTrace.func1)` + + "\n\t.+/github.com/pkg/errors/stack_test.go:190", // this is the stack of New + "github.com/pkg/errors.TestStackTrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:190", // this is the stack of New's caller + }, + }, { + Cause(func() error { + return func() error { + return Errorf("hello %s", fmt.Sprintf("world")) + }() + }()), []string{ + `github.com/pkg/errors.(func·006|TestStackTrace.func2.1)` + + "\n\t.+/github.com/pkg/errors/stack_test.go:199", // this is the stack of Errorf + `github.com/pkg/errors.(func·007|TestStackTrace.func2)` + + "\n\t.+/github.com/pkg/errors/stack_test.go:200", // this is the stack of Errorf's caller + "github.com/pkg/errors.TestStackTrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:201", // this is the stack of Errorf's caller's caller + }, + }} + for _, tt := range tests { + x, ok := tt.err.(interface { + StackTrace() StackTrace + }) + if !ok { + t.Errorf("expected %#v to implement StackTrace() StackTrace", tt.err) + continue + } + st := x.StackTrace() + for j, want := range tt.want { + testFormatRegexp(t, st[j], "%+v", want) + } + } +} + +func stacktrace() StackTrace { + const depth = 8 + var pcs [depth]uintptr + n := runtime.Callers(1, pcs[:]) + var st stack = pcs[0:n] + return st.StackTrace() +} + +func TestStackTraceFormat(t *testing.T) { + tests := []struct { + StackTrace + format string + want string + }{{ + nil, + "%s", + `\[\]`, + }, { + nil, + "%v", + `\[\]`, + }, { + nil, + "%+v", + "", + }, { + nil, + "%#v", + `\[\]errors.Frame\(nil\)`, + }, { + make(StackTrace, 0), + "%s", + `\[\]`, + }, { + make(StackTrace, 0), + "%v", + `\[\]`, + }, { + make(StackTrace, 0), + "%+v", + "", + }, { + make(StackTrace, 0), + "%#v", + `\[\]errors.Frame{}`, + }, { + stacktrace()[:2], + "%s", + `\[stack_test.go stack_test.go\]`, + }, { + stacktrace()[:2], + "%v", + `\[stack_test.go:228 stack_test.go:275\]`, + }, { + stacktrace()[:2], + "%+v", + "\n" + + "github.com/pkg/errors.stacktrace\n" + + "\t.+/github.com/pkg/errors/stack_test.go:228\n" + + "github.com/pkg/errors.TestStackTraceFormat\n" + + "\t.+/github.com/pkg/errors/stack_test.go:279", + }, { + stacktrace()[:2], + "%#v", + `\[\]errors.Frame{stack_test.go:228, stack_test.go:287}`, + }} + + for _, tt := range tests { + testFormatRegexp(t, tt.StackTrace, tt.format, tt.want) + } +} From 09a2c4336be0cfe63679bfbf4033bcda1ebe1957 Mon Sep 17 00:00:00 2001 From: Taylor Blau Date: Wed, 17 Aug 2016 15:12:46 -0600 Subject: [PATCH 12/15] script/test: skip github.com/pkg/errors stack_test.go fails due to being run in a vendored environment --- script/test | 1 + 1 file changed, 1 insertion(+) diff --git a/script/test b/script/test index 55a8aed1..60212e05 100755 --- a/script/test +++ b/script/test @@ -13,6 +13,7 @@ else | grep -v "github.com/kr/pty" \ | grep -v "github.com/kr/text" \ | grep -v "github.com/olekukonko/ts" \ + | grep -v "github.com/pkg/errors" \ | grep -v "github.com/stretchr/testify" \ | grep -v "github.com/technoweenie/go-contentaddressable" \ | grep -v "github.com/xeipuuv/gojsonreference" \ From a343a11ddb8e3d3755590d1a3786831ccc18888b Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Wed, 17 Aug 2016 16:13:36 -0600 Subject: [PATCH 13/15] check the git version is ok in some key commands --- commands/command_clone.go | 1 + commands/command_install.go | 5 ++++- commands/command_pre_push.go | 2 ++ commands/command_pull.go | 1 + commands/command_push.go | 2 ++ commands/command_track.go | 2 ++ commands/command_update.go | 1 + commands/commands.go | 38 ++++++++++++++++++++++-------------- git/git.go | 16 ++++++++++++++- 9 files changed, 51 insertions(+), 17 deletions(-) diff --git a/commands/command_clone.go b/commands/command_clone.go index dd0a37bb..5729b586 100644 --- a/commands/command_clone.go +++ b/commands/command_clone.go @@ -19,6 +19,7 @@ var ( ) func cloneCommand(cmd *cobra.Command, args []string) { + requireGitVersion() // We pass all args to git clone err := git.CloneWithoutFilters(cloneFlags, args) diff --git a/commands/command_install.go b/commands/command_install.go index 01e7519b..95dc42dd 100644 --- a/commands/command_install.go +++ b/commands/command_install.go @@ -1,9 +1,10 @@ package commands import ( + "os" + "github.com/github/git-lfs/lfs" "github.com/spf13/cobra" - "os" ) var ( @@ -14,6 +15,8 @@ var ( ) func installCommand(cmd *cobra.Command, args []string) { + requireGitVersion() + if localInstall { requireInRepo() } diff --git a/commands/command_pre_push.go b/commands/command_pre_push.go index fafb96c8..af147349 100644 --- a/commands/command_pre_push.go +++ b/commands/command_pre_push.go @@ -43,6 +43,8 @@ func prePushCommand(cmd *cobra.Command, args []string) { os.Exit(1) } + requireGitVersion() + // Remote is first arg if err := git.ValidateRemote(args[0]); err != nil { Exit("Invalid remote name %q", args[0]) diff --git a/commands/command_pull.go b/commands/command_pull.go index 036686d3..2f862960 100644 --- a/commands/command_pull.go +++ b/commands/command_pull.go @@ -8,6 +8,7 @@ import ( ) func pullCommand(cmd *cobra.Command, args []string) { + requireGitVersion() requireInRepo() if len(args) > 0 { diff --git a/commands/command_push.go b/commands/command_push.go index 0754385d..36bc1dc9 100644 --- a/commands/command_push.go +++ b/commands/command_push.go @@ -113,6 +113,8 @@ func pushCommand(cmd *cobra.Command, args []string) { os.Exit(1) } + requireGitVersion() + // Remote is first arg if err := git.ValidateRemote(args[0]); err != nil { Exit("Invalid remote name %q", args[0]) diff --git a/commands/command_track.go b/commands/command_track.go index f6df1923..a5ae0196 100644 --- a/commands/command_track.go +++ b/commands/command_track.go @@ -26,6 +26,8 @@ var ( ) func trackCommand(cmd *cobra.Command, args []string) { + requireGitVersion() + if config.LocalGitDir == "" { Print("Not a git repository.") os.Exit(128) diff --git a/commands/command_update.go b/commands/command_update.go index 8a683260..3f1ab78e 100644 --- a/commands/command_update.go +++ b/commands/command_update.go @@ -16,6 +16,7 @@ var ( // updateCommand is used for updating parts of Git LFS that reside under // .git/lfs. func updateCommand(cmd *cobra.Command, args []string) { + requireGitVersion() requireInRepo() lfsAccessRE := regexp.MustCompile(`\Alfs\.(.*)\.access\z`) diff --git a/commands/commands.go b/commands/commands.go index fe7bb070..e87447d1 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -118,23 +118,23 @@ func FullError(err error) { } func errorWith(err error, fatalErrFn func(error, string, ...interface{}), errFn func(string, ...interface{})) { - var innermsg string - if inner := errutil.GetInnerError(err); inner != nil { - innermsg = inner.Error() - } - - errmsg := err.Error() - if errmsg != innermsg { - Error(innermsg) - } - - if Debugging || errutil.IsFatalError(err) { - fatalErrFn(err, errmsg) - } else { - errFn(errmsg) - } + var innermsg string + if inner := errutil.GetInnerError(err); inner != nil { + innermsg = inner.Error() } + errmsg := err.Error() + if errmsg != innermsg { + Error(innermsg) + } + + if Debugging || errutil.IsFatalError(err) { + fatalErrFn(err, errmsg) + } else { + errFn(errmsg) + } +} + // Debug prints a formatted message if debugging is enabled. The formatted // message also shows up in the panic log, if created. func Debug(format string, args ...interface{}) { @@ -334,6 +334,14 @@ func isCommandEnabled(cfg *config.Configuration, cmd string) bool { return cfg.Os.Bool(fmt.Sprintf("GITLFS%sENABLED", strings.ToUpper(cmd)), false) } +func requireGitVersion() { + minimumGit := "1.8.2" + gitver, _ := git.Config.Version() + if !git.Config.IsGitVersionAtLeast(minimumGit) { + Exit(fmt.Sprintf("git version >= %s is required for Git LFS, your version: %s", minimumGit, gitver)) + } +} + func init() { log.SetOutput(ErrorWriter) } diff --git a/git/git.go b/git/git.go index e4a10ca0..14322f24 100644 --- a/git/git.go +++ b/git/git.go @@ -14,6 +14,7 @@ import ( "regexp" "strconv" "strings" + "sync" "time" "github.com/github/git-lfs/subprocess" @@ -306,6 +307,8 @@ func UpdateIndex(file string) error { } type gitConfig struct { + gitVersion string + mu sync.Mutex } var Config = &gitConfig{} @@ -398,7 +401,18 @@ func (c *gitConfig) ListFromFile(f string) (string, error) { // Version returns the git version func (c *gitConfig) Version() (string, error) { - return subprocess.SimpleExec("git", "version") + c.mu.Lock() + defer c.mu.Unlock() + + if len(c.gitVersion) == 0 { + v, err := subprocess.SimpleExec("git", "version") + if err != nil { + return v, err + } + c.gitVersion = v + } + + return c.gitVersion, nil } // IsVersionAtLeast returns whether the git version is the one specified or higher From 3d5b99df30379586910eb5fdd8104684e6db8c12 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Wed, 17 Aug 2016 16:17:35 -0600 Subject: [PATCH 14/15] don't ignore a 'git version' error --- commands/commands.go | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/commands/commands.go b/commands/commands.go index e87447d1..1a18aa9d 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -336,9 +336,13 @@ func isCommandEnabled(cfg *config.Configuration, cmd string) bool { func requireGitVersion() { minimumGit := "1.8.2" - gitver, _ := git.Config.Version() + gitver, err := git.Config.Version() + if err != nil { + Exit("Error getting git version: %s", err) + } + if !git.Config.IsGitVersionAtLeast(minimumGit) { - Exit(fmt.Sprintf("git version >= %s is required for Git LFS, your version: %s", minimumGit, gitver)) + Exit("git version >= %s is required for Git LFS, your version: %s", minimumGit, gitver) } } From 18f863f2188a276341b32b46c8aba377ec944d99 Mon Sep 17 00:00:00 2001 From: risk danger olson Date: Wed, 17 Aug 2016 16:21:00 -0600 Subject: [PATCH 15/15] only re-run Version() if they don't match --- commands/commands.go | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/commands/commands.go b/commands/commands.go index 1a18aa9d..15bdf06c 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -336,12 +336,12 @@ func isCommandEnabled(cfg *config.Configuration, cmd string) bool { func requireGitVersion() { minimumGit := "1.8.2" - gitver, err := git.Config.Version() - if err != nil { - Exit("Error getting git version: %s", err) - } if !git.Config.IsGitVersionAtLeast(minimumGit) { + gitver, err := git.Config.Version() + if err != nil { + Exit("Error getting git version: %s", err) + } Exit("git version >= %s is required for Git LFS, your version: %s", minimumGit, gitver) } }