diff --git a/pkg/app/entry_point.go b/pkg/app/entry_point.go index a3225e311..14e13a739 100644 --- a/pkg/app/entry_point.go +++ b/pkg/app/entry_point.go @@ -136,6 +136,25 @@ func Start(buildInfo *BuildInfo, integrationTest integrationTypes.IntegrationTes } defer os.RemoveAll(tempDir) + // When git invokes lazygit as a daemon (e.g. as the editor of a rebase todo + // file), skip loading and migrating the user config entirely; the daemon + // doesn't need it, and migrating the config here would dirty the working + // tree in the middle of the git operation (see #5998). + if daemon.InDaemonMode() { + appConfig, err := config.NewAppConfigForDaemon("lazygit", buildInfo.Version, buildInfo.Commit, buildInfo.Date, buildInfo.BuildSource, cliArgs.Debug, tempDir) + if err != nil { + log.Fatal(err.Error()) + } + + common, err := NewCommon(appConfig) + if err != nil { + log.Fatal(err) + } + + daemon.Handle(common) + return + } + appConfig, err := config.NewAppConfig("lazygit", buildInfo.Version, buildInfo.Commit, buildInfo.Date, buildInfo.BuildSource, cliArgs.Debug, tempDir) if err != nil { log.Fatal(err.Error()) @@ -159,11 +178,6 @@ func Start(buildInfo *BuildInfo, integrationTest integrationTypes.IntegrationTes log.Fatal(err) } - if daemon.InDaemonMode() { - daemon.Handle(common) - return - } - if cliArgs.Profile { go func() { if err := http.ListenAndServe("localhost:6060", nil); err != nil { diff --git a/pkg/config/app_config.go b/pkg/config/app_config.go index ade614f7d..a041497af 100644 --- a/pkg/config/app_config.go +++ b/pkg/config/app_config.go @@ -130,6 +130,45 @@ func NewAppConfig( return appConfig, nil } +// NewAppConfigForDaemon makes a new app config for running lazygit in daemon +// mode, i.e. when git invokes lazygit as e.g. the editor of a rebase todo +// file. The daemon doesn't need the user config for anything, so it is neither +// loaded nor migrated here: migrating the config from within a git operation +// would modify the working tree mid-operation, aborting e.g. a rebase with +// "error: Your local changes to the following files would be overwritten by +// merge" when the config file is checked in (see #5998). +func NewAppConfigForDaemon( + name string, + version, + commit, + date string, + buildSource string, + debuggingFlag bool, + tempDir string, +) (*AppConfig, error) { + configDir, err := findOrCreateConfigDir() + if err != nil && !os.IsPermission(err) { + return nil, err + } + + appState, err := loadAppState() + if err != nil { + return nil, err + } + + return &AppConfig{ + name: name, + version: version, + buildDate: date, + debug: debuggingFlag, + buildSource: buildSource, + userConfig: GetDefaultConfigForPlatform(KeybindingPlatform()), + userConfigDir: configDir, + tempDir: tempDir, + appState: appState, + }, nil +} + func ConfigDir() string { _, filePath := findConfigFile(ConfigFilename) diff --git a/pkg/config/app_config_daemon_test.go b/pkg/config/app_config_daemon_test.go new file mode 100644 index 000000000..79bf88a43 --- /dev/null +++ b/pkg/config/app_config_daemon_test.go @@ -0,0 +1,41 @@ +package config + +import ( + "os" + "path/filepath" + "testing" + + "github.com/stretchr/testify/assert" +) + +// Invoking lazygit as a daemon (e.g. as the editor of a rebase todo file) must +// not load or migrate the user config: writing a migrated config back to disk +// dirties the working tree in the middle of the git operation, aborting a +// rebase whose repo has the config file checked in (see #5998). +func TestNewAppConfigForDaemonDoesNotTouchUserConfig(t *testing.T) { + stateDir := t.TempDir() + t.Setenv("CONFIG_DIR", stateDir) + + configPath := filepath.Join(stateDir, ConfigFilename) + preMigrationConfig := "git:\n pagers:\n - pager: less\n" + if err := os.WriteFile(configPath, []byte(preMigrationConfig), 0o644); err != nil { + t.Fatal(err) + } + + appConfig, err := NewAppConfigForDaemon("lazygit", "test-version", "test-commit", "test-date", "test-build", false, t.TempDir()) + assert.NoError(t, err) + if appConfig == nil { + t.Fatal("expected non-nil app config") + } + + // The config file must be untouched (not created, not migrated, not + // rewritten). + content, err := os.ReadFile(configPath) + assert.NoError(t, err) + assert.Equal(t, preMigrationConfig, string(content)) + + // The daemon still gets a usable config object with defaults, and usable + // app state. + assert.NotNil(t, appConfig.GetUserConfig()) + assert.NotNil(t, appConfig.GetAppState()) +}