From 2d61c048392f2b646070cae2f8be0372a44bae50 Mon Sep 17 00:00:00 2001 From: xooooooooox Date: Thu, 6 Aug 2026 20:43:30 +0800 Subject: [PATCH] Restore GIT_DIR/GIT_WORK_TREE when switching back to the parent repo Entering a submodule clears the GIT_DIR/GIT_WORK_TREE env vars, but returning only restored the working directory. For repos opened via --git-dir/--work-tree (dotfile-style bare repos, e.g. yadm/vcsh), the parent repo's git dir cannot be rediscovered from the path alone, so escape failed with 'not a git repository' -- or silently switched to an enclosing repo when one existed above the worktree. The repo-path stack now records the env vars alongside the path (StringStack generalized to Stack[RepoLocation]), and switching back restores them. Failed switches restore the previous env vars so the repo we stay in keeps working. Fixes #1118 --- pkg/gui/controllers/helpers/repos_helper.go | 58 +++++++++++++--- pkg/gui/gui.go | 8 +-- pkg/gui/types/common.go | 13 +++- .../submodule/enter_dotfile_bare_repo.go | 68 +++++++++++++++++++ pkg/integration/tests/test_list.go | 1 + pkg/utils/stack.go | 29 ++++++++ pkg/utils/string_stack.go | 27 -------- 7 files changed, 161 insertions(+), 43 deletions(-) create mode 100644 pkg/integration/tests/submodule/enter_dotfile_bare_repo.go create mode 100644 pkg/utils/stack.go delete mode 100644 pkg/utils/string_stack.go diff --git a/pkg/gui/controllers/helpers/repos_helper.go b/pkg/gui/controllers/helpers/repos_helper.go index a61ad001331..d67f1efea1f 100644 --- a/pkg/gui/controllers/helpers/repos_helper.go +++ b/pkg/gui/controllers/helpers/repos_helper.go @@ -54,7 +54,11 @@ func (self *ReposHelper) EnterSubmodule(submodule *models.SubmoduleConfig) error if err != nil { return err } - self.c.State().GetRepoPathStack().Push(wd) + self.c.State().GetRepoPathStack().Push(types.RepoLocation{ + Path: wd, + GitDirEnv: env.GetGitDirEnv(), + WorkTreeEnv: env.GetWorkTreeEnv(), + }) return self.switchTo(submodule.FullPath(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } @@ -164,7 +168,7 @@ func (self *ReposHelper) SwitchToParentRepo() error { if self.switchRefusedBecauseBusy() { return nil } - return self.switchTo(self.c.State().GetRepoPathStack().Pop(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) + return self.switchToLocation(self.c.State().GetRepoPathStack().Pop(), self.c.Tr.ErrRepositoryMovedOrDeleted, context.NO_CONTEXT) } func (self *ReposHelper) DispatchSwitchTo(path string, errMsg string, contextKey types.ContextKey) error { @@ -189,23 +193,42 @@ func (self *ReposHelper) switchRefusedBecauseBusy() bool { return false } -// switchTo switches lazygit to the repository (or worktree) at the given path. -// It runs synchronously on the UI thread: the switch swaps gui.State (in -// resetState) and reassigns gui.git and the process cwd, all of which the UI -// thread also reads, so doing it here rather than on a worker avoids racing -// those reads. The heavy data loading is still dispatched asynchronously by the -// refresh that onNewRepo kicks off. +// switchTo switches lazygit to the repository (or worktree) at the given path, +// discovering its git dir from the path alone (the GIT_DIR/GIT_WORK_TREE env +// vars are cleared). func (self *ReposHelper) switchTo(path string, errMsg string, contextKey types.ContextKey) error { - env.UnsetGitLocationEnvVars() + return self.switchToLocation(types.RepoLocation{Path: path}, errMsg, contextKey) +} + +// switchToLocation switches lazygit to the repository (or worktree) at the +// given location. It runs synchronously on the UI thread: the switch swaps +// gui.State (in resetState) and reassigns gui.git and the process cwd, all of +// which the UI thread also reads, so doing it here rather than on a worker +// avoids racing those reads. The heavy data loading is still dispatched +// asynchronously by the refresh that onNewRepo kicks off. +// +// The location's GitDirEnv/WorkTreeEnv replace the process env vars: most +// switches clear them (a freshly entered repo is discovered from its working +// directory), but switching back to a repo that was opened via +// --git-dir/--work-tree (e.g. a dotfile bare repo) must restore them, because +// its git dir cannot be discovered from the path alone (#1118). A failed +// switch puts the previous env vars back, so the repo we stay in keeps +// working. +func (self *ReposHelper) switchToLocation(location types.RepoLocation, errMsg string, contextKey types.ContextKey) error { + previousGitDirEnv := env.GetGitDirEnv() + previousWorkTreeEnv := env.GetWorkTreeEnv() + setGitLocationEnvVars(location.GitDirEnv, location.WorkTreeEnv) + originalPath, err := os.Getwd() if err != nil { return nil } - msg := utils.ResolvePlaceholderString(self.c.Tr.ChangingDirectoryTo, map[string]string{"path": path}) + msg := utils.ResolvePlaceholderString(self.c.Tr.ChangingDirectoryTo, map[string]string{"path": location.Path}) self.c.LogCommand(msg, false) - if err := os.Chdir(path); err != nil { + if err := os.Chdir(location.Path); err != nil { + setGitLocationEnvVars(previousGitDirEnv, previousWorkTreeEnv) if os.IsNotExist(err) { return errors.New(errMsg) } @@ -213,6 +236,7 @@ func (self *ReposHelper) switchTo(path string, errMsg string, contextKey types.C } if err := commands.VerifyInGitRepo(self.c.OS()); err != nil { + setGitLocationEnvVars(previousGitDirEnv, previousWorkTreeEnv) if err := os.Chdir(originalPath); err != nil { return err } @@ -238,6 +262,18 @@ func (self *ReposHelper) switchTo(path string, errMsg string, contextKey types.C return direnvResult.Err } +// setGitLocationEnvVars makes the process env vars match a RepoLocation: +// GIT_DIR/GIT_WORK_TREE are cleared, then set to the given values if non-empty. +func setGitLocationEnvVars(gitDirEnv string, workTreeEnv string) { + env.UnsetGitLocationEnvVars() + if gitDirEnv != "" { + env.SetGitDirEnv(gitDirEnv) + } + if workTreeEnv != "" { + env.SetWorkTreeEnv(workTreeEnv) + } +} + // logDirenvResult writes whatever direnv emitted to the command log and the // debug log; both happen for every load attempt regardless of outcome. func (self *ReposHelper) logDirenvResult(result direnv.LoadResult) direnv.LoadResult { diff --git a/pkg/gui/gui.go b/pkg/gui/gui.go index a7466d584d1..48ed4497404 100644 --- a/pkg/gui/gui.go +++ b/pkg/gui/gui.go @@ -94,9 +94,9 @@ type Gui struct { Mutexes types.Mutexes - // when you enter into a submodule we'll append the superproject's path to this array + // when you enter into a submodule we'll append the superproject's location to this array // so that you can return to the superproject - RepoPathStack *utils.StringStack + RepoPathStack *utils.Stack[types.RepoLocation] // this tells us whether our views have been initially set up ViewsSetup bool @@ -158,7 +158,7 @@ type StateAccessor struct { var _ types.IStateAccessor = new(StateAccessor) -func (self *StateAccessor) GetRepoPathStack() *utils.StringStack { +func (self *StateAccessor) GetRepoPathStack() *utils.Stack[types.RepoLocation] { return self.gui.RepoPathStack } @@ -796,7 +796,7 @@ func NewGui( viewBufferManagerMap: map[string]*tasks.ViewBufferManager{}, viewPtmxMap: map[string]oscommands.Pty{}, showRecentRepos: showRecentRepos, - RepoPathStack: &utils.StringStack{}, + RepoPathStack: &utils.Stack[types.RepoLocation]{}, RepoStateMap: map[Repo]*GuiRepoState{}, GuiLog: []string{}, diff --git a/pkg/gui/types/common.go b/pkg/gui/types/common.go index b2b924bc0b5..4b377620f59 100644 --- a/pkg/gui/types/common.go +++ b/pkg/gui/types/common.go @@ -403,8 +403,19 @@ type HasUrn interface { URN() string } +// RepoLocation identifies a repo the way it was opened: the working directory, +// plus the GIT_DIR/GIT_WORK_TREE env vars that were in effect at the time (both +// empty for repos discovered from the working directory alone). Keeping the env +// vars is what lets us switch back to a bare repo opened via +// --git-dir/--work-tree, whose git dir cannot be rediscovered from the path. +type RepoLocation struct { + Path string + GitDirEnv string + WorkTreeEnv string +} + type IStateAccessor interface { - GetRepoPathStack() *utils.StringStack + GetRepoPathStack() *utils.Stack[RepoLocation] GetRepoState() IRepoStateAccessor GetDiffRendererConfigManager() *config.DiffRendererConfigManager // tells us whether we're currently updating lazygit diff --git a/pkg/integration/tests/submodule/enter_dotfile_bare_repo.go b/pkg/integration/tests/submodule/enter_dotfile_bare_repo.go new file mode 100644 index 00000000000..7820945baa5 --- /dev/null +++ b/pkg/integration/tests/submodule/enter_dotfile_bare_repo.go @@ -0,0 +1,68 @@ +package submodule + +import ( + "github.com/jesseduffield/lazygit/pkg/config" + . "github.com/jesseduffield/lazygit/pkg/integration/components" +) + +// Regression test for https://github.com/jesseduffield/lazygit/issues/1118: +// when lazygit is launched against a dotfile-style bare repo via +// --git-dir/--work-tree (the yadm/vcsh setup), entering a submodule and +// pressing escape must return to the parent repo. The parent repo is only +// discoverable through the GIT_DIR/GIT_WORK_TREE env vars, so escape fails +// with "not a git repository" if those aren't restored on the way back. +var EnterDotfileBareRepo = NewIntegrationTest(NewIntegrationTestArgs{ + Description: "Enter a submodule of a dotfile bare repo (--git-dir/--work-tree) and escape back out", + ExtraCmdArgs: []string{"--git-dir={{.actualPath}}/.bare", "--work-tree={{.actualPath}}/repo"}, + Skip: false, + SetupConfig: func(config *config.AppConfig) {}, + SetupRepo: func(shell *Shell) { + // directory structure (like worktree/dotfile_bare_repo.go): + // + // - .bare (the git dir; only reachable via GIT_DIR env var) + // - repo (the worktree; has no .git entry at all) + // - my_submodule_name (bare clone serving as the submodule's remote) + + // create a repo to act as the submodule's remote, using the default + // .git dir that every test repo starts with + shell.EmptyCommit("submodule initial commit") + shell.Clone("my_submodule_name") + + // now turn the test repo into a dotfile-style bare repo + shell.DeleteFile(".git") + shell.RunCommand([]string{"git", "init", "--bare", "../.bare"}) + shell.RunCommand([]string{"git", "--git-dir=../.bare", "--work-tree=.", "checkout", "-b", "mybranch"}) + shell.CreateFile("blah", "blah\n") + shell.RunCommand([]string{"git", "--git-dir=../.bare", "--work-tree=.", "add", "blah"}) + shell.RunCommand([]string{"git", "--git-dir=../.bare", "--work-tree=.", "commit", "-m", "initial commit"}) + shell.RunCommand([]string{"git", "--git-dir=../.bare", "--work-tree=.", "-c", "protocol.file.allow=always", "submodule", "add", "--name", "my_submodule_name", "../my_submodule_name", "my_submodule_path"}) + shell.RunCommand([]string{"git", "--git-dir=../.bare", "--work-tree=.", "commit", "-m", "add submodule"}) + }, + Run: func(t *TestDriver, keys config.KeybindingConfig) { + assertInParentRepo := func() { + t.Views().Status().Content(Contains("mybranch")) + } + assertInSubmodule := func() { + t.Views().Status().Content(Contains("(my_submodule_name)")) + } + + assertInParentRepo() + + t.Views().Submodules().Focus(). + Lines( + Contains("my_submodule_name").IsSelected(), + ). + // enter the submodule + PressEnter() + + assertInSubmodule() + + t.Views().Files().IsFocused(). + // return to the parent repo + PressEscape() + + assertInParentRepo() + + t.Views().Submodules().IsFocused() + }, +}) diff --git a/pkg/integration/tests/test_list.go b/pkg/integration/tests/test_list.go index a6b5aeafdf0..361b0841c20 100644 --- a/pkg/integration/tests/test_list.go +++ b/pkg/integration/tests/test_list.go @@ -441,6 +441,7 @@ var tests = []*components.IntegrationTest{ status.LogCmdStatusPanelAllBranchesLog, submodule.Add, submodule.Enter, + submodule.EnterDotfileBareRepo, submodule.EnterNested, submodule.Remove, submodule.RemoveNested, diff --git a/pkg/utils/stack.go b/pkg/utils/stack.go new file mode 100644 index 00000000000..4e37f94fcec --- /dev/null +++ b/pkg/utils/stack.go @@ -0,0 +1,29 @@ +package utils + +type Stack[T any] struct { + stack []T +} + +func (self *Stack[T]) Push(v T) { + self.stack = append(self.stack, v) +} + +// Pop returns the zero value of T if the stack is empty. +func (self *Stack[T]) Pop() T { + var v T + if len(self.stack) == 0 { + return v + } + n := len(self.stack) - 1 + v = self.stack[n] + self.stack = self.stack[:n] + return v +} + +func (self *Stack[T]) IsEmpty() bool { + return len(self.stack) == 0 +} + +func (self *Stack[T]) Clear() { + self.stack = nil +} diff --git a/pkg/utils/string_stack.go b/pkg/utils/string_stack.go deleted file mode 100644 index c2d18c70cb0..00000000000 --- a/pkg/utils/string_stack.go +++ /dev/null @@ -1,27 +0,0 @@ -package utils - -type StringStack struct { - stack []string -} - -func (self *StringStack) Push(s string) { - self.stack = append(self.stack, s) -} - -func (self *StringStack) Pop() string { - if len(self.stack) == 0 { - return "" - } - n := len(self.stack) - 1 - last := self.stack[n] - self.stack = self.stack[:n] - return last -} - -func (self *StringStack) IsEmpty() bool { - return len(self.stack) == 0 -} - -func (self *StringStack) Clear() { - self.stack = []string{} -}