Fix escape from submodule when repo was opened with --git-dir/--work-tree

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,30 +193,50 @@ 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)
 		}
 		return err
 	}
 
 	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 {

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{},
 

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

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):
+		// <root>
+		//  - .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()
+	},
+})

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,

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
+}