simplify pull logic

This commit is contained in:
Jesse Duffield 2021-10-20 22:21:16 +11:00
parent 5ee559b896
commit 6388af70ac
14 changed files with 163 additions and 224 deletions

View file

@ -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}} --'

View file

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

View file

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

View file

@ -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

View file

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

View file

@ -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,
}
}

View file

@ -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

View file

@ -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 {
})
}
type PullOptions struct {
PromptUserForCredential func(string) string
RemoteName string
BranchName string
FastForwardOnly bool
}
func (c *GitCommand) Pull(opts PullOptions) error {
if opts.PromptUserForCredential == nil {
return errors.New("PromptUserForCredential is required")
}
cmdStr := "git pull --no-edit"
if opts.FastForwardOnly {
cmdStr += " --ff-only"
}
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 {
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)
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 {
command := fmt.Sprintf("git fetch %s", c.OSCommand.Quote(remoteName))
return c.OSCommand.DetectUnamePass(command, promptUserForCredential)
}
func (c *GitCommand) GetPullMode(mode string) string {
if mode != "auto" {
return mode
}
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"
}
cmdStr := fmt.Sprintf("git fetch %s", c.OSCommand.Quote(remoteName))
cmdObj := c.NewCmdObjFromStr(cmdStr)
return c.OSCommand.DetectUnamePass(cmdObj, promptUserForCredential)
}

View file

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

View file

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

View file

@ -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}} --",

View file

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

View file

@ -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 {

View file

@ -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,