Compare commits

...
Author SHA1 Message Date
Vasanthdev2004 142282a1d9 test(gitsafe): keep the real-git setup off an inherited GIT_DIR
The setup helper in TestCommandAgainstRealGit ran git with the inherited
environment, so from a hook, or a shell that exports GIT_DIR or
GIT_WORK_TREE, git init and git config would act on the repository those
name instead of the temporary one. It now uses the same environment as
Command, and the test sets both variables to a decoy repository and checks
after each setup step that the decoy was not touched.
2026-10-15 09:51:22 +05:30
Vasanthdev2004 786735655d fix(tui,cli): stop a repository's git config from running programs in Zero's own git calls
Zero runs git status and git diff on its own: the TUI's files sweep at
startup and after each command, its pull request diffstat, and the
scratch-file warning after a run. A repository's own config can name
programs those commands run (core.fsmonitor on every status, hooks when
the index is rewritten, clean filters and diff drivers assigned through
.gitattributes), so a workspace unpacked from an archive ran its own code
on the first turn, although Zero refuses the same workspace's project
hooks and plugins until the user trusts it.

internal/gitsafe builds those commands with core.fsmonitor and
core.hooksPath turned off and GIT_OPTIONAL_LOCKS=0, and drops the
repository-location variables git exports to hooks, using the same
environment for its config check and for the command. A repository whose
own config, or per-worktree config when extensions.worktreeConfig is on,
defines a filter or diff driver command is refused, and the callers
already treat that like a directory that is not a repository. The status
and diff calls also pass --ignore-submodules=dirty, because a submodule's
own config is not checked.
2026-10-14 16:27:51 +05:30
7 changed files with 771 additions and 10 deletions

No files matched your search

