-
Notifications
You must be signed in to change notification settings - Fork 0
Define Git change-detection helpers #584
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Changes from all commits
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,105 @@ | ||
| # Architecture decision record (ADR): Use bounded Git CLI queries for change detection | ||
|
|
||
| ## Status | ||
|
|
||
| Proposed. | ||
|
|
||
| ## Date | ||
|
|
||
| 2026-08-22 | ||
|
|
||
| ## Context and problem statement | ||
|
|
||
| Netsuke needs a template helper that returns paths changed between two Git | ||
| commits. The implementation must preserve Git's range semantics and unusual | ||
| path names without exposing arbitrary command arguments or shell parsing to a | ||
| manifest. It must also keep child output bounded and remain testable without | ||
| mutating `PATH`, changing the process working directory, or depending on the | ||
| repository that runs the test suite. | ||
|
|
||
| The repository has command runners for arbitrary shell filters and Ninja build | ||
| processes, but neither owns fixed, read-only Git queries. Adding a native Git | ||
| library would create another interpretation of revision resolution, | ||
| configuration, and merge-base behaviour. | ||
|
|
||
| ## Decision | ||
|
|
||
| Implement `git_changed_files()` through a feature-private `GitRepository` port | ||
| with a bounded Git CLI adapter. The port belongs to | ||
| `src/stdlib/change_detection/` and permits only three fixed operations: | ||
|
|
||
| - resolve one caller endpoint to a commit object ID with `git rev-parse`; | ||
| - find every best merge base for a three-dot comparison with | ||
| `git merge-base --all`; and | ||
| - obtain NUL-delimited changed paths with a fixed `git diff` argument vector. | ||
|
|
||
| The adapter resolves caller input before diffing. Later operations receive only | ||
| validated hexadecimal object IDs, never the original revision text. The diff | ||
| disables external diff drivers, text conversion, and rename detection. It | ||
| inherits no stdin, reads stdout and stderr concurrently, and applies the | ||
| standard-library command capture limit to both streams. | ||
|
Comment on lines
+27
to
+40
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🩺 Stability & Availability | 🟠 Major | ⚡ Quick win 🧩 Analysis chain🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- ADR and design references ---'
sed -n '1,220p' docs/adr-014-use-bounded-git-cli-for-change-detection.md
printf '%s\n' '--- design command sections ---'
sed -n '130,180p' docs/git-change-detection-helpers-design.md
printf '%s\n' '--- change-detection implementation files ---'
fd -t f . | rg 'change_detection|git.*(rs|md)|Cargo.toml$'
printf '%s\n' '--- Git command construction and process waiting ---'
rg -n -C 4 --glob '*.rs' \
'rev-parse|merge-base|git diff|Command::new|no-lazy-fetch|wait_with_output|try_wait|kill|stdin|GIT_|GIT_CONFIG|pager|fetch' .Repository: leynos/netsuke Length of output: 50373 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- tracked change-detection and command files ---'
git ls-files | rg '(^|/)(change_detection|command)(/|.*\.rs$)|git-change-detection|adr-014'
printf '%s\n' '--- design sections mentioning execution, environment, fetch, pager, timeout, and wait ---'
rg -n -i -C 5 \
'environment|env(ironment)?|fetch|lazy|pager|timeout|deadline|wait|process|stdin|stderr|stdout|remote|config' \
docs/git-change-detection-helpers-design.md
printf '%s\n' '--- standard-library command implementation symbols ---'
rg -n -C 6 --glob 'src/stdlib/**/*.rs' \
'Command::new|std::process|wait_with_output|try_wait|child\.wait|child\.kill|Stdio|current_dir|env_clear|env\(|PATH|GIT_|timeout|deadline|capture|read_to_end|spawn'
printf '%s\n' '--- all source references to the proposed helper and GitRepository ---'
rg -n -C 5 --glob '*.{rs,md}' \
'git_changed_files|GitRepository|merge-base|rev-parse|no-ext-diff|no-textconv|no-renames'Repository: leynos/netsuke Length of output: 12507 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- relevant design contract ---'
sed -n '250,370p' docs/git-change-detection-helpers-design.md
printf '%s\n' '--- stdlib command configuration and execution ---'
sed -n '1,260p' src/stdlib/command/config.rs
sed -n '1,320p' src/stdlib/command/execution.rs
sed -n '1,260p' src/stdlib/command/pipes.rs
printf '%s\n' '--- command environment policy ---'
sed -n '1,260p' src/runner/process/command_env.rs
rg -n -C 5 \
'CommandEnv|env_clear|inherit|GIT_|GIT_CONFIG|GIT_DIR|GIT_WORK_TREE|GIT_PAGER|PAGER|PAGER交|PATH|Stdio::null|wait' \
src/stdlib src/runner/process tests docs
printf '%s\n' '--- local Git version and global options ---'
git --version
git --help -a 2>/dev/null | rg -n 'no-lazy-fetch|no-pager' || true
git -c help.format=man help git 2>/dev/null | col -b | rg -n -C 3 'no-lazy-fetch|no-pager|environment' || trueRepository: leynos/netsuke Length of output: 50373 🌐 Web query:
💡 Result: In Git, --no-lazy-fetch is a command-line option, not a global configuration option [1][2][3]. It is used to prevent Git from automatically ("lazily") fetching missing objects from a promisor remote when a command needs them [1][2][4]. Key details regarding --no-lazy-fetch: Usage: It is passed as a top-level command option, placed before the specific Git command being executed (e.g., git --no-lazy-fetch rev-parse ) [5]. Purpose: When using partial clones, Git normally triggers a network request to a promisor remote if a required object is missing [6][4]. The --no-lazy-fetch option disables this behavior for the duration of that command, which is primarily useful for debugging or when you want to verify if an object is already present locally [1][2][5]. Environment Variable Equivalent: This option is equivalent to setting the GIT_NO_LAZY_FETCH environment variable to 1 [1][2]. Impact on Commands: When used with commands like rev-parse or merge-base, if the operation requires an object that has not yet been fetched, the command will fail or behave as if the object does not exist rather than attempting to download it [1][4]. This option is particularly effective when combined with commands like git cat-file -e, allowing you to check for the local existence of an object without triggering an unintended network fetch [1][2]. There is no --no-lazy-fetch option in git config, nor can it be set as a persistent global configuration; it is designed strictly as a runtime override for specific command executions [1][2]. Top results: [1], [2], [5], [4] Citations:
🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- command config ---'
sed -n '1,240p' src/stdlib/command/config.rs
printf '%s\n' '--- command execution ---'
sed -n '1,280p' src/stdlib/command/execution.rs
printf '%s\n' '--- bounded pipes ---'
sed -n '1,300p' src/stdlib/command/pipes.rs
printf '%s\n' '--- process environment implementation ---'
sed -n '1,230p' src/runner/process/command_env.rs
printf '%s\n' '--- focused policy references ---'
rg -n -C 3 --glob 'src/stdlib/command/*.rs' \
'Command::new|env_clear|env\(|PATH|Stdio|spawn|wait|kill|timeout|capture|read'
rg -n -C 3 --glob 'src/runner/process/*.rs' \
'CommandEnv|env_clear|env\(|Stdio|spawn|wait|kill|timeout|capture'
printf '%s\n' '--- Git support/version statements ---'
rg -n -i -C 3 --glob '*.{md,rs,toml,yml,yaml}' \
'minimum.*git|git.*version|required.*git|git [0-9]|partial clone|promisor|remote contact|no remote|lazy fetch'Repository: leynos/netsuke Length of output: 35075 🏁 Script executed: #!/bin/bash
set -eu
printf '%s\n' '--- top-level option is accepted ---'
git --no-lazy-fetch rev-parse --git-dir
printf '%s\n' '--- environment equivalent is accepted ---'
GIT_NO_LAZY_FETCH=1 git rev-parse --git-dir
printf '%s\n' '--- subcommand placement is rejected ---'
set +e
output=$(git rev-parse --no-lazy-fetch --git-dir 2>&1)
status=$?
set -e
printf 'status=%s\n%s\n' "$status" "$output"
test "$status" -ne 0Repository: leynos/netsuke Length of output: 313 Disable Git lazy fetching for every operation. Add Git’s top-level 🤖 Prompt for AI Agents |
||
|
|
||
| The port is not a general Git or command service. Only the change-detection | ||
| module may call it, and no caller may provide flags, subcommands, pathspecs, or | ||
| environment mutations. Production receives the absolute workspace path and | ||
| optional command `PATH` override through `StdlibConfig`; tests supply scripted | ||
| port responses. | ||
|
|
||
| ## Rationale | ||
|
|
||
| - **Git remains the semantic authority.** Revision peeling, object lookup, and | ||
| merge-base computation follow the Git implementation installed for the | ||
| repository workflow. | ||
| - **The command surface is closed.** Parsing a strict range and resolving it to | ||
| object IDs prevents template text from entering an option or pathname | ||
| position. | ||
| - **Path fidelity is explicit.** `git diff --name-only -z` preserves newlines | ||
| and other non-NUL bytes until Netsuke performs its required UTF-8 check. | ||
| - **Resource use stays bounded.** The adapter shares the stdlib capture budget | ||
| and drains both output streams rather than using an unbounded convenience | ||
| call. | ||
| - **Tests remain isolated.** A feature-private port supports parser, error, and | ||
| composition tests without process-global environment or directory changes. | ||
|
|
||
| ## Consequences | ||
|
|
||
| Netsuke requires a discoverable Git executable when a manifest invokes | ||
| `git_changed_files()`. Registration still succeeds when Git or an absolute | ||
| workspace path is unavailable; invocation reports the missing dependency. | ||
|
|
||
| Git queries mark the standard-library render impure immediately before the | ||
| first process starts. Validation failures remain pure. Manifest-query | ||
| environments reject the function because discovery must not inspect repository | ||
| state. | ||
|
|
||
| The adapter must retain fixed argv tests, output-limit tests, and | ||
| low-cardinality telemetry. A future Git-backed feature must either fit the | ||
| three declared operations or propose a new decision; it must not widen the port | ||
| into arbitrary Git execution. | ||
|
|
||
| ## Alternatives considered | ||
|
|
||
| - **Run a shell pipeline from the template.** Rejected because every manifest | ||
| would own shell quoting, option separation, platform syntax, and path record | ||
| parsing. | ||
| - **Use `git2`.** Rejected because it adds a substantial native dependency and | ||
| a second semantic implementation for behaviour Git already owns. | ||
| - **Reuse the arbitrary shell-filter runner as the public boundary.** Rejected | ||
| because that runner admits user-selected commands and output modes, while | ||
| change detection needs a closed read-only protocol. | ||
| - **Reuse the Ninja process boundary.** Rejected because Ninja execution owns | ||
| long-running build lifecycle, status parsing, and reporter integration that | ||
| do not belong to a manifest query. | ||
|
|
||
| ## Implementation references | ||
|
|
||
| - Detailed contract and verification: | ||
| [`docs/git-change-detection-helpers-design.md`](git-change-detection-helpers-design.md) | ||
| - Standard-library configuration and registration: | ||
| [`src/stdlib/config/mod.rs`](../src/stdlib/config/mod.rs) and | ||
| [`src/stdlib/register.rs`](../src/stdlib/register.rs) | ||
| - Existing bounded command primitives: | ||
| [`src/stdlib/command/execution.rs`](../src/stdlib/command/execution.rs) and | ||
| [`src/stdlib/command/pipes.rs`](../src/stdlib/command/pipes.rs) | ||
| - Delivery roadmap: | ||
| [`docs/roadmap.md`](roadmap.md#6-change-aware-manifest-planning) | ||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Because this ADR is marked
Proposed, the documentation style guide requires an Outstanding Decisions section listing the questions that must be resolved before acceptance. Omitting it makes the proposal appear settled despite deferred choices such as timeout and pattern-budget policy, leaving no explicit acceptance checklist.AGENTS.md reference: AGENTS.md:L48-L51
Useful? React with 👍 / 👎.