diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index ade614f7df8..b51abb9cc9a 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -847,6 +847,12 @@ func (c *AppConfig) SaveGlobalUserConfig() { type AppState struct { LastUpdateCheck int64 RecentRepos []string + // RecentRepoLocations is the richer successor of RecentRepos: one entry + // per recently-opened repo carrying the environment needed to reopen it + // (empty for every repo git can find from its work tree). RecentRepos is + // still written in parallel with the plain paths so an older lazygit + // reading the same state file keeps working (#5942). + RecentRepoLocations []RecentRepoLocation StartupPopupVersion int DidShowHunkStagingHint bool LastVersion string // this is the last version the user was using, for the purpose of showing release notes diff --git a/pkg/config/recent_repo_location.go b/pkg/config/recent_repo_location.go new file mode 100644 index 00000000000..8b1b5e03f83 --- /dev/null +++ b/pkg/config/recent_repo_location.go @@ -0,0 +1,11 @@ +package config + +// RecentRepoLocation is one entry of AppState.RecentRepoLocations: the +// directory to change back to, plus the GIT_DIR/GIT_WORK_TREE environment a +// repo opened with --git-dir/--work-tree (or core.worktree) needs to be +// found again. GitLocationEnvVars is empty for every repo git can find from +// the work tree alone (#5942). +type RecentRepoLocation struct { + Path string + GitLocationEnvVars []string +} diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index d3b6bfe29b9..6393ee7a715 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -9,6 +9,7 @@ import ( "sync" appTypes "github.com/jesseduffield/lazygit/pkg/app/types" + "github.com/jesseduffield/lazygit/pkg/config" "github.com/jesseduffield/lazygit/pkg/commands" "github.com/jesseduffield/lazygit/pkg/commands/direnv" "github.com/jesseduffield/lazygit/pkg/commands/models" @@ -106,27 +107,24 @@ func (self *ReposHelper) getCurrentBranch(path string) string { func (self *ReposHelper) CreateRecentReposMenu() error { // we'll show an empty panel if there are no recent repos - recentRepoPaths := []string{} - if len(self.c.GetAppState().RecentRepos) > 0 { - // we skip the first one because we're currently in it - recentRepoPaths = self.c.GetAppState().RecentRepos[1:] - } + recentLocations := recentRepoMenuEntries(self.c.GetAppState()) currentBranches := sync.Map{} wg := sync.WaitGroup{} - wg.Add(len(recentRepoPaths)) + wg.Add(len(recentLocations)) - for _, path := range recentRepoPaths { + for _, location := range recentLocations { go func(path string) { defer wg.Done() currentBranches.Store(path, self.getCurrentBranch(path)) - }(path) + }(location.Path) } wg.Wait() - menuItems := lo.Map(recentRepoPaths, func(path string, _ int) *types.MenuItem { + menuItems := lo.Map(recentLocations, func(location config.RecentRepoLocation, _ int) *types.MenuItem { + path := location.Path branchName, _ := currentBranches.Load(path) if icons.IsIconEnabled() { branchName = icons.BRANCH_ICON + " " + fmt.Sprintf("%v", branchName) @@ -148,7 +146,7 @@ func (self *ReposHelper) CreateRecentReposMenu() error { // if we were in a submodule, we want to forget about that stack of repos // so that hitting escape in the new repo does nothing self.c.State().GetRepoPathStack().Clear() - return self.switchTo(path, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) + return self.switchToLocation(types.RepoLocation{Path: location.Path, GitLocationEnvVars: location.GitLocationEnvVars}, self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) }, } }) @@ -302,3 +300,21 @@ func (self *ReposHelper) promptDirenvApproval(envrcPath string) { }, }) } + +// recentRepoMenuEntries is the menu's slice of the recent-repo list, skipping +// the head entry (the repo we are currently in). It reads the richer +// location list and falls back to the legacy plain-path list when the state +// file was last written by an older version (#5942). +func recentRepoMenuEntries(appState *config.AppState) []config.RecentRepoLocation { + if len(appState.RecentRepoLocations) > 0 { + return appState.RecentRepoLocations[1:] + } + locations := make([]config.RecentRepoLocation, 0, len(appState.RecentRepos)) + for _, repo := range appState.RecentRepos { + locations = append(locations, config.RecentRepoLocation{Path: repo}) + } + if len(locations) > 0 { + return locations[1:] + } + return nil +} diff --git a/pkg/gui/recent_repos_panel.go b/pkg/gui/recent_repos_panel.go index ba6bc8ce133..5899e715f37 100644 --- a/pkg/gui/recent_repos_panel.go +++ b/pkg/gui/recent_repos_panel.go @@ -3,41 +3,127 @@ package gui import ( "os" "path/filepath" + + "github.com/jesseduffield/lazygit/pkg/config" + "github.com/jesseduffield/lazygit/pkg/env" ) // updateRecentRepoList registers the fact that we opened lazygit in this repo, -// so that we can open the same repo via the 'recent repos' menu +// so that we can open the same repo via the 'recent repos' menu. +// +// Bare repos are still skipped (there is no work tree to change back to), but +// a repo whose git dir does not live at /.git — opened with +// --git-dir/--work-tree (yadm) or found through core.worktree (vcsh) — is now +// recorded with the environment it takes to reopen it (#5942). The plain-path +// RecentRepos list is still maintained in parallel with the repos git can +// find on its own, so an older lazygit reading the same state file keeps +// working. func (gui *Gui) updateRecentRepoList() error { if gui.git.Status.IsBareRepo() { - // we could totally do this but it would require storing both the git-dir and the - // worktree in our recent repos list, which is a change that would need to be - // backwards compatible gui.c.Log.Info("Not appending bare repo to recent repo list") return nil } - recentRepos := gui.c.GetAppState().RecentRepos currentRepo, err := os.Getwd() if err != nil { return err } - recentRepos = newRecentReposList(recentRepos, currentRepo) + + appState := gui.c.GetAppState() + + // Migration: an existing install has the old plain-path list. Fold it + // into the richer one so those entries keep working (their env is empty, + // which is correct for repos git finds from the work tree). + if len(appState.RecentRepoLocations) == 0 && len(appState.RecentRepos) > 0 { + appState.RecentRepoLocations = migrateRecentRepos(appState.RecentRepos, currentRepo) + } + + // The env we must restore to reopen THIS repo: empty for ordinary repos. + gitLocationEnvVars := env.GetGitLocationEnvVars() + + locations := newRecentRepoLocationsList( + appState.RecentRepoLocations, currentRepo, gitLocationEnvVars) + // The plain-path companion keeps only the repos git can find from the + // path alone, matching what the old list could represent. + plainRepos := []string{} + for _, location := range locations { + if len(location.GitLocationEnvVars) == 0 { + plainRepos = append(plainRepos, location.Path) + } + } + + appState.RecentRepoLocations = locations + appState.RecentRepos = plainRepos // TODO: migrate this file to use forward slashes on all OSes for consistency // (windows uses backslashes at the moment) - gui.c.GetAppState().RecentRepos = recentRepos return gui.c.SaveAppState() } -// newRecentReposList returns a new repo list with a new entry but only when it doesn't exist yet -func newRecentReposList(recentRepos []string, currentRepo string) []string { - newRepos := []string{currentRepo} +// migrateRecentRepos converts a legacy plain-path list into locations, +// prepending the current repo (the list's head is the repo we are in). +func migrateRecentRepos(recentRepos []string, currentRepo string) []config.RecentRepoLocation { + locations := []config.RecentRepoLocation{{Path: currentRepo}} for _, repo := range recentRepos { - if repo != currentRepo { - if _, err := os.Stat(filepath.Join(repo, ".git")); err != nil { + if repo == currentRepo { + continue + } + locations = append(locations, config.RecentRepoLocation{Path: repo}) + } + return locations +} + +// newRecentRepoLocationsList returns a new location list with the current +// repo's entry refreshed, keeping the others in order. Entries whose +// directory no longer exists are dropped, so a deleted repo ages out of the +// menu. A dotfile repo's entry is kept even though /.git does not +// exist: its GitLocationEnvVars are what make it openable again. +func newRecentRepoLocationsList( + recentLocations []config.RecentRepoLocation, + currentRepo string, + gitLocationEnvVars []string, +) []config.RecentRepoLocation { + newLocations := []config.RecentRepoLocation{{Path: currentRepo, GitLocationEnvVars: gitLocationEnvVars}} + for _, location := range recentLocations { + if location.Path == currentRepo { + continue + } + if _, err := os.Stat(location.Path); err != nil { + continue + } + if len(location.GitLocationEnvVars) == 0 { + // A legacy entry that had no env recorded: it was only kept in + // the old list when /.git existed, so preserve that + // contract for the plain entries. + if _, err := os.Stat(filepath.Join(location.Path, ".git")); err != nil { continue } - newRepos = append(newRepos, repo) } + newLocations = append(newLocations, location) + } + return newLocations +} + +// recentRepoLocationsOrLegacy is the accessor other components use: it +// prefers the richer list and falls back to the legacy plain-path list when +// the state file was last written by an older version after a rollback. +func recentRepoLocationsOrLegacy(appState *config.AppState) []config.RecentRepoLocation { + if len(appState.RecentRepoLocations) > 0 { + return appState.RecentRepoLocations + } + locations := make([]config.RecentRepoLocation, 0, len(appState.RecentRepos)) + for _, repo := range appState.RecentRepos { + locations = append(locations, config.RecentRepoLocation{Path: repo}) + } + return locations +} + + +// RecentRepoMenuEntries is the menu's slice of the recent-repo list, +// skipping the head entry (the repo we are currently in). +func RecentRepoMenuEntries(appState *config.AppState) []config.RecentRepoLocation { + locations := recentRepoLocationsOrLegacy(appState) + if len(locations) > 0 { + return locations[1:] } - return newRepos + return nil } diff --git a/pkg/gui/recent_repos_panel_test.go b/pkg/gui/recent_repos_panel_test.go new file mode 100644 index 00000000000..cc790142cfe --- /dev/null +++ b/pkg/gui/recent_repos_panel_test.go @@ -0,0 +1,79 @@ +package gui + +import ( + "os" + "path/filepath" + "testing" + + "github.com/jesseduffield/lazygit/pkg/config" +) + +// #5942: a repo whose git dir does not live at /.git used to fall +// out of the recent-repos list — newRecentReposList kept an entry only when +// /.git existed, so the next run in any other repo dropped it. +func TestNewRecentRepoLocationsListKeepsDotfileRepo(t *testing.T) { + dir := t.TempDir() + plainDir := t.TempDir() + plainGit := filepath.Join(plainDir, ".git") + if err := os.MkdirAll(plainGit, 0o755); err != nil { + t.Fatal(err) + } + dotfileDir := t.TempDir() // no .git inside — the env carries the repo + + locations := newRecentRepoLocationsList( + []config.RecentRepoLocation{ + {Path: plainDir}, + {Path: dotfileDir, GitLocationEnvVars: []string{"GIT_DIR=/x/y", "GIT_WORK_TREE=" + dotfileDir}}, + {Path: filepath.Join(dir, "gone")}, // directory no longer exists: ages out + }, + dir, + nil, + ) + + paths := []string{} + for _, location := range locations { + paths = append(paths, location.Path) + } + if len(paths) != 3 || paths[0] != dir { + t.Fatalf("unexpected head/got %v", paths) + } + foundDotfile := false + for _, location := range locations { + if location.Path == dotfileDir { + foundDotfile = true + if len(location.GitLocationEnvVars) != 2 { + t.Fatalf("dotfile entry lost its env: %+v", location) + } + } + } + if !foundDotfile { + t.Fatal("dotfile repo entry was dropped from the list") + } +} + +func TestNewRecentRepoLocationsListDropsLegacyPlainEntryWithoutDotGit(t *testing.T) { + dir := t.TempDir() + noGitDir := t.TempDir() // legacy entry with no env and no .git: was dropped before, still is + + locations := newRecentRepoLocationsList( + []config.RecentRepoLocation{{Path: noGitDir}}, + dir, + nil, + ) + + if len(locations) != 1 || locations[0].Path != dir { + t.Fatalf("legacy envless entry without .git must age out, got %+v", locations) + } +} + +func TestMigrateRecentReposPrependsCurrent(t *testing.T) { + locations := migrateRecentRepos([]string{"/a", "/b"}, "/current") + if len(locations) != 3 || locations[0].Path != "/current" { + t.Fatalf("migration must prepend the current repo, got %+v", locations) + } + for _, location := range locations[1:] { + if location.GitLocationEnvVars != nil { + t.Fatalf("legacy entries carry no env, got %+v", location) + } + } +}