diff --git a/pkg/commands/git_commands/remote_loader.go b/pkg/commands/git_commands/remote_loader.go index 2a0d9dd29..1ae5a2144 100644 --- a/pkg/commands/git_commands/remote_loader.go +++ b/pkg/commands/git_commands/remote_loader.go @@ -5,10 +5,8 @@ import ( "maps" "slices" "strings" - "sync" "github.com/jesseduffield/lazygit/pkg/commands/models" - "github.com/jesseduffield/lazygit/pkg/utils" "github.com/samber/lo" ) @@ -20,30 +18,12 @@ func NewRemoteLoader(gitCommon *GitCommon) *RemoteLoader { return &RemoteLoader{GitCommon: gitCommon} } -func (self *RemoteLoader) GetRemotes() ([]*models.Remote, error) { - wg := sync.WaitGroup{} - wg.Add(1) - - var remoteBranchesByRemoteName map[string][]*models.RemoteBranch - var remoteBranchesErr error - go utils.Safe(func() { - defer wg.Done() - - remoteBranchesByRemoteName, remoteBranchesErr = self.getRemoteBranchesByRemoteName() - }) - +// GetRemotes returns the repo's remotes, without their branches; those are +// loaded separately with GetRemoteBranchesByRemoteName, which takes a lot longer +// in a repo with many remote branches. +func (self *RemoteLoader) GetRemotes() []*models.Remote { remotes := self.getRemotesFromConfig() - wg.Wait() - - if remoteBranchesErr != nil { - return nil, remoteBranchesErr - } - - for _, remote := range remotes { - remote.Branches = remoteBranchesByRemoteName[remote.Name] - } - // now lets sort our remotes by name alphabetically slices.SortFunc(remotes, func(a, b *models.Remote) int { // we want origin at the top because we'll be most likely to want it @@ -56,7 +36,7 @@ func (self *RemoteLoader) GetRemotes() ([]*models.Remote, error) { return strings.Compare(strings.ToLower(a.Name), strings.ToLower(b.Name)) }) - return remotes, nil + return remotes } func (self *RemoteLoader) getRemotesFromConfig() []*models.Remote { @@ -103,7 +83,9 @@ func (self *RemoteLoader) getRemotesFromConfig() []*models.Remote { return slices.Collect(maps.Values(remotesByName)) } -func (self *RemoteLoader) getRemoteBranchesByRemoteName() (map[string][]*models.RemoteBranch, error) { +// GetRemoteBranchesByRemoteName returns all remote branches, keyed by the name +// of the remote they belong to. +func (self *RemoteLoader) GetRemoteBranchesByRemoteName() (map[string][]*models.RemoteBranch, error) { remoteBranches, err := self.getRemoteBranches() if err != nil { return nil, err diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 730ed9a24..79488ed9d 100644 --- a/pkg/gui/controllers/helpers/refresh_helper.go +++ b/pkg/gui/controllers/helpers/refresh_helper.go @@ -348,6 +348,39 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr // branchesAndRemotesWg gives the fetch the happens-before to read them. var loadedBranches []*models.Branch var loadedRemotes []*models.Remote + + // The branches view also shows a pull request icon against a branch, and + // those icons come from the remotes, so the branches refresh waits for the + // remotes as well, for the same reason it waits for the worktrees. Only the + // remotes themselves are waited for, not their branches, which are loaded + // afterwards (see refreshRemotes). The remotes scope has to be started before + // the branches scope for that wait to be safe. One of the early returns below + // would otherwise leave the branches refresh waiting for a scope that never + // started. + remotesWg := sync.WaitGroup{} + waitForRemotes := func() { remotesWg.Wait() } + if scopeSet.Includes(types.REMOTES) { + // Capture the previously-selected remote on the UI thread; the worker + // needs it to keep the remote-branches selection valid, and reading + // the Remotes context off the UI thread races its render. + var prevSelectedRemote *models.Remote + if !self.captureOnUIThread(calledFromWorker, env.background, func() { + prevSelectedRemote = self.c.Contexts().Remotes.GetSelected() + }) { + return + } + remotesWg.Add(1) + remotesLoaded := sync.OnceFunc(remotesWg.Done) + branchesAndRemotesWg.Add(1) + refresh("remotes", func() { + // Signal again on the way out, so that a future early return in + // refreshRemotes can't leave the branches refresh waiting for ever. + defer remotesLoaded() + defer branchesAndRemotesWg.Done() + loadedRemotes = self.refreshRemotes(prevSelectedRemote, remotesLoaded, env) + }) + } + if scopeSet.Includes(types.COMMITS) { // Capture the refresh's inputs (model, contexts, modes) on the UI // thread, before the git work is dispatched to a worker, so the worker @@ -390,7 +423,7 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr if self.c.UserConfig().Git.LocalBranchSortOrder == "recency" { branchesAndRemotesWg.Add(1) refresh("reflog and branches", func() { - loadedBranches = self.refreshReflogAndBranches(capturedReflog, capturedBranches, waitForWorktrees, options.BranchSelection, options.SelectTopReflogCommit, env) + loadedBranches = self.refreshReflogAndBranches(capturedReflog, capturedBranches, waitForWorktrees, waitForRemotes, options.BranchSelection, options.SelectTopReflogCommit, env) branchesAndRemotesWg.Done() }) } else { @@ -399,7 +432,7 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr // Not a recency sort, so branches doesn't depend on the reflog // being fresh; it runs concurrently with the reflog refresh // below and uses the reflog we captured up front, as it always has. - loadedBranches = self.refreshBranches(capturedBranches, waitForWorktrees, options.BranchSelection, true, capturedReflog.reflogCommits, env) + loadedBranches = self.refreshBranches(capturedBranches, waitForWorktrees, waitForRemotes, options.BranchSelection, true, capturedReflog.reflogCommits, env) branchesAndRemotesWg.Done() }) refresh("reflog", func() { @@ -458,23 +491,6 @@ func (self *RefreshHelper) performRefresh(options types.RefreshOptions, calledFr refresh("tags", func() { _ = self.refreshTags(env) }) } - if scopeSet.Includes(types.REMOTES) { - // Capture the previously-selected remote on the UI thread; the worker - // needs it to keep the remote-branches selection valid, and reading - // the Remotes context off the UI thread races its render. - var prevSelectedRemote *models.Remote - if !self.captureOnUIThread(calledFromWorker, env.background, func() { - prevSelectedRemote = self.c.Contexts().Remotes.GetSelected() - }) { - return - } - branchesAndRemotesWg.Add(1) - refresh("remotes", func() { - loadedRemotes, _ = self.refreshRemotes(prevSelectedRemote, env) - branchesAndRemotesWg.Done() - }) - } - if scopeSet.Includes(types.PULL_REQUESTS) { // Fetching pull requests talks to the GitHub API over the network; on // a bad connection that request can stall for a long time. It runs no @@ -726,19 +742,19 @@ func (self *RefreshHelper) captureBranchState() capturedBranchState { } } -func (self *RefreshHelper) refreshReflogAndBranches(capturedReflog capturedReflogState, capturedBranches capturedBranchState, waitForWorktrees func(), branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, env refreshEnv) []*models.Branch { +func (self *RefreshHelper) refreshReflogAndBranches(capturedReflog capturedReflogState, capturedBranches capturedBranchState, waitForWorktrees func(), waitForRemotes func(), branchSelection types.BranchSelectionBehavior, selectTopReflogCommit bool, env refreshEnv) []*models.Branch { switch self.c.State().GetRepoState().GetStartupStage() { case types.INITIAL: // Return the immediate (non-recency) load's branches; the recency-sorted // reload below runs on its own worker after we return. Both hold the same // set of branches, which is all the caller (the PR fetch) needs. - branches := self.refreshBranches(capturedBranches, waitForWorktrees, branchSelection, false, capturedReflog.reflogCommits, env) + branches := self.refreshBranches(capturedBranches, waitForWorktrees, waitForRemotes, branchSelection, false, capturedReflog.reflogCommits, env) self.onWorker(env.background, func(_ gocui.Task) error { reflogCommits, _ := self.refreshReflogCommits(capturedReflog, env, false) - // The load above already waited for the worktrees, so this one has - // nothing left to wait for. - self.refreshBranches(capturedBranches, func() {}, types.SelectCheckedOutBranch, true, reflogCommits, env) + // The load above already waited for the worktrees and the remotes, so + // this one has nothing left to wait for. + self.refreshBranches(capturedBranches, func() {}, func() {}, types.SelectCheckedOutBranch, true, reflogCommits, env) self.c.State().GetRepoState().SetStartupStage(types.COMPLETE) return nil }) @@ -747,7 +763,7 @@ func (self *RefreshHelper) refreshReflogAndBranches(capturedReflog capturedReflo case types.COMPLETE: reflogCommits, _ := self.refreshReflogCommits(capturedReflog, env, selectTopReflogCommit) - return self.refreshBranches(capturedBranches, waitForWorktrees, branchSelection, true, reflogCommits, env) + return self.refreshBranches(capturedBranches, waitForWorktrees, waitForRemotes, branchSelection, true, reflogCommits, env) } return nil @@ -1134,7 +1150,7 @@ func (self *RefreshHelper) refreshStateSubmoduleConfigs(env refreshEnv) ([]*mode // self.refreshStatus is called at the end of this because that's when we can // be sure there is a State.Model.Branches array to pick the current branch from -func (self *RefreshHelper) refreshBranches(captured capturedBranchState, waitForWorktrees func(), branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, env refreshEnv) []*models.Branch { +func (self *RefreshHelper) refreshBranches(captured capturedBranchState, waitForWorktrees func(), waitForRemotes func(), branchSelection types.BranchSelectionBehavior, loadBehindCounts bool, reflogCommits []*models.Commit, env refreshEnv) []*models.Branch { loadSeq := self.branchLoadSeq.Add(1) branches, err := env.git.Loaders.BranchLoader.Load( @@ -1168,9 +1184,11 @@ func (self *RefreshHelper) refreshBranches(captured capturedBranchState, waitFor self.c.Log.Error(err) } - // Render only once the refreshed worktrees are in the model; the branches - // view shows them against the branches (see performRefresh). + // Render only once the refreshed worktrees and remotes are in the model. The + // branches view shows the worktrees against the branches, and the pull + // request icons come from the remotes (see performRefresh). waitForWorktrees() + waitForRemotes() self.onUIThreadUnlessRepoChanged(env, func() { // Drop this write if a branch load that started later has already applied @@ -1544,14 +1562,16 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en return reflogCommits, nil } -func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env refreshEnv) ([]*models.Remote, error) { - remotes, err := env.git.Loaders.RemoteLoader.GetRemotes() - if err != nil { - return nil, err - } +func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, remotesLoaded func(), env refreshEnv) []*models.Remote { + remotes := env.git.Loaders.RemoteLoader.GetRemotes() + // Put the remotes in the model before loading their branches below. The map + // from branches to pull requests is built from the remotes' URLs (see + // GenerateGithubPullRequestMap), and loading the remote branches takes much + // longer than loading the remotes themselves. If the map had to wait for the + // branches, the pull request icons would show up long after the branch list. self.onUIThreadUnlessRepoChanged(env, func() { - self.c.Model().Remotes = remotes + self.c.Model().Remotes = remotesWithCarriedOverBranches(remotes, self.c.Model().Remotes) hadPrs := len(self.c.Model().PullRequestsMap) != 0 self.rebuildPullRequestsMap() @@ -1559,11 +1579,35 @@ func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env // if we didn't have PRs in the map before but now we do, we need to redraw the branches view self.refreshView(self.c.Contexts().Branches, env) } + }) + + // Tell the branches refresh that the remotes write is queued, so that it can + // queue its own write behind ours and render the icons with the branches. + remotesLoaded() + + self.refreshView(self.c.Contexts().Remotes, env) + + remoteBranchesByRemoteName, err := env.git.Loaders.RemoteLoader.GetRemoteBranchesByRemoteName() + if err != nil { + // The remotes themselves are in the model already; log the failure and + // leave them there without their branches. + self.c.Log.Error(err) + return remotes + } + + remotesWithBranches := lo.Map(remotes, func(remote *models.Remote, _ int) *models.Remote { + withBranches := *remote + withBranches.Branches = remoteBranchesByRemoteName[remote.Name] + return &withBranches + }) + + self.onUIThreadUnlessRepoChanged(env, func() { + self.c.Model().Remotes = remotesWithBranches // we need to ensure our selected remote branches aren't now outdated if prevSelectedRemote != nil && self.c.Model().RemoteBranches != nil { // find remote now - for _, remote := range remotes { + for _, remote := range remotesWithBranches { if remote.Name == prevSelectedRemote.Name { self.c.Model().RemoteBranches = remote.Branches break @@ -1574,7 +1618,25 @@ func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env self.refreshView(self.c.Contexts().Remotes, env) self.refreshView(self.c.Contexts().RemoteBranches, env) - return remotes, nil + return remotesWithBranches +} + +// remotesWithCarriedOverBranches returns copies of the freshly loaded remotes, +// each carrying the branches of the remote of the same name in the model. Those +// branches are the ones the views are showing right now, so carrying them over +// keeps the remote branches, and the branch counts in the remotes view, in place +// until the fresh ones are loaded. The copies also leave the remotes we were +// given untouched, so that the caller can go on reading them off the UI thread. +func remotesWithCarriedOverBranches(remotes []*models.Remote, modelRemotes []*models.Remote) []*models.Remote { + return lo.Map(remotes, func(remote *models.Remote, _ int) *models.Remote { + copied := *remote + if previous, found := lo.Find(modelRemotes, func(modelRemote *models.Remote) bool { + return modelRemote.Name == remote.Name + }); found { + copied.Branches = previous.Branches + } + return &copied + }) } func (self *RefreshHelper) loadWorktrees(env refreshEnv) []*models.Worktree {