+9 -1
View File
@@ -7,6 +7,8 @@ import (
"strconv"
"strings"
"time"
"github.com/Gitlawb/zero/internal/gitsafe"
)
type scratchFileBaseline map[string]bool
@@ -78,7 +80,13 @@ func gitUntrackedFiles(workspaceRoot string) ([]string, bool) {
ctx, cancel := context.WithTimeout(context.Background(), 5*time.Second)
defer cancel()
output, err := exec.CommandContext(ctx, "git", "-C", workspaceRoot, "status", "--porcelain", "-z", "--untracked-files=all").Output()
// gitsafe keeps the workspace's own git config from running a program, and
// --ignore-submodules=dirty keeps git out of submodule work trees.
command, err := gitsafe.Command(ctx, workspaceRoot, "status", "--porcelain", "-z", "--untracked-files=all", "--ignore-submodules=dirty")
if err != nil {
return nil, false
}
output, err := command.Output()
if err != nil {
// Not a git repo (or git failed for some other reason) — nothing
// reliable to report against, so stay silent rather than guess.
+218
View File
@@ -0,0 +1,218 @@
package gitsafe
import (
"go/ast"
"go/parser"
"go/token"
"os"
"path/filepath"
"strconv"
"strings"
"testing"
)
// The packages whose git calls Zero makes on its own. The CLI runs git for the
// scratch-file warning and the TUI for its files sweep and pull request
// diffstat; each call has to come through Command.
var automaticGitPackages = []string{
filepath.Join("..", "cli"),
filepath.Join("..", "tui"),
}
// parsePackage parses the non-test Go files in dir.
func parsePackage(t *testing.T, dir string) (*token.FileSet, []*ast.File) {
t.Helper()
entries, err := os.ReadDir(dir)
if err != nil {
t.Fatalf("read %s: %v", dir, err)
}
fset := token.NewFileSet()
var files []*ast.File
for _, entry := range entries {
name := entry.Name()
if entry.IsDir() || !strings.HasSuffix(name, ".go") || strings.HasSuffix(name, "_test.go") {
continue
}
file, err := parser.ParseFile(fset, filepath.Join(dir, name), nil, 0)
if err != nil {
t.Fatalf("parse %s: %v", name, err)
}
files = append(files, file)
}
if len(files) == 0 {
t.Fatalf("no Go files in %s", dir)
}
return fset, files
}
// importName is the name file uses for the package at path, or "" when the
// file does not import it.
func importName(file *ast.File, path string) string {
for _, spec := range file.Imports {
if value, err := strconv.Unquote(spec.Path.Value); err != nil || value != path {
continue
}
if spec.Name != nil {
return spec.Name.Name
}
return filepath.Base(path)
}
return ""
}
// directGitExecs lists the exec.Command and exec.CommandContext calls in files
// whose program is the literal "git".
func directGitExecs(fset *token.FileSet, files []*ast.File) []string {
var found []string
for _, file := range files {
execName := importName(file, "os/exec")
if execName == "" {
continue
}
ast.Inspect(file, func(node ast.Node) bool {
call, ok := node.(*ast.CallExpr)
if !ok {
return true
}
selector, ok := call.Fun.(*ast.SelectorExpr)
if !ok {
return true
}
if pkg, ok := selector.X.(*ast.Ident); !ok || pkg.Name != execName {
return true
}
program := 0
switch selector.Sel.Name {
case "Command":
case "CommandContext":
program = 1
default:
return true
}
if len(call.Args) <= program {
return true
}
if literal, ok := call.Args[program].(*ast.BasicLit); ok && literal.Kind == token.STRING {
if value, err := strconv.Unquote(literal.Value); err == nil && value == "git" {
found = append(found, fset.Position(call.Pos()).String())
}
}
return true
})
}
return found
}
func TestAutomaticGitCallsDoNotExecGitDirectly(t *testing.T) {
for _, dir := range automaticGitPackages {
fset, files := parsePackage(t, dir)
for _, position := range directGitExecs(fset, files) {
t.Errorf("%s runs git directly; go through gitsafe.Command so the repository's config cannot make it run a program", position)
}
}
}
// gitArguments returns the git arguments of call when it is gitsafe.Command or a
// runner called with the program "git" (the TUI's runner(ctx, cwd, "git", ...)),
// as far as they are string literals.
func gitArguments(call *ast.CallExpr, gitsafeName string) ([]string, bool) {
start := -1
if selector, ok := call.Fun.(*ast.SelectorExpr); ok && gitsafeName != "" && selector.Sel.Name == "Command" {
if pkg, ok := selector.X.(*ast.Ident); ok && pkg.Name == gitsafeName {
start = 2
}
}
if start < 0 {
for index, argument := range call.Args {
if literal, ok := argument.(*ast.BasicLit); ok && literal.Kind == token.STRING && literal.Value == `"git"` {
start = index + 1
break
}
}
}
if start < 0 || start > len(call.Args) {
return nil, false
}
var arguments []string
for _, argument := range call.Args[start:] {
if literal, ok := argument.(*ast.BasicLit); ok && literal.Kind == token.STRING {
if value, err := strconv.Unquote(literal.Value); err == nil {
arguments = append(arguments, value)
continue
}
}
arguments = append(arguments, "")
}
return arguments, true
}
// A submodule has its own config, which Command does not check, so the
// automatic status and diff calls stay out of submodule work trees.
func TestAutomaticStatusAndDiffSkipSubmoduleWorkTrees(t *testing.T) {
checked := 0
for _, dir := range automaticGitPackages {
fset, files := parsePackage(t, dir)
for _, file := range files {
gitsafeName := importName(file, "github.com/Gitlawb/zero/internal/gitsafe")
ast.Inspect(file, func(node ast.Node) bool {
call, ok := node.(*ast.CallExpr)
if !ok {
return true
}
arguments, ok := gitArguments(call, gitsafeName)
if !ok || len(arguments) == 0 || (arguments[0] != "status" && arguments[0] != "diff") {
return true
}
checked++
for _, argument := range arguments {
if argument == "--ignore-submodules=dirty" {
return true
}
}
t.Errorf("%s runs git %s without --ignore-submodules=dirty", fset.Position(call.Pos()), arguments[0])
return true
})
}
}
// The scratch-file status, the files sweep's status and diff, and the pull
// request diffstat.
if checked != 4 {
t.Fatalf("checked %d automatic status or diff calls, want 4; update this test when they change", checked)
}
}
// The TUI runs every git command through one runner, whose program name is a
// parameter, so the check above cannot see it. The runner has to hand git to
// Command itself.
func TestTUICommandRunnerSendsGitThroughCommand(t *testing.T) {
_, files := parsePackage(t, filepath.Join("..", "tui"))
var runner *ast.FuncDecl
var runnerFile *ast.File
for _, file := range files {
for _, declaration := range file.Decls {
if function, ok := declaration.(*ast.FuncDecl); ok && function.Recv == nil && function.Name.Name == "defaultPRCommandRunner" {
runner, runnerFile = function, file
}
}
}
if runner == nil {
t.Fatal("defaultPRCommandRunner not found in internal/tui")
}
gitsafeName := importName(runnerFile, "github.com/Gitlawb/zero/internal/gitsafe")
calls := false
if gitsafeName != "" {
ast.Inspect(runner.Body, func(node ast.Node) bool {
if call, ok := node.(*ast.CallExpr); ok {
if selector, ok := call.Fun.(*ast.SelectorExpr); ok && selector.Sel.Name == "Command" {
if pkg, ok := selector.X.(*ast.Ident); ok && pkg.Name == gitsafeName {
calls = true
}
}
}
return !calls
})
}
if !calls {
t.Fatal("defaultPRCommandRunner does not call gitsafe.Command, so the TUI's git calls skip it")
}
}
+192
View File
@@ -0,0 +1,192 @@
// Package gitsafe builds the read-only git commands Zero runs on its own, as
// opposed to a git command the user or the model asked for: the status and diff
// calls behind the TUI's changed-files sweep, its pull request diffstat and the
// scratch-file warning, which run without anyone asking for git.
//
// A REPOSITORY CAN MAKE THOSE COMMANDS RUN A PROGRAM. git status and git diff read
// the repository's own .git/config, and several keys there name programs git then
// runs: core.fsmonitor on every status, a clean filter or diff driver assigned
// through .gitattributes whenever a file's content has to be read, and hooks such
// as post-index-change when the index is rewritten. A repository unpacked from an
// archive or handed over as a directory carries that config, and Zero would run it
// on the first turn, although it refuses the same workspace's project hooks and
// plugins until the user trusts it.
//
// The single-key cases are switched off on the command line. Filter and diff
// driver commands are named per driver, so they cannot be switched off without
// knowing the names; a repository whose own config defines any is skipped instead.
// Commands defined in the user's own global config, such as git-lfs, are the
// user's choice and are not affected.
package gitsafe
import (
"bytes"
"context"
"errors"
"os"
"os/exec"
"runtime"
"strings"
)
// ErrRepositoryDefinesCommands is returned for a repository whose own config
// defines a filter or diff driver command. The caller goes without the git
// information rather than run it.
var ErrRepositoryDefinesCommands = errors.New("the repository's own git config defines a filter or diff command, so Zero does not run git there on its own")
// commandKeys matches the config keys whose value is a program git can run while
// computing status or a diff, and which are named per driver. Git prints section
// and variable names in lower case, so the patterns do not need to fold case.
const commandKeys = `filter\..+\.(clean|smudge|process)|diff\..+\.(command|textconv)|diff\.external`
// worktreeConfigKey makes git also read per-worktree config (config.worktree),
// which a --local read leaves out. Without it git ignores that file.
const worktreeConfigKey = "extensions.worktreeconfig"
const (
localConfigPattern = `^(` + commandKeys + `|extensions\.worktreeconfig)$`
worktreeConfigPattern = `^(` + commandKeys + `)$`
)
// Command returns git with args, run in dir, with the programs a repository can
// configure turned off. It fails with ErrRepositoryDefinesCommands when the
// repository's own config defines a filter or diff driver command, and with git's
// own error when dir is not inside a repository.
//
// Callers that inspect a work tree that may contain submodules should also pass
// --ignore-submodules=dirty: a submodule has its own config, which this does not
// read, and git only runs commands there when it looks inside the submodule.
func Command(ctx context.Context, dir string, args ...string) (*exec.Cmd, error) {
env := environment()
defines, err := repositoryDefinesCommands(ctx, dir, env)
if err != nil {
return nil, err
}
if defines {
return nil, ErrRepositoryDefinesCommands
}
command := exec.CommandContext(ctx, "git", append(disabledPrograms(), args...)...)
command.Dir = dir
// No opportunistic index refresh: a read-only call never rewrites the index,
// so it never triggers the hooks that run when the index is written. Git
// before 2.15 does not know the variable and ignores it.
command.Env = append(env, "GIT_OPTIONAL_LOCKS=0")
return command, nil
}
// disabledPrograms turns off the single-key ways a repository runs a program
// during status or diff. Git ignores a key it does not know, so an older git
// without hooksPath (2.9) or fsmonitor (2.16) accepts these unchanged.
func disabledPrograms() []string {
return []string{"-c", "core.fsmonitor=false", "-c", "core.hooksPath=" + os.DevNull}
}
// repositoryLocationVariables make git use a given repository, work tree or
// index instead of discovering them from the working directory. Git exports
// them to hooks, so a Zero started from a hook inherits the hook's repository.
var repositoryLocationVariables = []string{
"GIT_DIR", "GIT_WORK_TREE", "GIT_INDEX_FILE", "GIT_COMMON_DIR",
"GIT_OBJECT_DIRECTORY", "GIT_ALTERNATE_OBJECT_DIRECTORIES", "GIT_NAMESPACE", "GIT_PREFIX",
}
// environment is Zero's environment without the repository-location variables,
// so git finds the repository from dir. The config check and the command share
// it: checking one repository's config and then running git in another would
// let the second one's commands through.
func environment() []string {
parent := os.Environ()
env := make([]string, 0, len(parent)+1)
for _, entry := range parent {
name, _, _ := strings.Cut(entry, "=")
if !isRepositoryLocationVariable(name) {
env = append(env, entry)
}
}
return env
}
func isRepositoryLocationVariable(name string) bool {
for _, variable := range repositoryLocationVariables {
// Windows environment names are case-insensitive.
if name == variable || (runtime.GOOS == "windows" && strings.EqualFold(name, variable)) {
return true
}
}
return false
}
// repositoryDefinesCommands reports whether the repository's own config, with
// includes followed, defines a filter or diff driver command. The local config is
// read once, together with the key that turns on per-worktree config, and that
// file is read only when it is on, so most repositories cost one git process.
// Reading config runs nothing.
func repositoryDefinesCommands(ctx context.Context, dir string, env []string) (bool, error) {
entries, err := readConfig(ctx, dir, env, "--local", localConfigPattern)
if err != nil {
return false, err
}
worktreeConfig := false
for _, entry := range entries {
if entry.key != worktreeConfigKey {
return true, nil
}
worktreeConfig = !configFalse(entry.value)
}
if !worktreeConfig {
return false, nil
}
// The extension needs git 2.20, which also has --worktree, so a failure here
// is real and the caller goes without git.
entries, err = readConfig(ctx, dir, env, "--worktree", worktreeConfigPattern)
if err != nil {
return false, err
}
return len(entries) > 0, nil
}
// configFalse reports whether a config value is one of git's spellings of false.
// Anything else, including a key with no value, counts as on, so an unexpected
// spelling costs one more read rather than skipping the file.
func configFalse(value string) bool {
switch strings.ToLower(strings.TrimSpace(value)) {
case "false", "no", "off", "0":
return true
}
return false
}
type configEntry struct {
key string
value string
}
// readConfig returns the entries in one config scope whose key matches pattern.
// It is a variable so tests can answer for git.
var readConfig = func(ctx context.Context, dir string, env []string, scope string, pattern string) ([]configEntry, error) {
command := exec.CommandContext(ctx, "git", "config", "-z", scope, "--includes", "--get-regexp", pattern)
command.Dir = dir
command.Env = env
output, err := command.Output()
if err != nil {
var exit *exec.ExitError
if errors.As(err, &exit) && exit.ExitCode() == 1 {
return nil, nil // no key matched
}
return nil, err
}
return parseConfigEntries(output), nil
}
// parseConfigEntries splits git config -z output: each entry ends with a NUL,
// and a newline separates the key from its value when there is one.
func parseConfigEntries(output []byte) []configEntry {
var entries []configEntry
for _, record := range bytes.Split(output, []byte{0}) {
if len(record) == 0 {
continue
}
key, value, _ := bytes.Cut(record, []byte{'\n'})
entries = append(entries, configEntry{key: string(key), value: string(value)})
}
return entries
}
+327
View File
@@ -0,0 +1,327 @@
package gitsafe
import (
"context"
"errors"
"fmt"
"os"
"os/exec"
"path/filepath"
"reflect"
"regexp"
"runtime"
"strings"
"testing"
)
type configCall struct {
scope string
env []string
}
// answerConfig stands in for git's config read and records each call.
func answerConfig(t *testing.T, answer func(scope string) ([]configEntry, error)) *[]configCall {
t.Helper()
var calls []configCall
previous := readConfig
readConfig = func(_ context.Context, _ string, env []string, scope string, _ string) ([]configEntry, error) {
calls = append(calls, configCall{scope: scope, env: env})
return answer(scope)
}
t.Cleanup(func() { readConfig = previous })
return &calls
}
func noEntries(string) ([]configEntry, error) { return nil, nil }
func scopes(calls *[]configCall) []string {
var read []string
for _, call := range *calls {
read = append(read, call.scope)
}
return read
}
// effectiveEnv returns the value a child process gets for name: os/exec keeps
// the last entry for a name, compared without case on Windows.
func effectiveEnv(env []string, name string) (string, bool) {
value, found := "", false
for _, entry := range env {
key, rest, ok := strings.Cut(entry, "=")
if ok && (key == name || (runtime.GOOS == "windows" && strings.EqualFold(key, name))) {
value, found = rest, true
}
}
return value, found
}
func TestCommandTurnsOffRepositoryPrograms(t *testing.T) {
answerConfig(t, noEntries)
t.Setenv("GIT_OPTIONAL_LOCKS", "1")
dir := t.TempDir()
command, err := Command(context.Background(), dir, "status", "--porcelain")
if err != nil {
t.Fatalf("Command: %v", err)
}
want := []string{"git", "-c", "core.fsmonitor=false", "-c", "core.hooksPath=" + os.DevNull, "status", "--porcelain"}
if !reflect.DeepEqual(command.Args, want) {
t.Fatalf("args = %q, want %q", command.Args, want)
}
if command.Dir != dir {
t.Fatalf("dir = %q, want %q", command.Dir, dir)
}
// An inherited GIT_OPTIONAL_LOCKS must not turn the index refresh back on.
if value, _ := effectiveEnv(command.Env, "GIT_OPTIONAL_LOCKS"); value != "0" {
t.Fatalf("effective GIT_OPTIONAL_LOCKS = %q, want 0", value)
}
}
func TestCommandDropsRepositoryLocationVariables(t *testing.T) {
// Turn per-worktree config on so both reads happen and both are checked.
calls := answerConfig(t, func(scope string) ([]configEntry, error) {
if scope == "--local" {
return []configEntry{{key: worktreeConfigKey, value: "true"}}, nil
}
return nil, nil
})
elsewhere := filepath.Join(t.TempDir(), "elsewhere")
for _, name := range repositoryLocationVariables {
t.Setenv(name, elsewhere)
}
t.Setenv("ZERO_GITSAFE_TEST_KEPT", "kept")
command, err := Command(context.Background(), t.TempDir(), "status")
if err != nil {
t.Fatalf("Command: %v", err)
}
for _, name := range repositoryLocationVariables {
if value, found := effectiveEnv(command.Env, name); found {
t.Fatalf("%s=%q reached git", name, value)
}
}
if value, _ := effectiveEnv(command.Env, "ZERO_GITSAFE_TEST_KEPT"); value != "kept" {
t.Fatalf("an unrelated variable was dropped: %q", value)
}
// The config check has to look at the repository the command runs in, so it
// gets the same environment, minus only the lock variable added afterwards.
if got := scopes(calls); !reflect.DeepEqual(got, []string{"--local", "--worktree"}) {
t.Fatalf("config reads = %q, want --local then --worktree", got)
}
for _, call := range *calls {
if !reflect.DeepEqual(call.env, command.Env[:len(command.Env)-1]) {
t.Fatalf("the %s config read ran with a different environment than the command", call.scope)
}
}
}
func TestLocationVariableNamesFoldCaseOnlyOnWindows(t *testing.T) {
if !isRepositoryLocationVariable("GIT_DIR") {
t.Fatal("GIT_DIR not recognised")
}
if got, want := isRepositoryLocationVariable("Git_Dir"), runtime.GOOS == "windows"; got != want {
t.Fatalf("Git_Dir recognised = %v, want %v on %s", got, want, runtime.GOOS)
}
for _, name := range []string{"GIT_DIRECTORY", "MY_GIT_DIR", "GIT_CONFIG_GLOBAL", ""} {
if isRepositoryLocationVariable(name) {
t.Fatalf("%q treated as a repository-location variable", name)
}
}
}
func TestCommandRefusesRepositoryThatDefinesCommands(t *testing.T) {
enabled := configEntry{key: worktreeConfigKey, value: "true"}
command := configEntry{key: "filter.x.clean", value: "x"}
cases := map[string]map[string][]configEntry{
"local": {"--local": {command}},
"local after the extension": {"--local": {enabled, command}},
"per-worktree": {"--local": {enabled}, "--worktree": {{key: "diff.x.textconv", value: "x"}}},
}
for name, answers := range cases {
t.Run(name, func(t *testing.T) {
answerConfig(t, func(scope string) ([]configEntry, error) { return answers[scope], nil })
got, err := Command(context.Background(), t.TempDir(), "status")
if !errors.Is(err, ErrRepositoryDefinesCommands) {
t.Fatalf("err = %v, want ErrRepositoryDefinesCommands", err)
}
if got != nil {
t.Fatal("a command came back for a repository that defines commands")
}
})
}
}
func TestCommandReadsPerWorktreeConfigOnlyWhenItIsOn(t *testing.T) {
cases := map[string]struct {
local []configEntry
want []string
}{
"no extension": {nil, []string{"--local"}},
"extension off": {[]configEntry{{key: worktreeConfigKey, value: "false"}}, []string{"--local"}},
"extension off as no": {[]configEntry{{key: worktreeConfigKey, value: "No"}}, []string{"--local"}},
"extension on": {[]configEntry{{key: worktreeConfigKey, value: "true"}}, []string{"--local", "--worktree"}},
"extension with no value": {[]configEntry{{key: worktreeConfigKey}}, []string{"--local", "--worktree"}},
"turned off again": {[]configEntry{{key: worktreeConfigKey, value: "true"}, {key: worktreeConfigKey, value: "off"}}, []string{"--local"}},
}
for name, tc := range cases {
t.Run(name, func(t *testing.T) {
calls := answerConfig(t, func(scope string) ([]configEntry, error) {
if scope == "--local" {
return tc.local, nil
}
return nil, nil
})
if _, err := Command(context.Background(), t.TempDir(), "status"); err != nil {
t.Fatalf("Command: %v", err)
}
if got := scopes(calls); !reflect.DeepEqual(got, tc.want) {
t.Fatalf("config reads = %q, want %q", got, tc.want)
}
})
}
}
func TestCommandFailsWhenConfigCannotBeRead(t *testing.T) {
unreadable := errors.New("not a git repository")
for _, failing := range []string{"--local", "--worktree"} {
t.Run(failing, func(t *testing.T) {
answerConfig(t, func(scope string) ([]configEntry, error) {
if scope == failing {
return nil, unreadable
}
return []configEntry{{key: worktreeConfigKey, value: "true"}}, nil
})
if _, err := Command(context.Background(), t.TempDir(), "status"); !errors.Is(err, unreadable) {
t.Fatalf("err = %v, want the config read error", err)
}
})
}
}
// Go's regexp stands in for git's extended regex here; the patterns use nothing
// the two treat differently.
func TestConfigPatterns(t *testing.T) {
local := regexp.MustCompile(localConfigPattern)
worktree := regexp.MustCompile(worktreeConfigPattern)
for _, key := range []string{
"filter.lfs.clean", "filter.lfs.smudge", "filter.lfs.process", "filter.a.b.clean",
"diff.word.textconv", "diff.tool.command", "diff.external",
} {
if !local.MatchString(key) || !worktree.MatchString(key) {
t.Errorf("%s is not matched in both scopes", key)
}
}
if !local.MatchString(worktreeConfigKey) || worktree.MatchString(worktreeConfigKey) {
t.Errorf("%s belongs to the local read only", worktreeConfigKey)
}
for _, key := range []string{
"filter.lfs.required", "diff.word.cachetextconv", "diff.word.binary",
"core.fsmonitor", "diff.external.x", "diff.renames", "extensions.worktreeconfigx",
} {
if local.MatchString(key) || worktree.MatchString(key) {
t.Errorf("%s is matched", key)
}
}
}
func TestParseConfigEntries(t *testing.T) {
got := parseConfigEntries([]byte("filter.x.clean\nrun this\nand that\x00extensions.worktreeconfig\x00"))
want := []configEntry{
{key: "filter.x.clean", value: "run this\nand that"},
{key: "extensions.worktreeconfig"},
}
if !reflect.DeepEqual(got, want) {
t.Fatalf("entries = %+v, want %+v", got, want)
}
}
// Real git: a fresh repository with nothing configured, the same repository with
// per-worktree config turned on, and a directory that is not a repository. This
// pins the output format and exit codes readConfig relies on; the command keys
// themselves are covered above.
func TestCommandAgainstRealGit(t *testing.T) {
if _, err := exec.LookPath("git"); err != nil {
t.Skip("git not available")
}
home := t.TempDir()
t.Setenv("HOME", home)
t.Setenv("USERPROFILE", home)
t.Setenv("XDG_CONFIG_HOME", filepath.Join(home, ".config"))
t.Setenv("GIT_CONFIG_GLOBAL", os.DevNull)
t.Setenv("GIT_CONFIG_NOSYSTEM", "1")
repo := t.TempDir()
outside := t.TempDir()
// Both share a parent; stop git from finding an enclosing repository there.
t.Setenv("GIT_CEILING_DIRECTORIES", filepath.Dir(repo))
// Run as if from a hook in another repository. The setup below has to reach
// repo, as Command does, and never the repository these name.
decoy := t.TempDir()
t.Setenv("GIT_DIR", filepath.Join(decoy, ".git"))
t.Setenv("GIT_WORK_TREE", decoy)
decoyUntouched := func(step string) {
t.Helper()
if _, err := os.Stat(filepath.Join(decoy, ".git")); !errors.Is(err, os.ErrNotExist) {
t.Fatalf("%s reached the repository GIT_DIR names instead of repo: %v", step, err)
}
}
git := func(args ...string) {
t.Helper()
command := exec.Command("git", append([]string{"-C", repo}, args...)...)
command.Env = environment()
if output, err := command.CombinedOutput(); err != nil {
t.Fatalf("git %v: %v: %s", args, err, output)
}
}
git("init", "-q")
decoyUntouched("git init")
gitRead := readConfig
var read []string
readConfig = func(ctx context.Context, dir string, env []string, scope string, pattern string) ([]configEntry, error) {
read = append(read, scope)
return gitRead(ctx, dir, env, scope, pattern)
}
t.Cleanup(func() { readConfig = gitRead })
command, err := Command(context.Background(), repo, "status", "--porcelain")
if err != nil {
t.Fatalf("Command in a plain repository: %v", err)
}
if output, err := command.CombinedOutput(); err != nil {
t.Fatalf("git status: %v: %s", err, output)
}
if !reflect.DeepEqual(read, []string{"--local"}) {
t.Fatalf("config reads in a plain repository = %q, want --local only", read)
}
if !gitAtLeast(t, 2, 20) {
t.Log("skipping the per-worktree case: it needs git 2.20 for extensions.worktreeConfig")
} else {
git("config", "extensions.worktreeConfig", "true")
decoyUntouched("git config")
read = nil
if _, err := Command(context.Background(), repo, "status"); err != nil {
t.Fatalf("Command with per-worktree config on: %v", err)
}
if !reflect.DeepEqual(read, []string{"--local", "--worktree"}) {
t.Fatalf("config reads with per-worktree config on = %q, want --local then --worktree", read)
}
}
if _, err := Command(context.Background(), outside, "status"); err == nil {
t.Fatal("Command outside a repository succeeded")
}
}
// gitAtLeast reports whether the git on PATH is at least major.minor.
func gitAtLeast(t *testing.T, major, minor int) bool {
t.Helper()
output, err := exec.Command("git", "version").Output()
if err != nil {
t.Fatalf("git version: %v", err)
}
var gotMajor, gotMinor int
if _, err := fmt.Sscanf(strings.TrimPrefix(strings.TrimSpace(string(output)), "git version "), "%d.%d", &gotMajor, &gotMinor); err != nil {
t.Fatalf("parse %q: %v", output, err)
}
return gotMajor > major || (gotMajor == major && gotMinor >= minor)
}
+4 -2
View File
@@ -54,7 +54,9 @@ func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd {
defer cancel()
// --untracked-files=all enumerates files inside new directories; plain
// porcelain collapses them to "dir/", which is useless as a FILES row.
status, err := defaultPRCommandRunner(ctx, cwd, "git", "status", "--porcelain", "--untracked-files=all")
// --ignore-submodules=dirty keeps git out of submodule work trees, whose
// own config gitsafe does not check.
status, err := defaultPRCommandRunner(ctx, cwd, "git", "status", "--porcelain", "--untracked-files=all", "--ignore-submodules=dirty")
if err != nil {
return gitSweepMsg{baseline: baseline, ok: false}
}
@@ -66,7 +68,7 @@ func gitSweepCmd(parent context.Context, cwd string, baseline bool) tea.Cmd {
// would never match a porcelain path). Best-effort: a failure (e.g. unborn
// HEAD in a fresh repo) keeps the file list with zero counts rather than
// dropping the sweep.
if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat", "-z"); err == nil {
if numstat, err := defaultPRCommandRunner(ctx, cwd, "git", "diff", "HEAD", "--numstat", "-z", "--ignore-submodules=dirty"); err == nil {
stats := parseGitNumstat(numstat)
for i := range files {
if counts, ok := stats[files[i].path]; ok {
+18 -4
View File
@@ -13,6 +13,8 @@ import (
"time"
"charm.land/lipgloss/v2"
"github.com/Gitlawb/zero/internal/gitsafe"
)
type PrStatus string
@@ -243,7 +245,7 @@ func getLocalDiffStats(ctx context.Context, cwd string, baseBranch string, run p
if err != nil {
mergeBase = originBase
}
output, err := run(ctx, cwd, "git", "diff", "--numstat", strings.TrimSpace(mergeBase))
output, err := run(ctx, cwd, "git", "diff", "--numstat", "--ignore-submodules=dirty", strings.TrimSpace(mergeBase))
if err != nil {
return 0, 0, err
}
@@ -290,9 +292,21 @@ func jsonScalarString(raw json.RawMessage) string {
}
func defaultPRCommandRunner(ctx context.Context, cwd string, name string, args ...string) (string, error) {
command := exec.CommandContext(ctx, name, args...)
if strings.TrimSpace(cwd) != "" {
command.Dir = cwd
var command *exec.Cmd
if name == "git" {
// Every git call the TUI makes on its own comes through here (the files
// sweep and the pull request diffstat), so the repository being shown
// cannot make them run a program.
safe, err := gitsafe.Command(ctx, cwd, args...)
if err != nil {
return "", fmt.Errorf("%s: %w", name, err)
}
command = safe
} else {
command = exec.CommandContext(ctx, name, args...)
if strings.TrimSpace(cwd) != "" {
command.Dir = cwd
}
}
var stdout bytes.Buffer
var stderr bytes.Buffer
+3 -3
View File
@@ -15,7 +15,7 @@ func TestPrServiceDetectsGitHubPRAndDiffStats(t *testing.T) {
"git merge-base origin/main HEAD": {
output: "abc123\n",
},
"git diff --numstat abc123": {
"git diff --numstat --ignore-submodules=dirty abc123": {
output: "42\t7\tinternal/tui/view.go\n",
},
}}
@@ -49,7 +49,7 @@ func TestPrServiceFallsBackToGitLabMR(t *testing.T) {
"git merge-base origin/develop HEAD": {
output: "def456\n",
},
"git diff --numstat def456": {
"git diff --numstat --ignore-submodules=dirty def456": {
output: "4\t5\ta.go\n5\t6\tb.go\n",
},
}}
@@ -71,7 +71,7 @@ func TestGetLocalDiffStatsFallsBackToOriginBase(t *testing.T) {
"git merge-base origin/main HEAD": {
err: errors.New("unknown revision"),
},
"git diff --numstat origin/main": {
"git diff --numstat --ignore-submodules=dirty origin/main": {
output: "3\t0\tREADME.md\n",
},
}}