From 0d49bfba42f2e77865a08565b18cffe2a9417261 Mon Sep 17 00:00:00 2001 From: Sam Coe Date: Wed, 19 May 2021 10:26:38 -0400 Subject: [PATCH] Add support for XDG_CONFIG_HOME and AppData on Windows --- internal/config/config_file.go | 90 +++++++++------- internal/config/config_file_test.go | 152 ++++++++++++++++++++++++++-- internal/config/from_env_test.go | 3 + internal/config/testing.go | 8 ++ 4 files changed, 209 insertions(+), 44 deletions(-) diff --git a/internal/config/config_file.go b/internal/config/config_file.go index e3d7d2d52..bcf475f64 100644 --- a/internal/config/config_file.go +++ b/internal/config/config_file.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "syscall" "github.com/mitchellh/go-homedir" @@ -13,16 +14,65 @@ import ( ) const ( - GH_CONFIG_DIR = "GH_CONFIG_DIR" + GH_CONFIG_DIR = "GH_CONFIG_DIR" + XDG_CONFIG_HOME = "XDG_CONFIG_HOME" + APP_DATA = "AppData" ) +// Config path precedence +// 1. GH_CONFIG_DIR +// 2. XDG_CONFIG_HOME +// 3. AppData (windows only) +// 4. HOME func ConfigDir() string { - if v := os.Getenv(GH_CONFIG_DIR); v != "" { - return v + var path string + if a := os.Getenv(GH_CONFIG_DIR); a != "" { + path = a + } else if b := os.Getenv(XDG_CONFIG_HOME); b != "" { + path = filepath.Join(b, "gh") + } else if c := os.Getenv(APP_DATA); runtime.GOOS == "windows" && c != "" { + path = filepath.Join(c, "GitHub CLI") + } else { + d, _ := os.UserHomeDir() + path = filepath.Join(d, ".config", "gh") } - homeDir, _ := homeDirAutoMigrate() - return homeDir + // If the path does not exist try migrating config from default paths + if _, err := os.Stat(path); errors.Is(err, os.ErrNotExist) { + autoMigrateConfigDir(path) + } + + return path +} + +// 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) { + path, err := os.UserHomeDir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + migrateConfigDir(oldPath, newPath) + return + } + + path, err = homedir.Dir() + if oldPath := filepath.Join(path, ".config", "gh"); err == nil && dirExists(oldPath) { + migrateConfigDir(oldPath, newPath) + } +} + +func dirExists(path string) bool { + f, err := os.Stat(path) + 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 ConfigFile() string { @@ -63,36 +113,6 @@ func HomeDirPath(subdir string) (string, error) { return newPath, nil } -// Looks up the `~/.config/gh` directory with backwards-compatibility with go-homedir and auto-migration -// when an old homedir location was found. -func homeDirAutoMigrate() (string, error) { - homeDir, err := os.UserHomeDir() - if err != nil { - // TODO: remove go-homedir fallback in GitHub CLI v2 - if legacyDir, err := homedir.Dir(); err == nil { - return filepath.Join(legacyDir, ".config", "gh"), nil - } - return "", err - } - - newPath := filepath.Join(homeDir, ".config", "gh") - _, newPathErr := os.Stat(newPath) - if newPathErr == nil || !os.IsNotExist(err) { - return newPath, newPathErr - } - - // TODO: remove go-homedir fallback in GitHub CLI v2 - if legacyDir, err := homedir.Dir(); err == nil { - legacyPath := filepath.Join(legacyDir, ".config", "gh") - if s, err := os.Stat(legacyPath); err == nil && s.IsDir() { - _ = os.MkdirAll(filepath.Dir(newPath), 0755) - return newPath, os.Rename(legacyPath, newPath) - } - } - - return newPath, nil -} - var ReadConfigFile = func(filename string) ([]byte, error) { f, err := os.Open(filename) if err != nil { diff --git a/internal/config/config_file_test.go b/internal/config/config_file_test.go index c7343b7ea..924fb6327 100644 --- a/internal/config/config_file_test.go +++ b/internal/config/config_file_test.go @@ -6,6 +6,7 @@ import ( "io/ioutil" "os" "path/filepath" + "runtime" "testing" "github.com/stretchr/testify/assert" @@ -152,20 +153,87 @@ func Test_parseConfigFile(t *testing.T) { func Test_ConfigDir(t *testing.T) { tests := []struct { - envVar string - want string + name string + onlyWindows bool + env map[string]string + output string }{ - {"/tmp/gh", ".tmp.gh"}, - {"", ".config.gh"}, + { + name: "no envVars", + env: map[string]string{ + "GH_CONFIG_DIR": "", + "XDG_CONFIG_HOME": "", + "AppData": "", + "USERPROFILE": "", + "HOME": "", + }, + output: ".config/gh", + }, + { + name: "GH_CONFIG_DIR specified", + env: map[string]string{ + "GH_CONFIG_DIR": "/tmp/gh_config_dir", + }, + output: "/tmp/gh_config_dir", + }, + { + name: "XDG_CONFIG_HOME specified", + env: map[string]string{ + "XDG_CONFIG_HOME": "/tmp", + }, + output: "/tmp/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", + }, + output: "/tmp/gh_config_dir", + }, + { + name: "AppData specified", + onlyWindows: true, + env: map[string]string{ + "AppData": "/tmp/", + }, + output: "/tmp/GitHub CLI", + }, + { + name: "GH_CONFIG_DIR and AppData specified", + onlyWindows: true, + env: map[string]string{ + "GH_CONFIG_DIR": "/tmp/gh_config_dir", + "AppData": "/tmp", + }, + output: "/tmp/gh_config_dir", + }, + { + name: "XDG_CONFIG_HOME and AppData specified", + onlyWindows: true, + env: map[string]string{ + "XDG_CONFIG_HOME": "/tmp", + "AppData": "/tmp", + }, + output: "/tmp/gh", + }, } for _, tt := range tests { - t.Run(fmt.Sprintf("envVar: %q", tt.envVar), func(t *testing.T) { - if tt.envVar != "" { - os.Setenv(GH_CONFIG_DIR, tt.envVar) - defer os.Unsetenv(GH_CONFIG_DIR) + 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, filepath.FromSlash(v)) + defer os.Setenv(k, old) + } } - assert.Regexp(t, tt.want, ConfigDir()) + + defer stubMigrateConfigDir()() + assert.Equal(t, filepath.FromSlash(tt.output), ConfigDir()) }) } } @@ -194,3 +262,69 @@ func Test_configFile_Write_toDisk(t *testing.T) { t.Errorf("unexpected hosts.yml: %q", string(configBytes)) } } + +func Test_autoMigrateConfigDir_noMigration(t *testing.T) { + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, "/nonexistent-dir") + defer os.Setenv(homeEnvVar, old) + + autoMigrateConfigDir(migrateDir) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateConfigDir_noMigration_samePath(t *testing.T) { + migrateDir := t.TempDir() + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, migrateDir) + defer os.Setenv(homeEnvVar, old) + + autoMigrateConfigDir(migrateDir) + + files, err := ioutil.ReadDir(migrateDir) + assert.NoError(t, err) + assert.Equal(t, 0, len(files)) +} + +func Test_autoMigrateConfigDir_migration(t *testing.T) { + defaultDir := t.TempDir() + dd := filepath.Join(defaultDir, ".config", "gh") + migrateDir := t.TempDir() + md := filepath.Join(migrateDir, ".config", "gh") + + homeEnvVar := "HOME" + if runtime.GOOS == "windows" { + homeEnvVar = "USERPROFILE" + } + old := os.Getenv(homeEnvVar) + os.Setenv(homeEnvVar, defaultDir) + defer os.Setenv(homeEnvVar, old) + + err := os.MkdirAll(dd, 0777) + assert.NoError(t, err) + f, err := ioutil.TempFile(dd, "") + assert.NoError(t, err) + f.Close() + + autoMigrateConfigDir(md) + + _, err = ioutil.ReadDir(dd) + assert.True(t, os.IsNotExist(err)) + + files, err := ioutil.ReadDir(md) + assert.NoError(t, err) + assert.Equal(t, 1, len(files)) +} diff --git a/internal/config/from_env_test.go b/internal/config/from_env_test.go index a5e03edd3..a3ad8fee4 100644 --- a/internal/config/from_env_test.go +++ b/internal/config/from_env_test.go @@ -13,11 +13,13 @@ func TestInheritEnv(t *testing.T) { orig_GITHUB_ENTERPRISE_TOKEN := os.Getenv("GITHUB_ENTERPRISE_TOKEN") orig_GH_TOKEN := os.Getenv("GH_TOKEN") orig_GH_ENTERPRISE_TOKEN := os.Getenv("GH_ENTERPRISE_TOKEN") + orig_AppData := os.Getenv("AppData") t.Cleanup(func() { os.Setenv("GITHUB_TOKEN", orig_GITHUB_TOKEN) os.Setenv("GITHUB_ENTERPRISE_TOKEN", orig_GITHUB_ENTERPRISE_TOKEN) os.Setenv("GH_TOKEN", orig_GH_TOKEN) os.Setenv("GH_ENTERPRISE_TOKEN", orig_GH_ENTERPRISE_TOKEN) + os.Setenv("AppData", orig_AppData) }) type wants struct { @@ -264,6 +266,7 @@ func TestInheritEnv(t *testing.T) { os.Setenv("GITHUB_ENTERPRISE_TOKEN", tt.GITHUB_ENTERPRISE_TOKEN) os.Setenv("GH_TOKEN", tt.GH_TOKEN) os.Setenv("GH_ENTERPRISE_TOKEN", tt.GH_ENTERPRISE_TOKEN) + os.Setenv("AppData", "") baseCfg := NewFromString(tt.baseConfig) cfg := InheritEnv(baseCfg) diff --git a/internal/config/testing.go b/internal/config/testing.go index 31a5fb2a8..1908ac30e 100644 --- a/internal/config/testing.go +++ b/internal/config/testing.go @@ -62,3 +62,11 @@ func stubConfig(main, hosts string) func() { ReadConfigFile = orig } } + +func stubMigrateConfigDir() func() { + orig := migrateConfigDir + migrateConfigDir = func(_, _ string) {} + return func() { + migrateConfigDir = orig + } +}