diff --git a/cmd/gh/main.go b/cmd/gh/main.go index b9dc52858..d4459dd0c 100644 --- a/cmd/gh/main.go +++ b/cmd/gh/main.go @@ -264,7 +264,7 @@ func checkForUpdate(currentVersion string) (*update.ReleaseInfo, error) { } repo := updaterEnabled - stateFilePath := filepath.Join(config.ConfigDir(), "state.yml") + stateFilePath := filepath.Join(config.StateDir(), "state.yml") return update.CheckForUpdate(client, stateFilePath, repo, currentVersion) } diff --git a/internal/config/config_file.go b/internal/config/config_file.go index bcf475f64..e90d40ab0 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -16,7 +16,9 @@ import ( const ( GH_CONFIG_DIR = "GH_CONFIG_DIR" XDG_CONFIG_HOME = "XDG_CONFIG_HOME" + XDG_STATE_HOME = "XDG_STATE_HOME" APP_DATA = "AppData" + LOCAL_APP_DATA = "LocalAppData" ) // Config path precedence @@ -38,27 +40,100 @@ func ConfigDir() string { } // If the path does not exist try migrating config from default paths - if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { - autoMigrateConfigDir(path) + if !dirExists(path) { + _ = autoMigrateConfigDir(path) } return path } +// State path precedence +// 1. XDG_CONFIG_HOME +// 2. LocalAppData (windows only) +// 3. HOME +func StateDir() string { + var path string + if a := os.Getenv(XDG_STATE_HOME); a != "" { + path = filepath.Join(a, "gh") + } else if b := os.Getenv(LOCAL_APP_DATA); runtime.GOOS == "windows" && b != "" { + path = filepath.Join(b, "GitHub CLI") + } else { + c, _ := os.UserHomeDir() + path = filepath.Join(c, ".local", "state", "gh") + } + + // If the path does not exist try migrating state from default paths + if !dirExists(path) { + _ = autoMigrateStateDir(path) + } + + return path +} + +var errSamePath = errors.New("same path") +var errNotExist = errors.New("not exist") + // Check default paths (os.UserHomeDir, and homedir.Dir) for existing configs // If configs exist then move them to newPath // TODO: Remove support for homedir.Dir location in v2 -func autoMigrateConfigDir(newPath string) { +func autoMigrateConfigDir(newPath string) error { path, err := os.UserHomeDir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) - return + return migrateDir(oldPath, newPath) } path, err = homedir.Dir() if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { - migrateConfigDir(oldPath, newPath) + return migrateDir(oldPath, newPath) } + + return errNotExist +} + +// Check default paths (os.UserHomeDir, and homedir.Dir) for existing state file (state.yml) +// If state file exist then move it to newPath +// TODO: Remove support for homedir.Dir location in v2 +func autoMigrateStateDir(newPath string) error { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + return migrateFile(oldPath, newPath, "state.yml") + } + + return errNotExist +} + +func migrateFile(oldPath, newPath, file string) error { + if oldPath == newPath { + return errSamePath + } + + oldFile := filepath.Join(oldPath, file) + newFile := filepath.Join(newPath, file) + + if !fileExists(oldFile) { + return errNotExist + } + + _ = os.MkdirAll(filepath.Dir(newFile), 0755) + return os.Rename(oldFile, newFile) +} + +func migrateDir(oldPath, newPath string) error { + if oldPath == newPath { + return errSamePath + } + + if !dirExists(oldPath) { + return errNotExist + } + + _ = os.MkdirAll(filepath.Dir(newPath), 0755) + return os.Rename(oldPath, newPath) } func dirExists(path string) bool { @@ -66,13 +141,9 @@ func dirExists(path string) bool { return err == nil && f.IsDir() } -var migrateConfigDir = func(oldPath, newPath string) { - if oldPath == newPath { - return - } - - _ = os.MkdirAll(filepath.Dir(newPath), 0755) - _ = os.Rename(oldPath, newPath) +func fileExists(path string) bool { + f, err := os.Stat(path) + return err == nil && !f.IsDir() } func ConfigFile() string { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index 188f10ff0..e6150aae3 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -152,6 +152,8 @@ func Test_parseConfigFile(t *testing.T) { } func Test_ConfigDir(t *testing.T) { + tempDir := t.TempDir() + tests := []struct { name string onlyWindows bool @@ -159,63 +161,63 @@ func Test_ConfigDir(t *testing.T) { output string }{ { - name: "no envVars", + name: "HOME/USERPROFILE specified", env: map[string]string{ "GH_CONFIG_DIR": "", "XDG_CONFIG_HOME": "", "AppData": "", - "USERPROFILE": "", - "HOME": "", + "USERPROFILE": tempDir, + "HOME": tempDir, }, - output: ".config/gh", + output: filepath.Join(tempDir, ".config", "gh"), }, { name: "GH_CONFIG_DIR specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME specified", env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, { name: "GH_CONFIG_DIR and XDG_CONFIG_HOME specified", env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "XDG_CONFIG_HOME": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "XDG_CONFIG_HOME": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "AppData specified", onlyWindows: true, env: map[string]string{ - "AppData": "/tmp/", + "AppData": tempDir, }, - output: "/tmp/GitHub CLI", + output: filepath.Join(tempDir, "GitHub CLI"), }, { name: "GH_CONFIG_DIR and AppData specified", onlyWindows: true, env: map[string]string{ - "GH_CONFIG_DIR": "/tmp/gh_config_dir", - "AppData": "/tmp", + "GH_CONFIG_DIR": filepath.Join(tempDir, "gh_config_dir"), + "AppData": tempDir, }, - output: "/tmp/gh_config_dir", + output: filepath.Join(tempDir, "gh_config_dir"), }, { name: "XDG_CONFIG_HOME and AppData specified", onlyWindows: true, env: map[string]string{ - "XDG_CONFIG_HOME": "/tmp", - "AppData": "/tmp", + "XDG_CONFIG_HOME": tempDir, + "AppData": tempDir, }, - output: "/tmp/gh", + output: filepath.Join(tempDir, "gh"), }, } @@ -227,22 +229,25 @@ func Test_ConfigDir(t *testing.T) { if tt.env != nil { for k, v := range tt.env { old := os.Getenv(k) - os.Setenv(k, filepath.FromSlash(v)) + os.Setenv(k, v) defer os.Setenv(k, old) } } - defer stubMigrateConfigDir()() - assert.Equal(t, filepath.FromSlash(tt.output), ConfigDir()) + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, ConfigDir()) }) } } func Test_configFile_Write_toDisk(t *testing.T) { configDir := filepath.Join(t.TempDir(), ".config", "gh") + _ = os.MkdirAll(configDir, 0755) os.Setenv(GH_CONFIG_DIR, configDir) defer os.Unsetenv(GH_CONFIG_DIR) - defer stubMigrateConfigDir()() cfg := NewFromString(`pager: less`) err := cfg.Write() @@ -264,7 +269,8 @@ func Test_configFile_Write_toDisk(t *testing.T) { } } -func Test_autoMigrateConfigDir_noMigration(t *testing.T) { +func Test_autoMigrateConfigDir_noMigration_notExist(t *testing.T) { + homeDir := t.TempDir() migrateDir := t.TempDir() homeEnvVar := "HOME" @@ -272,10 +278,11 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, "/nonexistent-dir") + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err := autoMigrateConfigDir(migrateDir) + assert.Equal(t, errNotExist, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -283,17 +290,21 @@ func Test_autoMigrateConfigDir_noMigration(t *testing.T) { } func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { - migrateDir := t.TempDir() + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) homeEnvVar := "HOME" if runtime.GOOS == "windows" { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, migrateDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - autoMigrateConfigDir(migrateDir) + err = autoMigrateConfigDir(migrateDir) + assert.Equal(t, errSamePath, err) files, err := ioutil.ReadDir(migrateDir) assert.NoError(t, err) @@ -301,31 +312,175 @@ func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { } func Test_autoMigrateConfigDir_migration(t *testing.T) { - defaultDir := t.TempDir() - dd := filepath.Join(defaultDir, ".config", "gh") + homeDir := t.TempDir() migrateDir := t.TempDir() - md := filepath.Join(migrateDir, ".config", "gh") + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateConfigDir := filepath.Join(migrateDir, ".config", "gh") homeEnvVar := "HOME" if runtime.GOOS == "windows" { homeEnvVar = "USERPROFILE" } old := os.Getenv(homeEnvVar) - os.Setenv(homeEnvVar, defaultDir) + os.Setenv(homeEnvVar, homeDir) defer os.Setenv(homeEnvVar, old) - err := os.MkdirAll(dd, 0777) + err := os.MkdirAll(homeConfigDir, 0755) assert.NoError(t, err) - f, err := ioutil.TempFile(dd, "") + f, err := ioutil.TempFile(homeConfigDir, "") assert.NoError(t, err) f.Close() - autoMigrateConfigDir(md) + err = autoMigrateConfigDir(migrateConfigDir) + assert.NoError(t, err) - _, err = ioutil.ReadDir(dd) + _, err = ioutil.ReadDir(homeConfigDir) assert.True(t, os.IsNotExist(err)) - files, err := ioutil.ReadDir(md) + files, err := ioutil.ReadDir(migrateConfigDir) assert.NoError(t, err) assert.Equal(t, 1, len(files)) } + +func Test_StateDir(t *testing.T) { + tempDir := t.TempDir() + + tests := []struct { + name string + onlyWindows bool + env map[string]string + output string + }{ + { + name: "HOME/USERPROFILE specified", + env: map[string]string{ + "XDG_STATE_HOME": "", + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "LocalAppData": "", + "USERPROFILE": tempDir, + "HOME": tempDir, + }, + output: filepath.Join(tempDir, ".local", "state", "gh"), + }, + { + name: "XDG_STATE_HOME specified", + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + { + name: "LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "GitHub CLI"), + }, + { + name: "XDG_STATE_HOME and LocalAppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_STATE_HOME": tempDir, + "LocalAppData": tempDir, + }, + output: filepath.Join(tempDir, "gh"), + }, + } + + for _, tt := range tests { + if tt.onlyWindows && runtime.GOOS != "windows" { + continue + } + t.Run(tt.name, func(t *testing.T) { + if tt.env != nil { + for k, v := range tt.env { + old := os.Getenv(k) + os.Setenv(k, v) + defer os.Setenv(k, old) + } + } + + // Create directory to skip auto migration code + // which gets run when target directory does not exist + _ = os.MkdirAll(tt.output, 0755) + + assert.Equal(t, tt.output, StateDir()) + }) + } +} + +func Test_autoMigrateStateDir_noMigration_notExist(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := autoMigrateStateDir(migrateDir) + assert.Equal(t, errNotExist, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_noMigration_samePath(t *testing.T) { + homeDir := t.TempDir() + migrateDir := filepath.Join(homeDir, ".config", "gh") + err := os.MkdirAll(migrateDir, 0755) + assert.NoError(t, err) + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err = autoMigrateStateDir(migrateDir) + assert.Equal(t, errSamePath, err) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateStateDir_migration(t *testing.T) { + homeDir := t.TempDir() + migrateDir := t.TempDir() + homeConfigDir := filepath.Join(homeDir, ".config", "gh") + migrateStateDir := filepath.Join(migrateDir, ".local", "state", "gh") + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, homeDir) + defer os.Setenv(homeEnvVar, old) + + err := os.MkdirAll(homeConfigDir, 0755) + assert.NoError(t, err) + err = ioutil.WriteFile(filepath.Join(homeConfigDir, "state.yml"), nil, 0755) + assert.NoError(t, err) + + err = autoMigrateStateDir(migrateStateDir) + assert.NoError(t, err) + + files, err := ioutil.ReadDir(homeConfigDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) + + files, err = ioutil.ReadDir(migrateStateDir) + assert.NoError(t, err) + assert.Equal(t, 1, len(files)) + assert.Equal(t, "state.yml", files[0].Name()) +} diff --git a/internal/config/testing.go b/internal/config/testing.go index 1908ac30e..31a5fb2a8 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -62,11 +62,3 @@ func stubConfig(main, hosts string) func() { ReadConfigFile = orig } } - -func stubMigrateConfigDir() func() { - orig := migrateConfigDir - migrateConfigDir = func(_, _ string) {} - return func() { - migrateConfigDir = orig - } -} diff --git a/internal/update/update.go b/internal/update/update.go index 024fd2377..f525544e9 100644 --- a/internal/update/update.go +++ b/internal/update/update.go @@ -3,6 +3,8 @@ package update import ( "fmt" "io/ioutil" + "os" + "path/filepath" "regexp" "strconv" "strings" @@ -83,9 +85,14 @@ func setStateEntry(stateFilePath string, t time.Time, r ReleaseInfo) error { if err != nil { return err } - _ = ioutil.WriteFile(stateFilePath, content, 0600) - return nil + err = os.MkdirAll(filepath.Dir(stateFilePath), 0755) + if err != nil { + return err + } + + err = ioutil.WriteFile(stateFilePath, content, 0600) + return err } func versionGreaterThan(v, w string) bool {