generate-patch-files.sh: check inputs; fail on errors

[mernst]

(There is no need to review file plan-upgrade-valgrind.md in this PR.)

generate-patch-files.sh creates the patch files for a Valgrind merge. It ran even when its inputs were wrong, and wrong output is easy to miss in a multi-megabyte patch:

• With $INV unset, cd $INV went to $HOME, and the script wrote patch files there from whatever it found.
• Any build product in valgrind-old or valgrind-new (*.o, Makefile.in, configure, config.h, test binaries, ...) ended up in coregrind.patch, and make git-update would then git add it to Fjalar. That's easy to hit by building the upstream regtest baseline in valgrind-new.

This PR is based on master and doesn't depend on any other open PR. #103 depends on it and contains its commits. #101 documents building the regtest baseline in a copy of valgrind-new (including removing an old copy first); this PR documents only the script's checks and resetting valgrind-old.

Changes

generate-patch-files.sh:

• Fail if $INV is unset, or if $INV/valgrind-old, $INV/valgrind-new, or $INV/fjalar/valgrind doesn't exist.
• Fail if valgrind-old or valgrind-new is not the top level of a git clone, or has any change or file not in its git repository (git status --porcelain --ignored is non-empty). The error lists the first 20 offending files and the number of others, and gives the git reset --hard && git clean -fdx command that makes the tree pristine.
• check_version: if $VOLDVER or $VNEWVER is set, fail unless valgrind-old or valgrind-new, respectively, is at that tag. This catches forgetting to move valgrind-new to valgrind-old, or cloning the wrong tag.
• Remove all four output files before the checks, so that a failed check or a failure later does not leave stale ones from an earlier run.
• unset CDPATH, so that cd with a relative directory resolves it against the current directory.
• set -e and pipefail, with a run_diff helper that treats diff's exit status 1 (files differ, the normal case) as success and 2 (trouble) as failure, and that creates its output file only if diff succeeds.
• The Fjalar diffs (coregrind-Fjalar.diff, memcheck-Fjalar.diff) compare a temporary copy of the files in fjalar/valgrind that git does not ignore (git ls-files --cached --others --exclude-standard | tar), instead of fjalar/valgrind itself with -xinst -xMakefile.in -xfjalar/html. fjalar/valgrind is usually built in place, and the exclusions missed most build products. The copy omits tracked files that have been deleted from the work tree; any other unreadable file stops the script, rather than producing an incomplete diff.
• Add a #!/bin/bash shebang.

Documentation:

• doc/valgrind-merge.texinfo: when the previous merge's valgrind-new becomes valgrind-old, reset it with git reset --hard and git clean -fdx, because the script requires a pristine clone.
• plan-upgrade-valgrind.md: steps 3 and 4 describe the script's checks instead of a manual check for build products; step 10 resets valgrind-old.

Testing

In a scratch $INV with VALGRIND_3_23_0 as valgrind-old, VALGRIND_3_24_0 as valgrind-new, and fjalar linking to this repository:

• INV unset, or set to a directory without the trees: error, exit 1.
• Pristine trees: exit 0, producing the same four files as before.
• Untracked config.h/*.o in either tree, or a modified file: error listing the files, exit 1.
• run_diff on its own: exit status 0 and 1 continue; a missing file (status 2) stops the script.
• In a small synthetic $INV: a tracked file deleted from fjalar/valgrind is omitted from the Fjalar diffs, with exit 0; an unreadable file in fjalar/valgrind stops the script (exit 2) and leaves no Fjalar diffs.
• shellcheck is clean.

[coderabbitai]

No actionable comments were generated in the recent review. 🎉

ℹ️ Recent review info

⚙️ Run configuration

• Configuration used: Repository: codespecs/fjalar/.coderabbit.yaml
• Review profile: ASSERTIVE
• Plan: Advanced
• Run ID: 4a49213d-1128-4c6b-bf52-cacc51e405d5

📥 Commits

Reviewing files that changed from the base of the PR and between 4263218 and 4676f0a.

📒 Files selected for processing: 3

• doc/valgrind-merge.texinfo
• generate-patch-files.sh
• plan-upgrade-valgrind.md

Included review availability: This review used your included allowance. Your plan provides up to 1 included review per hour; 0 remain after this review.

📝 Walkthrough

The script now validates required inputs, checks that both upstream trees are pristine clones at the requested revisions, and generates diffs through temporary files. It builds a temporary Fjalar tree from tracked and non-ignored files. The documentation describes the clean-tree requirements and adds reset and clean commands for preparing or reusing valgrind-old.

Suggested reviewers: markro49

Priority: ⬇️ Low

Change: Bug fix

Merge Risk: ⚪ Minimal · up to 4676f

No merge-blocking issue is established; the patch-generation safeguards are ready for normal checks.

Security architecture risk: ⚪ Minimal · up to 4676f

The changes strengthen input validation and prevent failed comparisons from publishing partial individual patch files. No material security risk was found introduced or worsened by this PR. Incomplete sets and concurrent generation remain workflow limitations that also existed before this change.

Retained concerns
No architecture-level concerns identified.

Security review details

Security Blast Radius

• inferred — The inspected exposure is the invoking developer's merge workspace and artifacts that may later be applied or committed to Fjalar. The changed script does not introduce a remote invocation path, credential acquisition, or privilege elevation.

Trust Boundaries and Controls

• observed — The new controls check local repository readiness and optional tag-to-HEAD equality; they do not authenticate upstream provenance. Filenames in the new copy pipeline are handled as NUL-delimited data rather than evaluated as shell commands.

Pre-merge checks: 3

Check name	Status	Explanation
Docstring Coverage		Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (2 skipped: 2 …
Linked Issues check		Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check		Check skipped because no linked issues were found for this pull request.

Full details: Docstring Coverage

Explanation

Docstring coverage is 100.00% which is sufficient. The required threshold is 80.00%. Docstring coverage is scoped to functions touched by this diff. Analyzed 2 functions across 1 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches

🧪 Generate unit tests (beta)

• Create a new PR

• Autofix · Keep fixing CodeRabbit findings and required CI, and resolving merge conflicts

_{Comment @coderabbitai help to get the list of available commands.}

[mernst]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mernst]

@coderabbitai review

[coderabbitai]

⚠️ Action not completed

Review rate limited.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.

[mernst]

@coderabbitai review

[coderabbitai]

✅ Action performed

Review finished.

Note: CodeRabbit is an incremental review system and does not re-review already reviewed commits. This command is applicable only when automatic reviews are paused.