feat(git): require git 2.38, run git in the C locale, and pin the porcelain format #529

Merged

hexajon merged 1 commit from issue-524-git-pin-the-git-output-contract-and-declare-a-mini into main 2026-09-16 18:59:24 +00:00

Conversation 1 Commits 1 Files changed 34 +1146 -89

hexajon commented 2026-09-16 18:59:22 +00:00

Owner

Copy link

Summary

madtea runs the git binary, but it declared no minimum version and no output contract. This adds both, and records them in ADR 0038.

What changes

• New leaf package internal/gitexec builds every git child process. It puts LC_ALL=C last in the environment, so git prints untranslated messages. madtea searches git stderr for English text such as CONFLICT, and a translated message hid the conflict before this change.
• EnsureMinVersion checks the git version once per process. Below 2.38.0 a command fails with a message that names the found version and the required version. It is wired into the plain runners, the unauthenticated clone, and both transports (HTTPS and SSH).
• Every direct exec.Command("git", ...) call now goes through the new package: internal/git, config, service/workflow, service/git, orchestrate, adr, analytics, and cmd/setup/web.go.
• git status uses --porcelain=v1, the form git documents as stable. git worktree list keeps the bare --porcelain, because git offers no versioned form there and documents that format as stable.
• pr conflicts now reports a merge-tree failure instead of an empty conflict list. That command needs git 2.38.
• A static guard test (go/ast) fails on a new direct git call and on a new bare --porcelain.
• A test proves a UTF-8 commit subject and a UTF-8 file path read back unchanged under LC_ALL=C.
• The README states git 2.38.0 or newer as a runtime requirement.

Why 2.38

madtea uses git merge-tree --write-tree (git 2.38) and git rev-parse --end-of-options (git 2.30). One floor for every command keeps one check and one CI range. Debian 12 and later, Ubuntu 24.04 and later, and RHEL 8.10 and later all meet it. Ubuntu 20.04, Ubuntu 22.04, and Debian 11 do not.

Follow-ups

• Issue 525: run the git tests against a version matrix, starting at 2.38.0.
• Issue 527: replace the 17 Run* variants with one call that takes options.

Closes #524

## Summary madtea runs the `git` binary, but it declared no minimum version and no output contract. This adds both, and records them in ADR 0038. ## What changes - New leaf package `internal/gitexec` builds every git child process. It puts `LC_ALL=C` last in the environment, so git prints untranslated messages. madtea searches git stderr for English text such as `CONFLICT`, and a translated message hid the conflict before this change. - `EnsureMinVersion` checks the git version once per process. Below 2.38.0 a command fails with a message that names the found version and the required version. It is wired into the plain runners, the unauthenticated clone, and both transports (HTTPS and SSH). - Every direct `exec.Command("git", ...)` call now goes through the new package: `internal/git`, `config`, `service/workflow`, `service/git`, `orchestrate`, `adr`, `analytics`, and `cmd/setup/web.go`. - `git status` uses `--porcelain=v1`, the form git documents as stable. `git worktree list` keeps the bare `--porcelain`, because git offers no versioned form there and documents that format as stable. - `pr conflicts` now reports a `merge-tree` failure instead of an empty conflict list. That command needs git 2.38. - A static guard test (go/ast) fails on a new direct git call and on a new bare `--porcelain`. - A test proves a UTF-8 commit subject and a UTF-8 file path read back unchanged under `LC_ALL=C`. - The README states git 2.38.0 or newer as a runtime requirement. ## Why 2.38 madtea uses `git merge-tree --write-tree` (git 2.38) and `git rev-parse --end-of-options` (git 2.30). One floor for every command keeps one check and one CI range. Debian 12 and later, Ubuntu 24.04 and later, and RHEL 8.10 and later all meet it. Ubuntu 20.04, Ubuntu 22.04, and Debian 11 do not. ## Follow-ups - Issue 525: run the git tests against a version matrix, starting at 2.38.0. - Issue 527: replace the 17 `Run*` variants with one call that takes options. Closes #524

hexajon added 1 commit 2026-09-16 18:59:22 +00:00

feat(git): require git 2.38, run git in the C locale, and pin the porcelain format ... 975e6d1597

Closes #524

hexajon merged commit 975e6d1597 into main 2026-09-16 18:59:24 +00:00

hexajon reviewed 2026-09-16 18:59:33 +00:00

hexajon left a comment

Copy link

Pre-merge review notes.

I read the whole diff against ADR 0038 and ran the full gate. It passes: build, gofmt, vet, tests with the race detector, staticcheck, govulncheck, gosec, modernize, docs verify, cross-compile, and the ADR check.

