Merge pull request #153 from cli/wm/support-xdg-cache-home

Attempt to use user configured dirs for caching
cli · Mar 19, 2024 · 288843f · 288843f
2 parents 45fa8a4 + 72d06aa
commit 288843f
Show file tree

Hide file tree

Showing 2 changed files with 82 additions and 5 deletions.
diff --git a/pkg/config/config.go b/pkg/config/config.go
@@ -21,6 +21,7 @@ const (
 	xdgConfigHome = "XDG_CONFIG_HOME"
 	xdgDataHome   = "XDG_DATA_HOME"
 	xdgStateHome  = "XDG_STATE_HOME"
+	xdgCacheHome  = "XDG_CACHE_HOME"
 )
 
 var (
@@ -287,9 +288,21 @@ func DataDir() string {
 	return path
 }
 
-// CacheDir returns the default path for gh cli cache.
+// Cache path precedence: XDG_CACHE_HOME, LocalAppData (windows only), HOME, legacy gh-cli-cache.
 func CacheDir() string {
-	return filepath.Join(os.TempDir(), "gh-cli-cache")
+	if a := os.Getenv(xdgCacheHome); a != "" {
+		return filepath.Join(a, "gh")
+	} else if b := os.Getenv(localAppData); runtime.GOOS == "windows" && b != "" {
+		return filepath.Join(b, "GitHub CLI")
+	} else if c, err := os.UserHomeDir(); err == nil {
+		return filepath.Join(c, ".cache", "gh")
+	} else {
+		// Note that this has a minor security issue because /tmp is world-writeable.
+		// As such, it is possible for other users on a shared system to overwrite cached data.
+		// The practical risk of this is low, but it's worth calling out as a risk.
+		// I've included this here for backwards compatibility but we should consider removing it.
+		return filepath.Join(os.TempDir(), "gh-cli-cache")
+	}
 }
 
 func readFile(filename string) ([]byte, error) {

diff --git a/pkg/config/config_test.go b/pkg/config/config_test.go
@@ -221,9 +221,73 @@ func TestDataDir(t *testing.T) {
 }
 
 func TestCacheDir(t *testing.T) {
-	expected := filepath.Join(os.TempDir(), "gh-cli-cache")
-	actual := CacheDir()
-	assert.Equal(t, expected, actual)
+	expectedCacheDir := "/expected-cache-dir"
+	unexpectedCacheDir := "/unexpected-cache-dir"
+
+	tests := []struct {
+		name        string
+		onlyWindows bool
+		env         map[string]string
+		output      string
+	}{
+		{
+			name: "XDG_CACHE_HOME is highest precedence",
+			env: map[string]string{
+				"XDG_CACHE_HOME": expectedCacheDir,
+				"LocalAppData":   unexpectedCacheDir,
+				"USERPROFILE":    unexpectedCacheDir,
+				"HOME":           unexpectedCacheDir,
+			},
+			output: filepath.Join(expectedCacheDir, "gh"),
+		},
+		{
+			name:        "on windows, LocalAppData is preferred to home dir",
+			onlyWindows: true,
+			env: map[string]string{
+				"XDG_CACHE_HOME": "",
+				"LocalAppData":   expectedCacheDir,
+				"USERPROFILE":    unexpectedCacheDir,
+				"HOME":           unexpectedCacheDir,
+			},
+			output: filepath.Join(expectedCacheDir, "GitHub CLI"),
+		},
+		{
+			name: "tries to use the home dir cache directory",
+			env: map[string]string{
+				"XDG_CACHE_HOME": "",
+				"LocalAppData":   "",
+				"USERPROFILE":    expectedCacheDir,
+				"HOME":           expectedCacheDir,
+			},
+			output: filepath.Join(expectedCacheDir, ".cache", "gh"),
+		},
+		{
+			name: "finally falls back to tmpdir",
+			// We set the env vars to empty strings so that no home dir should be found
+			env: map[string]string{
+				"XDG_CACHE_HOME": "",
+				"LocalAppData":   "",
+				"USERPROFILE":    "",
+				"HOME":           "",
+			},
+			output: filepath.Join(os.TempDir(), "gh-cli-cache"),
+		},
+	}
+
+	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 {
+					t.Setenv(k, v)
+				}
+			}
+			assert.Equal(t, tt.output, CacheDir())
+		})
+	}
+
 }
 
 func TestLoad(t *testing.T) {