Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

check the git version is ok in some key commands #1461

Merged
merged 4 commits into from
Aug 18, 2016
Merged

check the git version is ok in some key commands #1461

Show file tree
Hide file tree
Changes from 3 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
a343a11
check the git version is ok in some key commands
technoweenie Aug 17, 2016
3d5b99d
don't ignore a 'git version' error
technoweenie Aug 17, 2016
18f863f
only re-run Version() if they don't match
technoweenie Aug 17, 2016
57a5bc8
Merge branch 'master' into check-git-version
technoweenie Aug 18, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
1 change: 1 addition & 0 deletions commands/command_clone.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
5 changes: 4 additions & 1 deletion commands/command_install.go
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,9 +1,10 @@
package commands

import (
"os"

"github.com/github/git-lfs/lfs"
"github.com/spf13/cobra"
"os"
)

var (
Expand All @@ -14,6 +15,8 @@ var (
)

func installCommand(cmd *cobra.Command, args []string) {
requireGitVersion()

if localInstall {
requireInRepo()
}
Expand Down
2 changes: 2 additions & 0 deletions commands/command_pre_push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
1 change: 1 addition & 0 deletions commands/command_pull.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ import (
)

func pullCommand(cmd *cobra.Command, args []string) {
requireGitVersion()
requireInRepo()

if len(args) > 0 {
Expand Down
2 changes: 2 additions & 0 deletions commands/command_push.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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])
Expand Down
2 changes: 2 additions & 0 deletions commands/command_track.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,8 @@ var (
)

func trackCommand(cmd *cobra.Command, args []string) {
requireGitVersion()

if config.LocalGitDir == "" {
Print("Not a git repository.")
os.Exit(128)
Expand Down
1 change: 1 addition & 0 deletions commands/command_update.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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`)
Expand Down
38 changes: 25 additions & 13 deletions commands/commands.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,22 +118,22 @@ 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()
}
var innermsg string
if inner := errutil.GetInnerError(err); inner != nil {
innermsg = inner.Error()
}

errmsg := err.Error()
if errmsg != innermsg {
Error(innermsg)
}
errmsg := err.Error()
if errmsg != innermsg {
Error(innermsg)
}

if Debugging || errutil.IsFatalError(err) {
fatalErrFn(err, errmsg)
} else {
errFn(errmsg)
}
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.
Expand Down Expand Up @@ -334,6 +334,18 @@ 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"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I wonder if it would be worth elevating this to a method argument, and having a const in the commands package.

This would allow us to easily require different versions of Git for different commands, but I'm not sure that's behavior we need.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is the absolute minimum requirement for Git LFS to run safely. We can add a const if something else needs this specific value, but nothing does right now. Other bits in the codebase that check the git version already use IsGitVersionAtLeast():

$ ack IsGitVersionAtLeast
commands/command_clone.go
89:     if !git.Config.IsGitVersionAtLeast("2.9.0") {

git/git.go
406:func (c *gitConfig) IsGitVersionAtLeast(ver string) bool {
790:    if !Config.IsGitVersionAtLeast("2.8.0") {

git/git_test.go
205:    if !Config.IsGitVersionAtLeast("2.5.0") {

lfs/hook.go
41:     customHooksSupported := git.Config.IsGitVersionAtLeast("2.9.0")

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah, this makes sense as an absolute min.


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)
}
}

func init() {
log.SetOutput(ErrorWriter)
}
16 changes: 15 additions & 1 deletion git/git.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@ import (
"regexp"
"strconv"
"strings"
"sync"
"time"

"github.com/github/git-lfs/subprocess"
Expand Down Expand Up @@ -306,6 +307,8 @@ func UpdateIndex(file string) error {
}

type gitConfig struct {
gitVersion string
mu sync.Mutex
}

var Config = &gitConfig{}
Expand Down Expand Up @@ -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
Expand Down