From 6388af70acda91bf1bdc9cba9f38dc810f49f9ca Mon Sep 17 00:00:00 2001 From: Jesse Duffield Date: Wed, 20 Oct 2021 22:21:16 +1100 Subject: simplify pull logic --- pkg/commands/git.go | 4 + pkg/commands/oscommands/cmd_obj.go | 33 +++++++ pkg/commands/oscommands/exec_live_default.go | 9 +- pkg/commands/oscommands/exec_live_win.go | 4 +- pkg/commands/oscommands/os.go | 35 +++++++- pkg/commands/remotes.go | 3 +- pkg/commands/sync.go | 91 ++++++++++--------- pkg/commands/sync_test.go | 125 --------------------------- pkg/commands/tags.go | 9 +- 9 files changed, 130 insertions(+), 183 deletions(-) create mode 100644 pkg/commands/oscommands/cmd_obj.go (limited to 'pkg/commands') diff --git a/pkg/commands/git.go b/pkg/commands/git.go index 3a9e434cb..de7569d81 100644 --- a/pkg/commands/git.go +++ b/pkg/commands/git.go @@ -246,3 +246,7 @@ func (c *GitCommand) RunCommandWithOutput(formatString string, formatArgs ...int return output, err } } + +func (c *GitCommand) NewCmdObjFromStr(cmdStr string) oscommands.ICmdObj { + return c.OSCommand.NewCmdObjFromStr(cmdStr).AddEnvVars("GIT_OPTIONAL_LOCKS=0") +} diff --git a/pkg/commands/oscommands/cmd_obj.go b/pkg/commands/oscommands/cmd_obj.go new file mode 100644 index 000000000..592a4a61d --- /dev/null +++ b/pkg/commands/oscommands/cmd_obj.go @@ -0,0 +1,33 @@ +package oscommands + +import ( + "os/exec" +) + +// A command object is a general way to represent a command to be run on the +// command line. If you want to log the command you'll use .ToString() and +// if you want to run it you'll use .GetCmd() +type ICmdObj interface { + GetCmd() *exec.Cmd + ToString() string + AddEnvVars(...string) ICmdObj +} + +type CmdObj struct { + cmdStr string + cmd *exec.Cmd +} + +func (self *CmdObj) GetCmd() *exec.Cmd { + return self.cmd +} + +func (self *CmdObj) ToString() string { + return self.cmdStr +} + +func (self *CmdObj) AddEnvVars(vars ...string) ICmdObj { + self.cmd.Env = append(self.cmd.Env, vars...) + + return self +} diff --git a/pkg/commands/oscommands/exec_live_default.go b/pkg/commands/oscommands/exec_live_default.go index f0c000c1e..95d5d09fb 100644 --- a/pkg/commands/oscommands/exec_live_default.go +++ b/pkg/commands/oscommands/exec_live_default.go @@ -19,11 +19,10 @@ import ( // Output is a function that executes by every word that gets read by bufio // As return of output you need to give a string that will be written to stdin // NOTE: If the return data is empty it won't written anything to stdin -func RunCommandWithOutputLiveWrapper(c *OSCommand, command string, output func(string) string) error { - c.Log.WithField("command", command).Info("RunCommand") - c.LogCommand(command, true) - cmd := c.ExecutableFromString(command) - cmd.Env = append(cmd.Env, "LANG=en_US.UTF-8", "LC_ALL=en_US.UTF-8") +func RunCommandWithOutputLiveWrapper(c *OSCommand, cmdObj ICmdObj, output func(string) string) error { + c.Log.WithField("command", cmdObj.ToString()).Info("RunCommand") + c.LogCommand(cmdObj.ToString(), true) + cmd := cmdObj.AddEnvVars("LANG=en_US.UTF-8", "LC_ALL=en_US.UTF-8").GetCmd() var stderr bytes.Buffer cmd.Stderr = &stderr diff --git a/pkg/commands/oscommands/exec_live_win.go b/pkg/commands/oscommands/exec_live_win.go index 83a40fe74..aa17242e3 100644 --- a/pkg/commands/oscommands/exec_live_win.go +++ b/pkg/commands/oscommands/exec_live_win.go @@ -5,6 +5,6 @@ package oscommands // RunCommandWithOutputLiveWrapper runs a command live but because of windows compatibility this command can't be ran there // TODO: Remove this hack and replace it with a proper way to run commands live on windows -func RunCommandWithOutputLiveWrapper(c *OSCommand, command string, output func(string) string) error { - return c.RunCommand(command) +func RunCommandWithOutputLiveWrapper(c *OSCommand, cmdObj ICmdObj, output func(string) string) error { + return c.RunCommand(cmdObj.ToString()) } diff --git a/pkg/commands/oscommands/os.go b/pkg/commands/oscommands/os.go index 4babbe8a3..f0c9525a5 100644 --- a/pkg/commands/oscommands/os.go +++ b/pkg/commands/oscommands/os.go @@ -218,16 +218,16 @@ func (c *OSCommand) ShellCommandFromString(commandStr string) *exec.Cmd { } // RunCommandWithOutputLive runs RunCommandWithOutputLiveWrapper -func (c *OSCommand) RunCommandWithOutputLive(command string, output func(string) string) error { - return RunCommandWithOutputLiveWrapper(c, command, output) +func (c *OSCommand) RunCommandWithOutputLive(cmdObj ICmdObj, output func(string) string) error { + return RunCommandWithOutputLiveWrapper(c, cmdObj, output) } // DetectUnamePass detect a username / password / passphrase question in a command // promptUserForCredential is a function that gets executed when this function detect you need to fillin a password or passphrase // The promptUserForCredential argument will be "username", "password" or "passphrase" and expects the user's password/passphrase or username back -func (c *OSCommand) DetectUnamePass(command string, promptUserForCredential func(string) string) error { +func (c *OSCommand) DetectUnamePass(cmdObj ICmdObj, promptUserForCredential func(string) string) error { ttyText := "" - errMessage := c.RunCommandWithOutputLive(command, func(word string) string { + errMessage := c.RunCommandWithOutputLive(cmdObj, func(word string) string { ttyText = ttyText + " " + word prompts := map[string]string{ @@ -563,3 +563,30 @@ func (c *OSCommand) RemoveFile(path string) error { return c.removeFile(path) } + +func (c *OSCommand) NewCmdObjFromStr(cmdStr string) ICmdObj { + args := str.ToArgv(cmdStr) + cmd := c.Command(args[0], args[1:]...) + cmd.Env = os.Environ() + + return &CmdObj{ + cmdStr: cmdStr, + cmd: cmd, + } +} + +func (c *OSCommand) NewCmdObjFromArgs(args []string) ICmdObj { + cmd := c.Command(args[0], args[1:]...) + + return &CmdObj{ + cmdStr: strings.Join(args, " "), + cmd: cmd, + } +} + +func (c *OSCommand) NewCmdObj(cmd *exec.Cmd) ICmdObj { + return &CmdObj{ + cmdStr: strings.Join(cmd.Args, " "), + cmd: cmd, + } +} diff --git a/pkg/commands/remotes.go b/pkg/commands/remotes.go index 2003a7dbd..b56893817 100644 --- a/pkg/commands/remotes.go +++ b/pkg/commands/remotes.go @@ -22,7 +22,8 @@ func (c *GitCommand) UpdateRemoteUrl(remoteName string, updatedUrl string) error func (c *GitCommand) DeleteRemoteBranch(remoteName string, branchName string, promptUserForCredential func(string) string) error { command := fmt.Sprintf("git push %s --delete %s", c.OSCommand.Quote(remoteName), c.OSCommand.Quote(branchName)) - return c.OSCommand.DetectUnamePass(command, promptUserForCredential) + cmdObj := c.NewCmdObjFromStr(command) + return c.OSCommand.DetectUnamePass(cmdObj, promptUserForCredential) } // CheckRemoteBranchExists Returns remote branch diff --git a/pkg/commands/sync.go b/pkg/commands/sync.go index 4fc7d73bb..7051f2236 100644 --- a/pkg/commands/sync.go +++ b/pkg/commands/sync.go @@ -2,7 +2,6 @@ package commands import ( "fmt" - "sync" "github.com/go-errors/errors" ) @@ -17,32 +16,33 @@ type PushOpts struct { } func (c *GitCommand) Push(opts PushOpts) error { - cmd := "git push" + cmdStr := "git push" if c.GetConfigValue("push.followTags") != "false" { - cmd += " --follow-tags" + cmdStr += " --follow-tags" } if opts.Force { - cmd += " --force-with-lease" + cmdStr += " --force-with-lease" } if opts.SetUpstream { - cmd += " --set-upstream" + cmdStr += " --set-upstream" } if opts.UpstreamRemote != "" { - cmd += " " + c.OSCommand.Quote(opts.UpstreamRemote) + cmdStr += " " + c.OSCommand.Quote(opts.UpstreamRemote) } if opts.UpstreamBranch != "" { if opts.UpstreamRemote == "" { return errors.New(c.Tr.MustSpecifyOriginError) } - cmd += " " + c.OSCommand.Quote(opts.UpstreamBranch) + cmdStr += " " + c.OSCommand.Quote(opts.UpstreamBranch) } - return c.OSCommand.DetectUnamePass(cmd, opts.PromptUserForCredential) + cmdObj := c.NewCmdObjFromStr(cmdStr) + return c.OSCommand.DetectUnamePass(cmdObj, opts.PromptUserForCredential) } type FetchOptions struct { @@ -53,16 +53,17 @@ type FetchOptions struct { // Fetch fetch git repo func (c *GitCommand) Fetch(opts FetchOptions) error { - command := "git fetch" + cmdStr := "git fetch" if opts.RemoteName != "" { - command = fmt.Sprintf("%s %s", command, c.OSCommand.Quote(opts.RemoteName)) + cmdStr = fmt.Sprintf("%s %s", cmdStr, c.OSCommand.Quote(opts.RemoteName)) } if opts.BranchName != "" { - command = fmt.Sprintf("%s %s", command, c.OSCommand.Quote(opts.BranchName)) + cmdStr = fmt.Sprintf("%s %s", cmdStr, c.OSCommand.Quote(opts.BranchName)) } - return c.OSCommand.DetectUnamePass(command, func(question string) string { + cmdObj := c.NewCmdObjFromStr(cmdStr) + return c.OSCommand.DetectUnamePass(cmdObj, func(question string) string { if opts.PromptUserForCredential != nil { return opts.PromptUserForCredential(question) } @@ -70,41 +71,45 @@ func (c *GitCommand) Fetch(opts FetchOptions) error { }) } -func (c *GitCommand) FastForward(branchName string, remoteName string, remoteBranchName string, promptUserForCredential func(string) string) error { - command := fmt.Sprintf("git fetch %s %s:%s", c.OSCommand.Quote(remoteName), c.OSCommand.Quote(remoteBranchName), c.OSCommand.Quote(branchName)) - return c.OSCommand.DetectUnamePass(command, promptUserForCredential) +type PullOptions struct { + PromptUserForCredential func(string) string + RemoteName string + BranchName string + FastForwardOnly bool } -func (c *GitCommand) FetchRemote(remoteName string, promptUserForCredential func(string) string) error { - command := fmt.Sprintf("git fetch %s", c.OSCommand.Quote(remoteName)) - return c.OSCommand.DetectUnamePass(command, promptUserForCredential) -} +func (c *GitCommand) Pull(opts PullOptions) error { + if opts.PromptUserForCredential == nil { + return errors.New("PromptUserForCredential is required") + } -func (c *GitCommand) GetPullMode(mode string) string { - if mode != "auto" { - return mode + cmdStr := "git pull --no-edit" + + if opts.FastForwardOnly { + cmdStr += " --ff-only" } - var isRebase bool - var isFf bool - var wg sync.WaitGroup - - wg.Add(2) - go func() { - isRebase = c.GetConfigValue("pull.rebase") == "true" - wg.Done() - }() - go func() { - isFf = c.GetConfigValue("pull.ff") == "only" - wg.Done() - }() - wg.Wait() - - if isRebase { - return "rebase" - } else if isFf { - return "ff-only" - } else { - return "merge" + if opts.RemoteName != "" { + cmdStr = fmt.Sprintf("%s %s", cmdStr, c.OSCommand.Quote(opts.RemoteName)) } + if opts.BranchName != "" { + cmdStr = fmt.Sprintf("%s %s", cmdStr, c.OSCommand.Quote(opts.BranchName)) + } + + // setting GIT_SEQUENCE_EDITOR to ':' as a way of skipping it, in case the user + // has 'pull.rebase = interactive' configured. + cmdObj := c.NewCmdObjFromStr(cmdStr).AddEnvVars("GIT_SEQUENCE_EDITOR=:") + return c.OSCommand.DetectUnamePass(cmdObj, opts.PromptUserForCredential) +} + +func (c *GitCommand) FastForward(branchName string, remoteName string, remoteBranchName string, promptUserForCredential func(string) string) error { + cmdStr := fmt.Sprintf("git fetch %s %s:%s", c.OSCommand.Quote(remoteName), c.OSCommand.Quote(remoteBranchName), c.OSCommand.Quote(branchName)) + cmdObj := c.NewCmdObjFromStr(cmdStr) + return c.OSCommand.DetectUnamePass(cmdObj, promptUserForCredential) +} + +func (c *GitCommand) FetchRemote(remoteName string, promptUserForCredential func(string) string) error { + cmdStr := fmt.Sprintf("git fetch %s", c.OSCommand.Quote(remoteName)) + cmdObj := c.NewCmdObjFromStr(cmdStr) + return c.OSCommand.DetectUnamePass(cmdObj, promptUserForCredential) } diff --git a/pkg/commands/sync_test.go b/pkg/commands/sync_test.go index 2f67f91fc..f793639be 100644 --- a/pkg/commands/sync_test.go +++ b/pkg/commands/sync_test.go @@ -248,128 +248,3 @@ func TestGitCommandPush(t *testing.T) { }) } } - -type getPullModeScenario struct { - testName string - getGitConfigValueMock func(string) (string, error) - configPullModeValue string - test func(string) -} - -func TestGetPullMode(t *testing.T) { - - scenarios := getPullModeScenarios(t) - - for _, s := range scenarios { - t.Run(s.testName, func(t *testing.T) { - gitCmd := NewDummyGitCommand() - gitCmd.getGitConfigValue = s.getGitConfigValueMock - s.test(gitCmd.GetPullMode(s.configPullModeValue)) - }) - } -} - -func getPullModeScenarios(t *testing.T) []getPullModeScenario { - return []getPullModeScenario{ - { - testName: "Merge is default", - getGitConfigValueMock: func(s string) (string, error) { - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "merge", actual) - }, - }, { - testName: "Reads rebase when pull.rebase is true", - getGitConfigValueMock: func(s string) (string, error) { - if s == "pull.rebase" { - return "true", nil - } - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "rebase", actual) - }, - }, { - testName: "Reads ff-only when pull.ff is only", - getGitConfigValueMock: func(s string) (string, error) { - if s == "pull.ff" { - return "only", nil - } - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "ff-only", actual) - }, - }, { - testName: "Reads rebase when rebase is true and ff is only", - getGitConfigValueMock: func(s string) (string, error) { - if s == "pull.rebase" { - return "true", nil - } - if s == "pull.ff" { - return "only", nil - } - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "rebase", actual) - }, - }, { - testName: "Reads rebase when pull.rebase is true", - getGitConfigValueMock: func(s string) (string, error) { - if s == "pull.rebase" { - return "true", nil - } - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "rebase", actual) - }, - }, { - testName: "Reads ff-only when pull.ff is only", - getGitConfigValueMock: func(s string) (string, error) { - if s == "pull.ff" { - return "only", nil - } - return "", nil - }, - configPullModeValue: "auto", - test: func(actual string) { - assert.Equal(t, "ff-only", actual) - }, - }, { - testName: "Respects merge config", - getGitConfigValueMock: func(s string) (string, error) { - return "", nil - }, - configPullModeValue: "merge", - test: func(actual string) { - assert.Equal(t, "merge", actual) - }, - }, { - testName: "Respects rebase config", - getGitConfigValueMock: func(s string) (string, error) { - return "", nil - }, - configPullModeValue: "rebase", - test: func(actual string) { - assert.Equal(t, "rebase", actual) - }, - }, { - testName: "Respects ff-only config", - getGitConfigValueMock: func(s string) (string, error) { - return "", nil - }, - configPullModeValue: "ff-only", - test: func(actual string) { - assert.Equal(t, "ff-only", actual) - }, - }, - } -} diff --git a/pkg/commands/tags.go b/pkg/commands/tags.go index e41a2191b..af40ae603 100644 --- a/pkg/commands/tags.go +++ b/pkg/commands/tags.go @@ -1,6 +1,8 @@ package commands -import "fmt" +import ( + "fmt" +) func (c *GitCommand) CreateLightweightTag(tagName string, commitSha string) error { return c.RunCommand("git tag -- %s %s", c.OSCommand.Quote(tagName), commitSha) @@ -11,6 +13,7 @@ func (c *GitCommand) DeleteTag(tagName string) error { } func (c *GitCommand) PushTag(remoteName string, tagName string, promptUserForCredential func(string) string) error { - command := fmt.Sprintf("git push %s %s", c.OSCommand.Quote(remoteName), c.OSCommand.Quote(tagName)) - return c.OSCommand.DetectUnamePass(command, promptUserForCredential) + cmdStr := fmt.Sprintf("git push %s %s", c.OSCommand.Quote(remoteName), c.OSCommand.Quote(tagName)) + cmdObj := c.NewCmdObjFromStr(cmdStr) + return c.OSCommand.DetectUnamePass(cmdObj, promptUserForCredential) } -- cgit v1.2.3