What I checked:

• Every git process starts through gitexec.NewCmd or NewCmdDir. A sweep finds no other exec of "git" in non-test code. The sites that add environment (askpass, terminal prompt, ssh batch mode) append to the constructor environment, so LC_ALL=C survives. The SSH transport keeps its no-credential intent.
• The static guard uses go/ast. It fails on a direct exec of "git" and on a bare --porcelain, except for git worktree list. The man page confirms that worktree list --porcelain takes no value and is stable, while status takes --porcelain=v1.
• The version check runs once per process, parses the Apple, Windows, and rc forms, keeps the git-not-found error unchanged, and probes git directly so it cannot recurse. Its message names both versions.
• The UTF-8 test commits a non-ASCII subject and path in a real temp repo and reads both back through the log reader and the ls-files reader under LC_ALL=C.

Changes I made after the build:

• setupSSHCmd now calls the version check, so the SSH transport fails the same way as the HTTPS transport on an old git.
• ADR 0038 said "never the bare --porcelain". It now states the git worktree list exception, which matches the guard and the code.
• The architecture package list names internal/gitexec, and the build-from-source prerequisites state the git version.

Still open, tracked elsewhere: the git version matrix in CI (issue 525), and the 17 Run* variants (issue 527).

Pre-merge review notes. I read the whole diff against ADR 0038 and ran the full gate. It passes: build, gofmt, vet, tests with the race detector, staticcheck, govulncheck, gosec, modernize, docs verify, cross-compile, and the ADR check. What I checked: - Every git process starts through `gitexec.NewCmd` or `NewCmdDir`. A sweep finds no other exec of "git" in non-test code. The sites that add environment (askpass, terminal prompt, ssh batch mode) append to the constructor environment, so `LC_ALL=C` survives. The SSH transport keeps its no-credential intent. - The static guard uses go/ast. It fails on a direct exec of "git" and on a bare `--porcelain`, except for `git worktree list`. The man page confirms that `worktree list --porcelain` takes no value and is stable, while `status` takes `--porcelain=v1`. - The version check runs once per process, parses the Apple, Windows, and rc forms, keeps the git-not-found error unchanged, and probes git directly so it cannot recurse. Its message names both versions. - The UTF-8 test commits a non-ASCII subject and path in a real temp repo and reads both back through the log reader and the ls-files reader under `LC_ALL=C`. Changes I made after the build: - `setupSSHCmd` now calls the version check, so the SSH transport fails the same way as the HTTPS transport on an old git. - ADR 0038 said "never the bare `--porcelain`". It now states the `git worktree list` exception, which matches the guard and the code. - The architecture package list names `internal/gitexec`, and the build-from-source prerequisites state the git version. Still open, tracked elsewhere: the git version matrix in CI (issue 525), and the 17 `Run*` variants (issue 527).

Sign in to join this conversation.

Reviewers

No reviewers

Labels

Clear labels   

breaking

Breaking change - flag for release notes

  

bug

Something is not working

  

documentation

Documentation only

  

enhancement

New feature

  

epic

Tracking epic with dependency children

  

good first issue

Interested in contributing? Get started here.

  

help wanted

Maintainer would welcome outside help

  

refactoring

Code restructuring without behavior changes

  

resolution/duplicate

Closed as duplicate of another issue

  

resolution/invalid

Closed as invalid or not reproducible

  

resolution/wontfix

Closed as won't fix

  

security

Security issue or hardening

  

severity/critical

Security severity: critical

  

severity/high

Security severity: high

  

severity/low

Security severity: low

  

severity/medium

Security severity: medium

  

status/abandoned

Someone started, then walked away - up for grabs

  

status/blocked

Blocked by another issue or an external dependency

  

status/needs-decision

Waiting on a maintainer decision; workable once decided

  

status/needs-info

Waiting on reporter / more information before work can start

  

status/needs-verification

Done except verification in a live interactive session

  

testing

Test suite / CI infrastructure work

  

upstream

Related to an upstream repository, already reported there

No labels

breaking

bug

documentation

enhancement

epic

good first issue

help wanted

refactoring

resolution/duplicate

resolution/invalid

resolution/wontfix

security

severity/critical

severity/high

severity/low

severity/medium

status/abandoned

status/blocked

status/needs-decision

status/needs-info

status/needs-verification

testing

upstream

Milestone

Clear milestone

No items

No milestone

Projects

Clear projects

No items

No project

Assignees

Clear assignees

No assignees

1 participant

Notifications

Subscribe

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set.

Reference

sixfold-space/madtea!529

No description provided.