Merge pull request #2333 from Ryooooooga/push-force-if-includes

pkg/app/app.go

@@ -7,8 +7,6 @@ import (
 	"log"
 	"os"
 	"path/filepath"
-	"regexp"
-	"strconv"
 	"strings"
 
 	"github.com/go-errors/errors"
@@ -17,7 +15,6 @@ import (
 	appTypes "github.com/jesseduffield/lazygit/pkg/app/types"
 	"github.com/jesseduffield/lazygit/pkg/commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
-	"github.com/jesseduffield/lazygit/pkg/commands/git_config"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/common"
 	"github.com/jesseduffield/lazygit/pkg/config"
@@ -101,54 +98,36 @@ func NewApp(config config.AppConfigurer, common *common.Common) (*App, error) {
 		return app, err
 	}
 
+	gitVersion, err := app.validateGitVersion()
+	if err != nil {
+		return app, err
+	}
+
 	showRecentRepos, err := app.setupRepo()
 	if err != nil {
 		return app, err
 	}
 
-	gitConfig := git_config.NewStdCachedGitConfig(app.Log)
+	app.Gui, err = gui.NewGui(common, config, gitVersion, app.Updater, showRecentRepos, dirName)
-
-	app.Gui, err = gui.NewGui(common, config, gitConfig, app.Updater, showRecentRepos, dirName)
 	if err != nil {
 		return app, err
 	}
 	return app, nil
 }
 
-func (app *App) validateGitVersion() error {
+func (app *App) validateGitVersion() (*git_commands.GitVersion, error) {
-	output, err := app.OSCommand.Cmd.New("git --version").RunWithOutput()
+	version, err := git_commands.GetGitVersion(app.OSCommand)
 	// if we get an error anywhere here we'll show the same status
 	minVersionError := errors.New(app.Tr.MinGitVersionError)
 	if err != nil {
-		return minVersionError
+		return nil, minVersionError
 	}
 
-	if isGitVersionValid(output) {
+	if version.IsOlderThan(2, 0, 0) {
-		return nil
+		return nil, minVersionError
 	}
 
-	return minVersionError
+	return version, nil
-}
-
-func isGitVersionValid(versionStr string) bool {
-	// output should be something like: 'git version 2.23.0 (blah)'
-	re := regexp.MustCompile(`[^\d]+([\d\.]+)`)
-	matches := re.FindStringSubmatch(versionStr)
-
-	if len(matches) == 0 {
-		return false
-	}
-
-	gitVersion := matches[1]
-	majorVersion, err := strconv.Atoi(gitVersion[0:1])
-	if err != nil {
-		return false
-	}
-	if majorVersion < 2 {
-		return false
-	}
-
-	return true
 }
 
 func isDirectoryAGitRepository(dir string) (bool, error) {
@@ -169,10 +148,6 @@ func openRecentRepo(app *App) bool {
 }
 
 func (app *App) setupRepo() (bool, error) {
-	if err := app.validateGitVersion(); err != nil {
-		return false, err
-	}
-
 	if env.GetGitDirEnv() != "" {
 		// we've been given the git dir directly. We'll verify this dir when initializing our Git object
 		return false, nil

pkg/app/app_test.go

@@ -1,45 +0,0 @@
-package app
-
-import (
-	"testing"
-
-	"github.com/stretchr/testify/assert"
-)
-
-func TestIsGitVersionValid(t *testing.T) {
-	type scenario struct {
-		versionStr     string
-		expectedResult bool
-	}
-
-	scenarios := []scenario{
-		{
-			"",
-			false,
-		},
-		{
-			"git version 1.9.0",
-			false,
-		},
-		{
-			"git version 1.9.0 (Apple Git-128)",
-			false,
-		},
-		{
-			"git version 2.4.0",
-			true,
-		},
-		{
-			"git version 2.24.3 (Apple Git-128)",
-			true,
-		},
-	}
-
-	for _, s := range scenarios {
-		s := s
-		t.Run(s.versionStr, func(t *testing.T) {
-			result := isGitVersionValid(s.versionStr)
-			assert.Equal(t, result, s.expectedResult)
-		})
-	}
-}

pkg/commands/git.go

@@ -53,6 +53,7 @@ type Loaders struct {
 
 func NewGitCommand(
 	cmn *common.Common,
+	version *git_commands.GitVersion,
 	osCommand *oscommands.OSCommand,
 	gitConfig git_config.IGitConfig,
 	syncMutex *deadlock.Mutex,
@@ -73,6 +74,7 @@ func NewGitCommand(
 
 	return NewGitCommandAux(
 		cmn,
+		version,
 		osCommand,
 		gitConfig,
 		dotGitDir,
@@ -83,6 +85,7 @@ func NewGitCommand(
 
 func NewGitCommandAux(
 	cmn *common.Common,
+	version *git_commands.GitVersion,
 	osCommand *oscommands.OSCommand,
 	gitConfig git_config.IGitConfig,
 	dotGitDir string,
@@ -100,7 +103,7 @@ func NewGitCommandAux(
 
 	fileLoader := git_commands.NewFileLoader(cmn, cmd, configCommands)
 
-	gitCommon := git_commands.NewGitCommon(cmn, cmd, osCommand, dotGitDir, repo, configCommands, syncMutex)
+	gitCommon := git_commands.NewGitCommon(cmn, version, cmd, osCommand, dotGitDir, repo, configCommands, syncMutex)
 	statusCommands := git_commands.NewStatusCommands(gitCommon)
 	flowCommands := git_commands.NewFlowCommands(gitCommon)
 	remoteCommands := git_commands.NewRemoteCommands(gitCommon)

pkg/commands/git_commands/common.go

@@ -9,6 +9,7 @@ import (
 
 type GitCommon struct {
 	*common.Common
+	version   *GitVersion
 	cmd       oscommands.ICmdObjBuilder
 	os        *oscommands.OSCommand
 	dotGitDir string
@@ -20,6 +21,7 @@ type GitCommon struct {
 
 func NewGitCommon(
 	cmn *common.Common,
+	version *GitVersion,
 	cmd oscommands.ICmdObjBuilder,
 	osCommand *oscommands.OSCommand,
 	dotGitDir string,
@@ -29,6 +31,7 @@ func NewGitCommon(
 ) *GitCommon {
 	return &GitCommon{
 		Common:    cmn,
+		version:   version,
 		cmd:       cmd,
 		os:        osCommand,
 		dotGitDir: dotGitDir,

pkg/commands/git_commands/deps_test.go

@@ -15,6 +15,7 @@ import (
 type commonDeps struct {
 	runner     *oscommands.FakeCmdObjRunner
 	userConfig *config.UserConfig
+	gitVersion *GitVersion
 	gitConfig  *git_config.FakeGitConfig
 	getenv     func(string) string
 	removeFile func(string) error
@@ -48,6 +49,11 @@ func buildGitCommon(deps commonDeps) *GitCommon {
 		gitCommon.Common.UserConfig = config.GetDefaultConfig()
 	}
 
+	gitCommon.version = deps.gitVersion
+	if gitCommon.version == nil {
+		gitCommon.version = &GitVersion{2, 0, 0, ""}
+	}
+
 	gitConfig := deps.gitConfig
 	if gitConfig == nil {
 		gitConfig = git_config.NewFakeGitConfig(nil)

pkg/commands/git_commands/sync.go

@@ -29,7 +29,11 @@ func (self *SyncCommands) PushCmdObj(opts PushOpts) (oscommands.ICmdObj, error)
 	cmdStr := "git push"
 
 	if opts.Force {
+		if self.version.IsOlderThan(2, 30, 0) {
 			cmdStr += " --force-with-lease"
+		} else {
+			cmdStr += " --force-with-lease --force-if-includes"
+		}
 	}
 
 	if opts.SetUpstream {

pkg/commands/git_commands/sync_test.go

@@ -10,6 +10,7 @@ import (
 func TestSyncPush(t *testing.T) {
 	type scenario struct {
 		testName string
+		version  *GitVersion
 		opts     PushOpts
 		test     func(oscommands.ICmdObj, error)
 	}
@@ -17,6 +18,7 @@ func TestSyncPush(t *testing.T) {
 	scenarios := []scenario{
 		{
 			testName: "Push with force disabled",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts:     PushOpts{Force: false},
 			test: func(cmdObj oscommands.ICmdObj, err error) {
 				assert.Equal(t, cmdObj.ToString(), "git push")
@@ -25,14 +27,25 @@ func TestSyncPush(t *testing.T) {
 		},
 		{
 			testName: "Push with force enabled",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts:     PushOpts{Force: true},
 			test: func(cmdObj oscommands.ICmdObj, err error) {
 				assert.Equal(t, cmdObj.ToString(), "git push --force-with-lease")
 				assert.NoError(t, err)
 			},
 		},
+		{
+			testName: "Push with force enabled (>= 2.30.0)",
+			version:  &GitVersion{2, 30, 0, ""},
+			opts:     PushOpts{Force: true},
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), "git push --force-with-lease --force-if-includes")
+				assert.NoError(t, err)
+			},
+		},
 		{
 			testName: "Push with force disabled, upstream supplied",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts: PushOpts{
 				Force:          false,
 				UpstreamRemote: "origin",
@@ -45,6 +58,7 @@ func TestSyncPush(t *testing.T) {
 		},
 		{
 			testName: "Push with force disabled, setting upstream",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts: PushOpts{
 				Force:          false,
 				UpstreamRemote: "origin",
@@ -58,6 +72,7 @@ func TestSyncPush(t *testing.T) {
 		},
 		{
 			testName: "Push with force enabled, setting upstream",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts: PushOpts{
 				Force:          true,
 				UpstreamRemote: "origin",
@@ -69,8 +84,23 @@ func TestSyncPush(t *testing.T) {
 				assert.NoError(t, err)
 			},
 		},
+		{
+			testName: "Push with force enabled, setting upstream (>= 2.30.0)",
+			version:  &GitVersion{2, 30, 0, ""},
+			opts: PushOpts{
+				Force:          true,
+				UpstreamRemote: "origin",
+				UpstreamBranch: "master",
+				SetUpstream:    true,
+			},
+			test: func(cmdObj oscommands.ICmdObj, err error) {
+				assert.Equal(t, cmdObj.ToString(), `git push --force-with-lease --force-if-includes --set-upstream "origin" "master"`)
+				assert.NoError(t, err)
+			},
+		},
 		{
 			testName: "Push with remote branch but no origin",
+			version:  &GitVersion{2, 29, 3, ""},
 			opts: PushOpts{
 				Force:          true,
 				UpstreamRemote: "",
@@ -87,7 +117,7 @@ func TestSyncPush(t *testing.T) {
 	for _, s := range scenarios {
 		s := s
 		t.Run(s.testName, func(t *testing.T) {
-			instance := buildSyncCommands(commonDeps{})
+			instance := buildSyncCommands(commonDeps{gitVersion: s.version})
 			s.test(instance.PushCmdObj(s.opts))
 		})
 	}

pkg/commands/git_commands/version.go

@@ -0,0 +1,67 @@
+package git_commands
+
+import (
+	"errors"
+	"regexp"
+	"strconv"
+	"strings"
+
+	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
+)
+
+type GitVersion struct {
+	Major, Minor, Patch int
+	Additional          string
+}
+
+func GetGitVersion(osCommand *oscommands.OSCommand) (*GitVersion, error) {
+	versionStr, _, err := osCommand.Cmd.New("git --version").RunWithOutputs()
+	if err != nil {
+		return nil, err
+	}
+
+	version, err := ParseGitVersion(versionStr)
+	if err != nil {
+		return nil, err
+	}
+
+	return version, nil
+}
+
+func ParseGitVersion(versionStr string) (*GitVersion, error) {
+	// versionStr should be something like:
+	// git version 2.39.0
+	// git version 2.37.1 (Apple Git-137.1)
+	re := regexp.MustCompile(`[^\d]+(\d+)(\.\d+)?(\.\d+)?(.*)`)
+	matches := re.FindStringSubmatch(versionStr)
+
+	if len(matches) < 5 {
+		return nil, errors.New("unexpected git version format: " + versionStr)
+	}
+
+	v := &GitVersion{}
+	var err error
+
+	if v.Major, err = strconv.Atoi(matches[1]); err != nil {
+		return nil, err
+	}
+	if len(matches[2]) > 1 {
+		if v.Minor, err = strconv.Atoi(matches[2][1:]); err != nil {
+			return nil, err
+		}
+	}
+	if len(matches[3]) > 1 {
+		if v.Patch, err = strconv.Atoi(matches[3][1:]); err != nil {
+			return nil, err
+		}
+	}
+	v.Additional = strings.Trim(matches[4], " \r\n")
+
+	return v, nil
+}
+
+func (v *GitVersion) IsOlderThan(major, minor, patch int) bool {
+	actual := v.Major*1000*1000 + v.Minor*1000 + v.Patch
+	required := major*1000*1000 + minor*1000 + patch
+	return actual < required
+}

pkg/commands/git_commands/version_test.go

@@ -0,0 +1,47 @@
+package git_commands
+
+import (
+	"testing"
+
+	"github.com/stretchr/testify/assert"
+)
+
+func TestParseGitVersion(t *testing.T) {
+	scenarios := []struct {
+		input    string
+		expected GitVersion
+	}{
+		{
+			input:    "git version 2.39.0",
+			expected: GitVersion{Major: 2, Minor: 39, Patch: 0, Additional: ""},
+		},
+		{
+			input:    "git version 2.37.1 (Apple Git-137.1)",
+			expected: GitVersion{Major: 2, Minor: 37, Patch: 1, Additional: "(Apple Git-137.1)"},
+		},
+		{
+			input:    "git version 2.37 (Apple Git-137.1)",
+			expected: GitVersion{Major: 2, Minor: 37, Patch: 0, Additional: "(Apple Git-137.1)"},
+		},
+	}
+
+	for _, s := range scenarios {
+		actual, err := ParseGitVersion(s.input)
+
+		assert.NoError(t, err)
+		assert.NotNil(t, actual)
+		assert.Equal(t, s.expected.Major, actual.Major)
+		assert.Equal(t, s.expected.Minor, actual.Minor)
+		assert.Equal(t, s.expected.Patch, actual.Patch)
+		assert.Equal(t, s.expected.Additional, actual.Additional)
+	}
+}
+
+func TestGitVersionIsOlderThan(t *testing.T) {
+	assert.False(t, (&GitVersion{2, 0, 0, ""}).IsOlderThan(1, 99, 99))
+	assert.False(t, (&GitVersion{2, 0, 0, ""}).IsOlderThan(2, 0, 0))
+	assert.False(t, (&GitVersion{2, 1, 0, ""}).IsOlderThan(2, 0, 9))
+
+	assert.True(t, (&GitVersion{2, 0, 1, ""}).IsOlderThan(2, 1, 0))
+	assert.True(t, (&GitVersion{2, 0, 1, ""}).IsOlderThan(3, 0, 0))
+}

pkg/commands/git_test.go

@@ -8,6 +8,7 @@ import (
 
 	"github.com/go-errors/errors"
 	gogit "github.com/jesseduffield/go-git/v5"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/git_config"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/utils"
@@ -218,6 +219,7 @@ func TestNewGitCommand(t *testing.T) {
 			s.setup()
 			s.test(
 				NewGitCommand(utils.NewDummyCommon(),
+					&git_commands.GitVersion{},
 					oscommands.NewDummyOSCommand(),
 					git_config.NewFakeGitConfig(nil),
 					&deadlock.Mutex{},

pkg/gui/dummies.go

@@ -1,7 +1,7 @@
 package gui
 
 import (
-	"github.com/jesseduffield/lazygit/pkg/commands/git_config"
+	"github.com/jesseduffield/lazygit/pkg/commands/git_commands"
 	"github.com/jesseduffield/lazygit/pkg/commands/oscommands"
 	"github.com/jesseduffield/lazygit/pkg/config"
 	"github.com/jesseduffield/lazygit/pkg/updates"
@@ -17,6 +17,6 @@ func NewDummyUpdater() *updates.Updater {
 
 func NewDummyGui() *Gui {
 	newAppConfig := config.NewDummyAppConfig()
-	dummyGui, _ := NewGui(utils.NewDummyCommon(), newAppConfig, git_config.NewFakeGitConfig(nil), NewDummyUpdater(), false, "")
+	dummyGui, _ := NewGui(utils.NewDummyCommon(), newAppConfig, &git_commands.GitVersion{}, NewDummyUpdater(), false, "")
 	return dummyGui
 }

pkg/gui/gui.go

@@ -77,6 +77,7 @@ type Repo string
 type Gui struct {
 	*common.Common
 	g          *gocui.Gui
+	gitVersion *git_commands.GitVersion
 	git        *commands.GitCommand
 	os         *oscommands.OSCommand
 
@@ -222,6 +223,7 @@ func (gui *Gui) onNewRepo(startArgs appTypes.StartArgs, reuseState bool) error {
 	var err error
 	gui.git, err = commands.NewGitCommand(
 		gui.Common,
+		gui.gitVersion,
 		gui.os,
 		git_config.NewStdCachedGitConfig(gui.Log),
 		gui.Mutexes.SyncMutex,
@@ -341,13 +343,14 @@ func initialContext(contextTree *context.ContextTree, startArgs appTypes.StartAr
 func NewGui(
 	cmn *common.Common,
 	config config.AppConfigurer,
-	gitConfig git_config.IGitConfig,
+	gitVersion *git_commands.GitVersion,
 	updater *updates.Updater,
 	showRecentRepos bool,
 	initialDir string,
 ) (*Gui, error) {
 	gui := &Gui{
 		Common:                  cmn,
+		gitVersion:              gitVersion,
 		Config:                  config,
 		Updater:                 updater,
 		statusManager:           &statusManager{},