From e807aa319283aae4d35ddf5a07cffc6f52dda387 Mon Sep 17 00:00:00 2001 From: rick olson Date: Wed, 25 Oct 2017 20:23:43 -0600 Subject: [PATCH] git: remove static Config, keep Version() static --- commands/command_clone.go | 4 +- commands/command_env.go | 3 +- commands/command_fetch.go | 2 +- commands/command_pull.go | 2 +- commands/command_status.go | 2 +- commands/commands.go | 7 ++-- config/config.go | 12 +----- git/config.go | 30 -------------- git/git.go | 70 +++++++------------------------- git/git_test.go | 11 ++++-- git/version.go | 81 ++++++++++++++++++++++++++++++++++++++ 11 files changed, 115 insertions(+), 109 deletions(-) create mode 100644 git/version.go diff --git a/commands/command_clone.go b/commands/command_clone.go index b77885e1..80536261 100644 --- a/commands/command_clone.go +++ b/commands/command_clone.go @@ -23,7 +23,7 @@ var ( func cloneCommand(cmd *cobra.Command, args []string) { requireGitVersion() - if cfg.IsGitVersionAtLeast("2.15.0") { + if git.IsGitVersionAtLeast("2.15.0") { msg := []string{ "WARNING: 'git lfs clone' is deprecated and will not be updated", " with new flags from 'git clone'", @@ -110,7 +110,7 @@ func cloneCommand(cmd *cobra.Command, args []string) { func postCloneSubmodules(args []string) error { // In git 2.9+ the filter option will have been passed through to submodules // So we need to lfs pull inside each - if !cfg.IsGitVersionAtLeast("2.9.0") { + if !git.IsGitVersionAtLeast("2.9.0") { // In earlier versions submodules would have used smudge filter return nil } diff --git a/commands/command_env.go b/commands/command_env.go index 62ddf64c..cc1090ec 100644 --- a/commands/command_env.go +++ b/commands/command_env.go @@ -2,6 +2,7 @@ package commands import ( "github.com/git-lfs/git-lfs/config" + "github.com/git-lfs/git-lfs/git" "github.com/git-lfs/git-lfs/lfs" "github.com/spf13/cobra" ) @@ -10,7 +11,7 @@ func envCommand(cmd *cobra.Command, args []string) { config.ShowConfigWarnings = true endpoint := getAPIClient().Endpoints.Endpoint("download", cfg.CurrentRemote) - gitV, err := cfg.GitVersion() + gitV, err := git.Version() if err != nil { gitV = "Error getting git version: " + err.Error() } diff --git a/commands/command_fetch.go b/commands/command_fetch.go index fc9120ce..1026f42e 100644 --- a/commands/command_fetch.go +++ b/commands/command_fetch.go @@ -271,7 +271,7 @@ func fetchAndReportToChan(allpointers []*lfs.WrappedPointer, filter *filepathfil // Lazily initialize the current remote. if len(cfg.CurrentRemote) == 0 { // Actively find the default remote, don't just assume origin - defaultRemote, err := git.DefaultRemote() + defaultRemote, err := cfg.GitConfig().DefaultRemote() if err != nil { Exit("No default remote") } diff --git a/commands/command_pull.go b/commands/command_pull.go index be3c203c..2a5ee179 100644 --- a/commands/command_pull.go +++ b/commands/command_pull.go @@ -27,7 +27,7 @@ func pullCommand(cmd *cobra.Command, args []string) { remote = args[0] } else { // Actively find the default remote, don't just assume origin - defaultRemote, err := git.DefaultRemote() + defaultRemote, err := cfg.GitConfig().DefaultRemote() if err != nil { Panic(err, "No default remote") } diff --git a/commands/command_status.go b/commands/command_status.go index 59cba8b5..7a6831e9 100644 --- a/commands/command_status.go +++ b/commands/command_status.go @@ -210,7 +210,7 @@ func statusScanRefRange(ref *git.Ref) { Print("On branch %s", ref.Name) - remoteRef, err := git.CurrentRemoteRef() + remoteRef, err := cfg.GitConfig().CurrentRemoteRef() if err != nil { return } diff --git a/commands/commands.go b/commands/commands.go index 922cd09a..887b817f 100644 --- a/commands/commands.go +++ b/commands/commands.go @@ -16,6 +16,7 @@ import ( "github.com/git-lfs/git-lfs/config" "github.com/git-lfs/git-lfs/errors" "github.com/git-lfs/git-lfs/filepathfilter" + "github.com/git-lfs/git-lfs/git" "github.com/git-lfs/git-lfs/lfs" "github.com/git-lfs/git-lfs/lfsapi" "github.com/git-lfs/git-lfs/locking" @@ -378,7 +379,7 @@ func ipAddresses() []string { func logPanicToWriter(w io.Writer, loggedError error, le string) { // log the version - gitV, err := cfg.GitVersion() + gitV, err := git.Version() if err != nil { gitV = "Error getting git version: " + err.Error() } @@ -445,8 +446,8 @@ func buildProgressMeter(dryRun bool) *progress.ProgressMeter { func requireGitVersion() { minimumGit := "1.8.2" - if !cfg.IsGitVersionAtLeast(minimumGit) { - gitver, err := cfg.GitVersion() + if !git.IsGitVersionAtLeast(minimumGit) { + gitver, err := git.Version() if err != nil { Exit("Error getting git version: %s", err) } diff --git a/config/config.go b/config/config.go index eec668e6..4cb45197 100644 --- a/config/config.go +++ b/config/config.go @@ -108,7 +108,7 @@ func NewFrom(v Values) *Configuration { c := &Configuration{ CurrentRemote: defaultRemote, Os: EnvironmentOf(mapFetcher(v.Os)), - gitConfig: git.Config, + gitConfig: git.NewConfig("", ""), } c.Git = &delayedEnvironment{ callback: func() Environment { @@ -175,7 +175,7 @@ func (c *Configuration) SetLockableFilesReadOnly() bool { } func (c *Configuration) HookDir() string { - if c.gitConfig.IsGitVersionAtLeast("2.9.0") { + if git.IsGitVersionAtLeast("2.9.0") { hp, ok := c.Git.Get("core.hooksPath") if ok { return hp @@ -284,14 +284,6 @@ func (c *Configuration) GitConfig() *git.Configuration { return c.gitConfig } -func (c *Configuration) GitVersion() (string, error) { - return c.gitConfig.Version() -} - -func (c *Configuration) IsGitVersionAtLeast(ver string) bool { - return c.gitConfig.IsGitVersionAtLeast(ver) -} - func (c *Configuration) FindGitGlobalKey(key string) string { return c.gitConfig.FindGlobal(key) } diff --git a/git/config.go b/git/config.go index 90db1e2e..cc937789 100644 --- a/git/config.go +++ b/git/config.go @@ -7,11 +7,8 @@ import ( "sync" "github.com/git-lfs/git-lfs/subprocess" - "github.com/rubyist/tracerx" ) -var Config = &Configuration{} - // Configuration can fetch or modify the current Git config and track the Git // version. type Configuration struct { @@ -138,33 +135,6 @@ func (c *Configuration) Source() (*ConfigurationSource, error) { return ParseConfigLines(out, false), nil } -// Version returns the git version -func (c *Configuration) Version() (string, error) { - c.mu.Lock() - defer c.mu.Unlock() - - if c.version == nil { - v, err := gitSimple("version") - c.version = &v - if err != nil { - return v, err - } - } - - return *c.version, nil -} - -// IsVersionAtLeast returns whether the git version is the one specified or higher -// argument is plain version string separated by '.' e.g. "2.3.1" but can omit minor/patch -func (c *Configuration) IsGitVersionAtLeast(ver string) bool { - gitver, err := c.Version() - if err != nil { - tracerx.Printf("Error getting git version: %v", err) - return false - } - return IsVersionAtLeast(gitver, ver) -} - func (c *Configuration) gitConfig(args ...string) (string, error) { args = append([]string{"config"}, args...) subprocess.Trace("git", args...) diff --git a/git/git.go b/git/git.go index a92dad09..983a0834 100644 --- a/git/git.go +++ b/git/git.go @@ -94,7 +94,7 @@ func gitConfigNoLFS(args ...string) []string { // causes difficult issues with passing through Stdin for login prompts // This way is simpler & more practical. filterOverride := "" - if !Config.IsGitVersionAtLeast("2.8.0") { + if !IsGitVersionAtLeast("2.8.0") { filterOverride = "cat" } @@ -233,8 +233,8 @@ func CurrentRef() (*Ref, error) { return ResolveRef("HEAD") } -func CurrentRemoteRef() (*Ref, error) { - remoteref, err := RemoteRefNameForCurrentBranch() +func (c *Configuration) CurrentRemoteRef() (*Ref, error) { + remoteref, err := c.RemoteRefNameForCurrentBranch() if err != nil { return nil, err } @@ -243,12 +243,12 @@ func CurrentRemoteRef() (*Ref, error) { } // RemoteForCurrentBranch returns the name of the remote that the current branch is tracking -func RemoteForCurrentBranch() (string, error) { +func (c *Configuration) RemoteForCurrentBranch() (string, error) { ref, err := CurrentRef() if err != nil { return "", err } - remote := RemoteForBranch(ref.Name) + remote := c.RemoteForBranch(ref.Name) if remote == "" { return "", fmt.Errorf("remote not found for branch %q", ref.Name) } @@ -257,7 +257,7 @@ func RemoteForCurrentBranch() (string, error) { // RemoteRefForCurrentBranch returns the full remote ref (refs/remotes/{remote}/{remotebranch}) // that the current branch is tracking. -func RemoteRefNameForCurrentBranch() (string, error) { +func (c *Configuration) RemoteRefNameForCurrentBranch() (string, error) { ref, err := CurrentRef() if err != nil { return "", err @@ -267,26 +267,26 @@ func RemoteRefNameForCurrentBranch() (string, error) { return "", errors.New("not on a branch") } - remote := RemoteForBranch(ref.Name) + remote := c.RemoteForBranch(ref.Name) if remote == "" { return "", fmt.Errorf("remote not found for branch %q", ref.Name) } - remotebranch := RemoteBranchForLocalBranch(ref.Name) + remotebranch := c.RemoteBranchForLocalBranch(ref.Name) return fmt.Sprintf("refs/remotes/%s/%s", remote, remotebranch), nil } // RemoteForBranch returns the remote name that a given local branch is tracking (blank if none) -func RemoteForBranch(localBranch string) string { - return Config.Find(fmt.Sprintf("branch.%s.remote", localBranch)) +func (c *Configuration) RemoteForBranch(localBranch string) string { + return c.Find(fmt.Sprintf("branch.%s.remote", localBranch)) } // RemoteBranchForLocalBranch returns the name (only) of the remote branch that the local branch is tracking // If no specific branch is configured, returns local branch name -func RemoteBranchForLocalBranch(localBranch string) string { +func (c *Configuration) RemoteBranchForLocalBranch(localBranch string) string { // get remote ref to track, may not be same name - merge := Config.Find(fmt.Sprintf("branch.%s.merge", localBranch)) + merge := c.Find(fmt.Sprintf("branch.%s.merge", localBranch)) if strings.HasPrefix(merge, "refs/heads/") { return merge[11:] } else { @@ -429,8 +429,8 @@ func ValidateRemoteURL(remote string) error { // 3. Any other SINGLE remote defined in .git/config // Returns an error if all of these fail, i.e. no tracked remote branch, no // "origin", and either no remotes defined or 2+ non-"origin" remotes -func DefaultRemote() (string, error) { - tracked, err := RemoteForCurrentBranch() +func (c *Configuration) DefaultRemote() (string, error) { + tracked, err := c.RemoteForCurrentBranch() if err == nil { return tracked, nil } @@ -730,48 +730,6 @@ func parseRefFile(filename string) (*Ref, error) { return ResolveRef(contents) } -// IsVersionAtLeast compares 2 version strings (ok to be prefixed with 'git version', ignores) -func IsVersionAtLeast(actualVersion, desiredVersion string) bool { - // Capture 1-3 version digits, optionally prefixed with 'git version' and possibly - // with suffixes which we'll ignore (e.g. unstable builds, MinGW versions) - verregex := regexp.MustCompile(`(?:git version\s+)?(\d+)(?:.(\d+))?(?:.(\d+))?.*`) - - var atleast uint64 - // Support up to 1000 in major/minor/patch digits - const majorscale = 1000 * 1000 - const minorscale = 1000 - - if match := verregex.FindStringSubmatch(desiredVersion); match != nil { - // Ignore errors as regex won't match anything other than digits - major, _ := strconv.Atoi(match[1]) - atleast += uint64(major * majorscale) - if len(match) > 2 { - minor, _ := strconv.Atoi(match[2]) - atleast += uint64(minor * minorscale) - } - if len(match) > 3 { - patch, _ := strconv.Atoi(match[3]) - atleast += uint64(patch) - } - } - - var actual uint64 - if match := verregex.FindStringSubmatch(actualVersion); match != nil { - major, _ := strconv.Atoi(match[1]) - actual += uint64(major * majorscale) - if len(match) > 2 { - minor, _ := strconv.Atoi(match[2]) - actual += uint64(minor * minorscale) - } - if len(match) > 3 { - patch, _ := strconv.Atoi(match[3]) - actual += uint64(patch) - } - } - - return actual >= atleast -} - // IsBare returns whether or not a repository is bare. It requires that the // current working directory is a repository. // diff --git a/git/git_test.go b/git/git_test.go index ee679ff6..e2e9a8d3 100644 --- a/git/git_test.go +++ b/git/git_test.go @@ -48,8 +48,11 @@ func TestCurrentRefAndCurrentRemoteRef(t *testing.T) { }, }, } + outputs := repo.AddCommits(inputs) + // last commit was on branch3 + gitConf := repo.GitConfig() ref, err := CurrentRef() assert.Nil(t, err) assert.Equal(t, &Ref{"branch3", RefTypeLocalBranch, outputs[3].Sha}, ref) @@ -60,15 +63,15 @@ func TestCurrentRefAndCurrentRemoteRef(t *testing.T) { // Check remote repo.AddRemote("origin") test.RunGitCommand(t, true, "push", "-u", "origin", "master:someremotebranch") - ref, err = CurrentRemoteRef() + ref, err = gitConf.CurrentRemoteRef() assert.Nil(t, err) assert.Equal(t, &Ref{"origin/someremotebranch", RefTypeRemoteBranch, outputs[2].Sha}, ref) - refname, err := RemoteRefNameForCurrentBranch() + refname, err := gitConf.RemoteRefNameForCurrentBranch() assert.Nil(t, err) assert.Equal(t, "refs/remotes/origin/someremotebranch", refname) - remote, err := RemoteForCurrentBranch() + remote, err := gitConf.RemoteForCurrentBranch() assert.Nil(t, err) assert.Equal(t, "origin", remote) @@ -201,7 +204,7 @@ func TestResolveEmptyCurrentRef(t *testing.T) { func TestWorkTrees(t *testing.T) { // Only git 2.5+ - if !Config.IsGitVersionAtLeast("2.5.0") { + if !IsGitVersionAtLeast("2.5.0") { return } diff --git a/git/version.go b/git/version.go new file mode 100644 index 00000000..8f97e388 --- /dev/null +++ b/git/version.go @@ -0,0 +1,81 @@ +package git + +import ( + "regexp" + "strconv" + "sync" + + "github.com/git-lfs/git-lfs/subprocess" + "github.com/rubyist/tracerx" +) + +var ( + gitVersion *string + gitVersionMu sync.Mutex +) + +func Version() (string, error) { + gitVersionMu.Lock() + defer gitVersionMu.Unlock() + + if gitVersion == nil { + v, err := subprocess.SimpleExec("git", "version") + gitVersion = &v + return v, err + } + + return *gitVersion, nil +} + +// IsVersionAtLeast returns whether the git version is the one specified or higher +// argument is plain version string separated by '.' e.g. "2.3.1" but can omit minor/patch +func IsGitVersionAtLeast(ver string) bool { + gitver, err := Version() + if err != nil { + tracerx.Printf("Error getting git version: %v", err) + return false + } + return IsVersionAtLeast(gitver, ver) +} + +// IsVersionAtLeast compares 2 version strings (ok to be prefixed with 'git version', ignores) +func IsVersionAtLeast(actualVersion, desiredVersion string) bool { + // Capture 1-3 version digits, optionally prefixed with 'git version' and possibly + // with suffixes which we'll ignore (e.g. unstable builds, MinGW versions) + verregex := regexp.MustCompile(`(?:git version\s+)?(\d+)(?:.(\d+))?(?:.(\d+))?.*`) + + var atleast uint64 + // Support up to 1000 in major/minor/patch digits + const majorscale = 1000 * 1000 + const minorscale = 1000 + + if match := verregex.FindStringSubmatch(desiredVersion); match != nil { + // Ignore errors as regex won't match anything other than digits + major, _ := strconv.Atoi(match[1]) + atleast += uint64(major * majorscale) + if len(match) > 2 { + minor, _ := strconv.Atoi(match[2]) + atleast += uint64(minor * minorscale) + } + if len(match) > 3 { + patch, _ := strconv.Atoi(match[3]) + atleast += uint64(patch) + } + } + + var actual uint64 + if match := verregex.FindStringSubmatch(actualVersion); match != nil { + major, _ := strconv.Atoi(match[1]) + actual += uint64(major * majorscale) + if len(match) > 2 { + minor, _ := strconv.Atoi(match[2]) + actual += uint64(minor * minorscale) + } + if len(match) > 3 { + patch, _ := strconv.Atoi(match[3]) + actual += uint64(patch) + } + } + + return actual >= atleast +}