Run git in literal-pathspec mode

app/diff/diff.go

@@ -10,6 +10,7 @@ import (
 	"os/exec"
 	"path/filepath"
 	"regexp"
+	"slices"
 	"strconv"
 	"strings"
 	"time"
@@ -42,8 +43,39 @@ const (
 	// BinaryPlaceholder is the content used for binary file placeholders.
 	// parseUnifiedDiff returns this when git reports "Binary files ... differ".
 	BinaryPlaceholder = "(binary file)"
+
+	// literalPathspecs makes git treat every path argument as a literal filename.
+	// Without it a tracked file whose name begins with ":(" or contains a glob
+	// character is parsed as a pathspec expression and selects a different file,
+	// or no file at all.
+	literalPathspecs = "GIT_LITERAL_PATHSPECS=1"
 )
 
+// conflictingPathspecEnv lists the global pathspec settings git refuses to combine
+// with literal mode: with either of them set truthy in the environment every
+// command dies with "global 'literal' pathspec setting is incompatible with all
+// other global pathspec settings". GIT_NOGLOB_PATHSPECS is absent on purpose,
+// git accepts it alongside literal mode.
+var conflictingPathspecEnv = []string{"GIT_GLOB_PATHSPECS=", "GIT_ICASE_PATHSPECS="}
+
+// GitEnv returns the environment for a child git process: the current environment
+// with literal pathspec handling forced on and the settings that conflict with it
+// removed. Paths reach git straight from the VCS listing or from the user, so a
+// file actually named ":(top)x" or "*.go" must select itself rather than be parsed
+// as a pathspec expression; nothing here relies on pathspec magic. Dropping the
+// conflicting entries is harmless, since neither affects a literal path.
+func GitEnv() []string {
+	env := os.Environ()
+	out := make([]string, 0, len(env)+1)
+	for _, kv := range env {
+		if slices.ContainsFunc(conflictingPathspecEnv, func(p string) bool { return strings.HasPrefix(kv, p) }) {
+			continue
+		}
+		out = append(out, kv)
+	}
+	return append(out, literalPathspecs)
+}
+
 // DiffLine holds parsed line info from a diff.
 type DiffLine struct {
 	OldNum        int        // line number in old version (0 for additions)
@@ -522,12 +554,11 @@ func (g *Git) tempIndexWithIntentToAdd(paths []string) (indexPath string, cleanu
 	return tmpPath, cleanup, nil
 }
 
-// renameIndexEnv builds the environment for git commands in the untracked-rename
-// path: GIT_INDEX_FILE points at the throwaway index, and GIT_LITERAL_PATHSPECS
-// makes git treat path arguments literally so a working-tree filename that looks
-// like pathspec magic (e.g. ":(top)x") is not misinterpreted.
+// renameIndexEnv builds the extra environment for git commands in the
+// untracked-rename path: GIT_INDEX_FILE points at the throwaway index. Literal
+// pathspec handling comes from GitEnv, which every git call goes through.
 func (g *Git) renameIndexEnv(indexPath string) []string {
-	return []string{"GIT_INDEX_FILE=" + indexPath, "GIT_LITERAL_PATHSPECS=1"}
+	return []string{"GIT_INDEX_FILE=" + indexPath}
 }
 
 // FileDiff returns the diff view for a single file.
@@ -681,28 +712,26 @@ func (g *Git) diffArgs(ref string, staged bool) []string {
 
 // runGit executes a git command in the working directory and returns its output.
 func (g *Git) runGit(args ...string) (string, error) {
-	return runVCS(g.workDir, "git", args...)
+	return runVCSEnv(g.workDir, GitEnv(), "git", args...)
 }
 
-// runGitEnv runs git with extra environment entries (e.g. GIT_INDEX_FILE) appended
-// to the process environment, used by the throwaway-index rename detection path.
+// runGitEnv runs git with extra environment entries (e.g. GIT_INDEX_FILE) on top of
+// GitEnv, used by the throwaway-index rename detection path.
 func (g *Git) runGitEnv(extraEnv []string, args ...string) (string, error) {
-	return runVCSEnv(g.workDir, extraEnv, "git", args...)
+	return runVCSEnv(g.workDir, append(GitEnv(), extraEnv...), "git", args...)
 }
 
 // runVCS executes a VCS command in the given directory and returns its output.
 func runVCS(workDir, binary string, args ...string) (string, error) {
 	return runVCSEnv(workDir, nil, binary, args...)
 }
 
-// runVCSEnv executes a VCS command in the given directory with extra environment
-// entries appended to os.Environ() and returns its output.
-func runVCSEnv(workDir string, extraEnv []string, binary string, args ...string) (string, error) {
+// runVCSEnv executes a VCS command in the given directory and returns its output.
+// A nil env inherits the process environment; otherwise env replaces it wholesale.
+func runVCSEnv(workDir string, env []string, binary string, args ...string) (string, error) {
 	cmd := exec.CommandContext(context.Background(), binary, args...) //nolint:gosec // args constructed internally, not user input
 	cmd.Dir = workDir
-	if len(extraEnv) > 0 {
-		cmd.Env = append(os.Environ(), extraEnv...)
-	}
+	cmd.Env = env
 	out, err := cmd.Output()
 	if err != nil {
 		var exitErr *exec.ExitError

app/diff/diff_test.go

@@ -5,6 +5,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 	"time"
@@ -1820,3 +1821,84 @@ func TestCountChanges(t *testing.T) {
 		})
 	}
 }
+
+func TestGit_FileDiffLiteralPathspec(t *testing.T) {
+	tests := []struct {
+		name      string
+		magic     string // filename git would otherwise read as a pathspec expression
+		decoy     string // file that expression selects instead
+		posixOnly bool
+	}{
+		{name: "top magic prefix", magic: ":(top)f.txt", decoy: "f.txt", posixOnly: true},
+		{name: "glob character", magic: "*.txt", decoy: "f.txt", posixOnly: true},
+		{name: "character class", magic: "[x].txt", decoy: "x.txt"},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			if tt.posixOnly && runtime.GOOS == "windows" {
+				t.Skip("filenames containing ':' or '*' are not valid on windows")
+			}
+			dir := setupTestRepo(t)
+			writeFile(t, dir, tt.decoy, "one\n")
+			writeFile(t, dir, tt.magic, "one\n")
+			gitCmd(t, dir, "add", "-A")
+			gitCmd(t, dir, "commit", "-m", "init")
+
+			writeFile(t, dir, tt.decoy, "one\ndecoy\n")
+			writeFile(t, dir, tt.magic, "one\nreal\n")
+
+			lines, err := NewGit(dir).FileDiff(FileDiffRequest{Path: tt.magic})
+			require.NoError(t, err)
+
+			added, removed := changeContents(lines)
+			assert.Equal(t, []string{"real"}, added, "diff must come from %q, not %q", tt.magic, tt.decoy)
+			assert.Empty(t, removed)
+		})
+	}
+}
+
+// git refuses to run with literal mode alongside either of these, so an inherited
+// one has to be dropped rather than passed through
+func TestGit_FileDiffWithConflictingPathspecEnv(t *testing.T) {
+	for _, envVar := range []string{"GIT_GLOB_PATHSPECS", "GIT_ICASE_PATHSPECS"} {
+		t.Run(envVar, func(t *testing.T) {
+			t.Setenv(envVar, "1")
+			dir := setupTestRepo(t)
+			writeFile(t, dir, "f.txt", "one\n")
+			gitCmd(t, dir, "add", "-A")
+			gitCmd(t, dir, "commit", "-m", "init")
+			writeFile(t, dir, "f.txt", "one\ntwo\n")
+
+			lines, err := NewGit(dir).FileDiff(FileDiffRequest{Path: "f.txt"})
+			require.NoError(t, err)
+			added, _ := changeContents(lines)
+			assert.Equal(t, []string{"two"}, added)
+		})
+	}
+}
+
+// the listing feeds FileDiff verbatim in the UI, so walk the whole path a
+// magic-looking filename takes: list the change, then diff the listed entry
+func TestGit_ChangedFileWithMagicNameRoundTrips(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("filenames containing ':' are not valid on windows")
+	}
+	dir := setupTestRepo(t)
+	writeFile(t, dir, "f.txt", "one\n")
+	writeFile(t, dir, ":(top)f.txt", "one\n")
+	gitCmd(t, dir, "add", "-A")
+	gitCmd(t, dir, "commit", "-m", "init")
+	writeFile(t, dir, ":(top)f.txt", "one\nreal\n")
+
+	g := NewGit(dir)
+	entries, err := g.ChangedFiles("", false)
+	require.NoError(t, err)
+	require.Len(t, entries, 1)
+	assert.Equal(t, ":(top)f.txt", entries[0].Path)
+
+	lines, err := g.FileDiff(FileDiffRequest{Path: entries[0].Path})
+	require.NoError(t, err)
+	added, _ := changeContents(lines)
+	assert.Equal(t, []string{"real"}, added)
+}

