diff --git a/pkg/commands/git_commands/commit_loader.go b/pkg/commands/git_commands/commit_loader.go index 1d4314c5b..72adf1b69 100644 --- a/pkg/commands/git_commands/commit_loader.go +++ b/pkg/commands/git_commands/commit_loader.go @@ -11,6 +11,7 @@ import ( "strings" "sync" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -98,23 +99,25 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, } }) - var ancestor string - var remoteAncestor string + var unmergedCommitHashes *set.Set[string] + var remoteUnmergedCommitHashes *set.Set[string] + mainBranches := opts.MainBranches.Get() + go utils.Safe(func() { defer wg.Done() - ancestor = opts.MainBranches.GetMergeBase(opts.RefName) - if opts.RefToShowDivergenceFrom != "" { - remoteAncestor = opts.MainBranches.GetMergeBase(opts.RefToShowDivergenceFrom) + if len(mainBranches) > 0 { + unmergedCommitHashes = self.getReachableHashes(opts.RefName, mainBranches) + if opts.RefToShowDivergenceFrom != "" { + remoteUnmergedCommitHashes = self.getReachableHashes(opts.RefToShowDivergenceFrom, mainBranches) + } } }) - passedFirstPushedCommit := false - // I can get this before - firstPushedCommit, err := self.getFirstPushedCommit(opts.RefForPushedStatus) - if err != nil || firstPushedCommit == "" { - // must have no upstream branch so we'll consider everything as pushed - passedFirstPushedCommit = true + var unpushedCommitHashes *set.Set[string] + if opts.RefForPushedStatus != nil { + unpushedCommitHashes = self.getReachableHashes(opts.RefForPushedStatus.FullRefName(), + append([]string{opts.RefForPushedStatus.RefName() + "@{u}"}, mainBranches...)) } wg.Wait() @@ -123,19 +126,6 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, return nil, logErr } - for _, commit := range commits { - if commit.Hash() == firstPushedCommit { - passedFirstPushedCommit = true - } - if !commit.IsTODO() { - if passedFirstPushedCommit { - commit.Status = models.StatusPushed - } else { - commit.Status = models.StatusUnpushed - } - } - } - if len(commits) == 0 { return commits, nil } @@ -153,10 +143,10 @@ func (self *CommitLoader) GetCommits(opts GetCommitsOptions) ([]*models.Commit, localSectionStart = len(commits) } - setCommitMergedStatuses(remoteAncestor, commits[:localSectionStart]) - setCommitMergedStatuses(ancestor, commits[localSectionStart:]) + setCommitStatuses(unpushedCommitHashes, remoteUnmergedCommitHashes, commits[:localSectionStart]) + setCommitStatuses(unpushedCommitHashes, unmergedCommitHashes, commits[localSectionStart:]) } else { - setCommitMergedStatuses(ancestor, commits) + setCommitStatuses(unpushedCommitHashes, unmergedCommitHashes, commits) } return commits, nil @@ -549,56 +539,40 @@ func (self *CommitLoader) getConflictedSequencerCommit(hashPool *utils.StringPoo }) } -func setCommitMergedStatuses(ancestor string, commits []*models.Commit) { - if ancestor == "" { - return - } - - passedAncestor := false +func setCommitStatuses(unpushedCommitHashes *set.Set[string], unmergedCommitHashes *set.Set[string], commits []*models.Commit) { for i, commit := range commits { - // some commits aren't really commits and don't have hashes, such as the update-ref todo - if commit.Hash() != "" && strings.HasPrefix(ancestor, commit.Hash()) { - passedAncestor = true - } - if commit.Status != models.StatusPushed && commit.Status != models.StatusUnpushed { + if commit.IsTODO() { continue } - if passedAncestor { + + if unmergedCommitHashes == nil || unmergedCommitHashes.Includes(commit.Hash()) { + if unpushedCommitHashes != nil && unpushedCommitHashes.Includes(commit.Hash()) { + commits[i].Status = models.StatusUnpushed + } else { + commits[i].Status = models.StatusPushed + } + } else { commits[i].Status = models.StatusMerged } } } -func ignoringWarnings(commandOutput string) string { - trimmedOutput := strings.TrimSpace(commandOutput) - split := strings.Split(trimmedOutput, "\n") - // need to get last line in case the first line is a warning about how the error is ambiguous. - // At some point we should find a way to make it unambiguous - lastLine := split[len(split)-1] - - return lastLine -} - -// getFirstPushedCommit returns the first commit hash which has been pushed to the ref's upstream. -// all commits above this are deemed unpushed and marked as such. -func (self *CommitLoader) getFirstPushedCommit(ref models.Ref) (string, error) { - if ref == nil { - return "", nil - } - - output, err := self.cmd.New( - NewGitCmd("merge-base"). - Arg(ref.FullRefName()). - Arg(ref.RefName() + "@{u}"). +func (self *CommitLoader) getReachableHashes(refName string, notRefNames []string) *set.Set[string] { + output, _, err := self.cmd.New( + NewGitCmd("rev-list"). + Arg(refName). + Arg(lo.Map(notRefNames, func(name string, _ int) string { + return "^" + name + })...). ToArgv(), ). DontLog(). - RunWithOutput() + RunWithOutputs() if err != nil { - return "", err + return set.New[string]() } - return ignoringWarnings(output), nil + return set.NewFromSlice(utils.SplitLines(output)) } // getLog gets the git log. diff --git a/pkg/commands/git_commands/commit_loader_test.go b/pkg/commands/git_commands/commit_loader_test.go index cbf170e5b..7f9873b0b 100644 --- a/pkg/commands/git_commands/commit_loader_test.go +++ b/pkg/commands/git_commands/commit_loader_test.go @@ -6,6 +6,7 @@ import ( "testing" "github.com/go-errors/errors" + "github.com/jesseduffield/generics/set" "github.com/jesseduffield/lazygit/pkg/commands/models" "github.com/jesseduffield/lazygit/pkg/commands/oscommands" "github.com/jesseduffield/lazygit/pkg/common" @@ -43,7 +44,7 @@ func TestGetCommits(t *testing.T) { logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -54,7 +55,7 @@ func TestGetCommits(t *testing.T) { logOrder: "topo-order", opts: GetCommitsOptions{RefName: "refs/heads/mybranch", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "refs/heads/mybranch", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -67,7 +68,7 @@ func TestGetCommits(t *testing.T) { mainBranches: []string{"master", "main", "develop"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}", "^refs/remotes/origin/master", "^refs/remotes/origin/main"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil). // here it's actually getting all the commits in a formatted form, one per line ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, commitsOutput, nil). // here it's testing which of the configured main branches have an upstream @@ -77,8 +78,9 @@ func TestGetCommits(t *testing.T) { ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "develop@{u}"}, "", errors.New("error")). // this one doesn't, so it checks origin instead ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/remotes/origin/develop"}, "", errors.New("error")). // doesn't exist there, either, so it checks for a local branch ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/develop"}, "", errors.New("error")). // no local branch either - // here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged' - ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/main"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil), + // here it's seeing which of our commits are not on any of the main branches yet + ExpectGitArgs([]string{"rev-list", "HEAD", "^refs/remotes/origin/master", "^refs/remotes/origin/main"}, + "0eea75e8c631fba6b58135697835d58ba4c18dbc\nb21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164\ne94e8fc5b6fab4cb755f29f1bdb3ee5e001df35c\nd8084cd558925eb7c9c38afeed5725c21653ab90\n65f910ebd85283b5cce9bf67d03d3f1a9ea3813a\n", nil), expectedCommitOpts: []models.NewCommitOpts{ { @@ -197,13 +199,13 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should not call merge-base for mainBranches if none exist", + testName: "should not call rev-list for mainBranches if none exist", logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil). // here it's actually getting all the commits in a formatted form, one per line ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist; neither does @@ -233,13 +235,13 @@ func TestGetCommits(t *testing.T) { expectedError: nil, }, { - testName: "should call merge-base for all main branches that exist", + testName: "should call rev-list with all main branches that exist", logOrder: "topo-order", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, mainBranches: []string{"master", "main", "develop", "1.0-hotfixes"}, runner: oscommands.NewFakeRunner(t). // here it's seeing which commits are yet to be pushed - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}", "^refs/remotes/origin/master", "^refs/remotes/origin/develop", "^refs/remotes/origin/1.0-hotfixes"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil). // here it's actually getting all the commits in a formatted form, one per line ExpectGitArgs([]string{"log", "HEAD", "--topo-order", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, singleCommitOutput, nil). // here it's testing which of the configured main branches exist @@ -249,8 +251,8 @@ func TestGetCommits(t *testing.T) { ExpectGitArgs([]string{"rev-parse", "--verify", "--quiet", "refs/heads/main"}, "", errors.New("error")). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "develop@{u}"}, "refs/remotes/origin/develop", nil). ExpectGitArgs([]string{"rev-parse", "--symbolic-full-name", "1.0-hotfixes@{u}"}, "refs/remotes/origin/1.0-hotfixes", nil). - // here it's seeing where our branch diverged from the master branch so that we can mark that commit and parent commits as 'merged' - ExpectGitArgs([]string{"merge-base", "HEAD", "refs/remotes/origin/master", "refs/remotes/origin/develop", "refs/remotes/origin/1.0-hotfixes"}, "26c07b1ab33860a1a7591a0638f9925ccf497ffa", nil), + // here it's seeing which of our commits are not on any of the main branches yet + ExpectGitArgs([]string{"rev-list", "HEAD", "^refs/remotes/origin/master", "^refs/remotes/origin/develop", "^refs/remotes/origin/1.0-hotfixes"}, "0eea75e8c631fba6b58135697835d58ba4c18dbc\n", nil), expectedCommitOpts: []models.NewCommitOpts{ { @@ -275,7 +277,7 @@ func TestGetCommits(t *testing.T) { logOrder: "default", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, IncludeRebaseCommits: false}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--no-show-signature", "--"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -286,7 +288,7 @@ func TestGetCommits(t *testing.T) { logOrder: "default", opts: GetCommitsOptions{RefName: "HEAD", RefForPushedStatus: &models.Branch{Name: "mybranch"}, FilterPath: "src"}, runner: oscommands.NewFakeRunner(t). - ExpectGitArgs([]string{"merge-base", "refs/heads/mybranch", "mybranch@{u}"}, "b21997d6b4cbdf84b149d8e6a2c4d06a8e9ec164", nil). + ExpectGitArgs([]string{"rev-list", "refs/heads/mybranch", "^mybranch@{u}"}, "", nil). ExpectGitArgs([]string{"log", "HEAD", "--oneline", "--pretty=format:+%H%x00%at%x00%aN%x00%ae%x00%P%x00%m%x00%D%x00%s", "--abbrev=40", "--follow", "--name-status", "--no-show-signature", "--", "src"}, "", nil), expectedCommitOpts: []models.NewCommitOpts{}, @@ -536,11 +538,12 @@ func TestCommitLoader_getConflictedCommitImpl(t *testing.T) { } } -func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { +func TestCommitLoader_setCommitStatuses(t *testing.T) { type scenario struct { testName string commitOpts []models.NewCommitOpts - ancestor string + unmergedCommits []string + unpushedCommits []string expectedCommitOpts []models.NewCommitOpts } @@ -548,13 +551,13 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { { testName: "basic", commitOpts: []models.NewCommitOpts{ - {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, - {Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusPushed}, - {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, + {Hash: "12345", Name: "1", Action: models.ActionNone}, + {Hash: "67890", Name: "2", Action: models.ActionNone}, + {Hash: "abcde", Name: "3", Action: models.ActionNone}, }, - ancestor: "67890", + unmergedCommits: []string{"12345"}, expectedCommitOpts: []models.NewCommitOpts{ - {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, + {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusPushed}, {Hash: "67890", Name: "2", Action: models.ActionNone, Status: models.StatusMerged}, {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusMerged}, }, @@ -562,11 +565,12 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { { testName: "with update-ref", commitOpts: []models.NewCommitOpts{ - {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, - {Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, - {Hash: "abcde", Name: "3", Action: models.ActionNone, Status: models.StatusPushed}, + {Hash: "12345", Name: "1", Action: models.ActionNone}, + {Hash: "", Name: "", Action: todo.UpdateRef}, + {Hash: "abcde", Name: "3", Action: models.ActionNone}, }, - ancestor: "deadbeef", + unmergedCommits: []string{"12345", "abcde"}, + unpushedCommits: []string{"12345"}, expectedCommitOpts: []models.NewCommitOpts{ {Hash: "12345", Name: "1", Action: models.ActionNone, Status: models.StatusUnpushed}, {Hash: "", Name: "", Action: todo.UpdateRef, Status: models.StatusNone}, @@ -581,7 +585,7 @@ func TestCommitLoader_setCommitMergedStatuses(t *testing.T) { commits := lo.Map(scenario.commitOpts, func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) - setCommitMergedStatuses(scenario.ancestor, commits) + setCommitStatuses(set.NewFromSlice(scenario.unpushedCommits), set.NewFromSlice(scenario.unmergedCommits), commits) expectedCommits := lo.Map(scenario.expectedCommitOpts, func(opts models.NewCommitOpts, _ int) *models.Commit { return models.NewCommit(hashPool, opts) }) assert.Equal(t, expectedCommits, commits)