diff --git a/pkg/gui/controllers/helpers/refresh_helper.go b/pkg/gui/controllers/helpers/refresh_helper.go index 096526955..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,7 +1562,7 @@ func (self *RefreshHelper) refreshReflogCommits(captured capturedReflogState, en return reflogCommits, nil } -func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, env refreshEnv) []*models.Remote { +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 @@ -1563,6 +1581,10 @@ func (self *RefreshHelper) refreshRemotes(prevSelectedRemote *models.Remote, 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()