diff options
-rw-r--r-- | docs/Config.md | 2 | ||||
-rw-r--r-- | pkg/commands/git.go | 4 | ||||
-rw-r--r-- | pkg/commands/oscommands/cmd_obj.go | 33 | ||||
-rw-r--r-- | pkg/commands/oscommands/exec_live_default.go | 9 | ||||
-rw-r--r-- | pkg/commands/oscommands/exec_live_win.go | 4 | ||||
-rw-r--r-- | pkg/commands/oscommands/os.go | 35 | ||||
-rw-r--r-- | pkg/commands/remotes.go | 3 | ||||
-rw-r--r-- | pkg/commands/sync.go | 91 | ||||
-rw-r--r-- | pkg/commands/sync_test.go | 125 | ||||
-rw-r--r-- | pkg/commands/tags.go | 9 | ||||
-rw-r--r-- | pkg/config/user_config.go | 8 | ||||
-rw-r--r-- | pkg/gui/branches_panel.go | 2 | ||||
-rw-r--r-- | pkg/gui/files_panel.go | 39 | ||||
-rw-r--r-- | pkg/gui/rebase_options_panel.go | 19 |
14 files changed, 161 insertions, 222 deletions
diff --git a/docs/Config.md b/docs/Config.md index 99c44855b..8f61d1b62 100644 --- a/docs/Config.md +++ b/docs/Config.md @@ -59,8 +59,6 @@ git: manualCommit: false # extra args passed to `git merge`, e.g. --no-ff args: '' - pull: - mode: 'auto' # one of 'auto' | 'merge' | 'rebase' | 'ff-only', auto reads from git configuration skipHookPrefix: WIP autoFetch: true branchLogCmd: 'git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --' 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) } diff --git a/pkg/config/user_config.go b/pkg/config/user_config.go index c4aee9985..06c293d71 100644 --- a/pkg/config/user_config.go +++ b/pkg/config/user_config.go @@ -61,7 +61,6 @@ type CommitLengthConfig struct { type GitConfig struct { Paging PagingConfig `yaml:"paging"` Merging MergingConfig `yaml:"merging"` - Pull PullConfig `yaml:"pull"` SkipHookPrefix string `yaml:"skipHookPrefix"` AutoFetch bool `yaml:"autoFetch"` BranchLogCmd string `yaml:"branchLogCmd"` @@ -83,10 +82,6 @@ type MergingConfig struct { Args string `yaml:"args"` } -type PullConfig struct { - Mode string `yaml:"mode"` -} - type CommitPrefixConfig struct { Pattern string `yaml:"pattern"` Replace string `yaml:"replace"` @@ -341,9 +336,6 @@ func GetDefaultConfig() *UserConfig { ManualCommit: false, Args: "", }, - Pull: PullConfig{ - Mode: "auto", - }, SkipHookPrefix: "WIP", AutoFetch: true, BranchLogCmd: "git log --graph --color=always --abbrev-commit --decorate --date=relative --pretty=medium {{branchName}} --", diff --git a/pkg/gui/branches_panel.go b/pkg/gui/branches_panel.go index 0d20b14fd..d7d972ae7 100644 --- a/pkg/gui/branches_panel.go +++ b/pkg/gui/branches_panel.go @@ -414,7 +414,7 @@ func (gui *Gui) handleFastForward() error { _ = gui.createLoaderPanel(message) if gui.State.Panels.Branches.SelectedLineIdx == 0 { - _ = gui.pullWithMode("ff-only", PullFilesOptions{span: span}) + _ = gui.pullWithLock(PullFilesOptions{span: span, FastForwardOnly: true}) } else { err := gui.GitCommand.WithSpan(span).FastForward(branch.Name, remoteName, remoteBranchName, gui.promptUserForCredential) gui.handleCredentialsPopup(err) diff --git a/pkg/gui/files_panel.go b/pkg/gui/files_panel.go index 06452a358..41ac25fb8 100644 --- a/pkg/gui/files_panel.go +++ b/pkg/gui/files_panel.go @@ -678,9 +678,10 @@ func (gui *Gui) handlePullFiles() error { } type PullFilesOptions struct { - RemoteName string - BranchName string - span string + RemoteName string + BranchName string + FastForwardOnly bool + span string } func (gui *Gui) pullFiles(opts PullFilesOptions) error { @@ -688,46 +689,30 @@ func (gui *Gui) pullFiles(opts PullFilesOptions) error { return err } - mode := &gui.Config.GetUserConfig().Git.Pull.Mode - *mode = gui.GitCommand.GetPullMode(*mode) - // TODO: this doesn't look like a good idea. Why the goroutine? - go utils.Safe(func() { _ = gui.pullWithMode(*mode, opts) }) + go utils.Safe(func() { _ = gui.pullWithLock(opts) }) return nil } -func (gui *Gui) pullWithMode(mode string, opts PullFilesOptions) error { +func (gui *Gui) pullWithLock(opts PullFilesOptions) error { gui.Mutexes.FetchMutex.Lock() defer gui.Mutexes.FetchMutex.Unlock() gitCommand := gui.GitCommand.WithSpan(opts.span) - err := gitCommand.Fetch( - commands.FetchOptions{ + err := gitCommand.Pull( + commands.PullOptions{ PromptUserForCredential: gui.promptUserForCredential, RemoteName: opts.RemoteName, BranchName: opts.BranchName, + FastForwardOnly: opts.FastForwardOnly, }, ) - gui.handleCredentialsPopup(err) - if err != nil { - return gui.refreshSidePanels(refreshOptions{mode: ASYNC}) - } - - switch mode { - case "rebase": - err := gitCommand.RebaseBranch("FETCH_HEAD") - return gui.handleGenericMergeCommandResult(err) - case "merge": - err := gitCommand.Merge("FETCH_HEAD", commands.MergeOpts{}) - return gui.handleGenericMergeCommandResult(err) - case "ff-only": - err := gitCommand.Merge("FETCH_HEAD", commands.MergeOpts{FastForwardOnly: true}) - return gui.handleGenericMergeCommandResult(err) - default: - return gui.createErrorPanel(fmt.Sprintf("git pull mode '%s' unrecognised", mode)) + if err == nil { + _ = gui.closeConfirmationPrompt(false) } + return gui.handleGenericMergeCommandResult(err) } type pushOpts struct { diff --git a/pkg/gui/rebase_options_panel.go b/pkg/gui/rebase_options_panel.go index a50608b67..d554bc746 100644 --- a/pkg/gui/rebase_options_panel.go +++ b/pkg/gui/rebase_options_panel.go @@ -63,6 +63,23 @@ func (gui *Gui) genericMergeCommand(command string) error { return nil } +var conflictStrings = []string{ + "Failed to merge in the changes", + "When you have resolved this problem", + "fix conflicts", + "Resolve all conflicts manually", +} + +func isMergeConflictErr(errStr string) bool { + for _, str := range conflictStrings { + if strings.Contains(errStr, str) { + return true + } + } + + return false +} + func (gui *Gui) handleGenericMergeCommandResult(result error) error { if err := gui.refreshSidePanels(refreshOptions{mode: ASYNC}); err != nil { return err @@ -76,7 +93,7 @@ func (gui *Gui) handleGenericMergeCommandResult(result error) error { } else if strings.Contains(result.Error(), "No rebase in progress?") { // assume in this case that we're already done return nil - } else if strings.Contains(result.Error(), "When you have resolved this problem") || strings.Contains(result.Error(), "fix conflicts") || strings.Contains(result.Error(), "Resolve all conflicts manually") { + } else if isMergeConflictErr(result.Error()) { return gui.ask(askOpts{ title: gui.Tr.FoundConflictsTitle, prompt: gui.Tr.FoundConflicts, |