app/history/history.go

@@ -11,6 +11,7 @@ import (
 	"strings"
 	"time"
 
+	"github.com/umputun/revdiff/app/diff"
 	"github.com/umputun/revdiff/app/fsutil"
 )
 
@@ -134,6 +135,9 @@ func (s *Service) gitDiff(p Params) string {
 
 	cmd := exec.CommandContext(context.Background(), "git", args...)
 	cmd.Dir = p.GitRoot
+	// annotated file names come from the working tree, so a file actually named
+	// ":(top)x" must select itself instead of being parsed as a pathspec expression
+	cmd.Env = diff.GitEnv()
 	out, err := cmd.Output()
 	if err != nil {
 		var exitErr *exec.ExitError

app/history/history_test.go

@@ -4,6 +4,7 @@ import (
 	"os"
 	"os/exec"
 	"path/filepath"
+	"runtime"
 	"strings"
 	"testing"
 
@@ -503,3 +504,37 @@ func readHistoryFiles(t *testing.T, histDir string) []string {
 	require.NoError(t, err)
 	return contents
 }
+
+func TestSave_DiffUsesLiteralPathspec(t *testing.T) {
+	if runtime.GOOS == "windows" {
+		t.Skip("filenames containing ':' are not valid on windows")
+	}
+	gitRoot := t.TempDir()
+	setupGitRepo(t, gitRoot)
+
+	// ":(top)hello.txt" is parsed as "hello.txt relative to the repo root" unless
+	// git runs in literal-pathspec mode, so hello.txt is the decoy here
+	magic := ":(top)hello.txt"
+	err := os.WriteFile(filepath.Join(gitRoot, magic), []byte("magic\n"), 0o600)
+	require.NoError(t, err)
+	runGit(t, gitRoot, "add", "-A")
+	runGit(t, gitRoot, "commit", "-m", "add magic-named file")
+
+	err = os.WriteFile(filepath.Join(gitRoot, magic), []byte("magic changed\n"), 0o600)
+	require.NoError(t, err)
+	err = os.WriteFile(filepath.Join(gitRoot, "hello.txt"), []byte("decoy changed\n"), 0o600)
+	require.NoError(t, err)
+
+	histDir := t.TempDir()
+	New(histDir).Save(Params{
+		Annotations:    "## " + magic + ":1 (+)\nlook here\n",
+		Path:           gitRoot,
+		GitRoot:        gitRoot,
+		AnnotatedFiles: []string{magic},
+	})
+
+	entries := readHistoryFiles(t, histDir)
+	require.Len(t, entries, 1)
+	assert.Contains(t, entries[0], "magic changed")
+	assert.NotContains(t, entries[0], "decoy changed")
+}