Generate valgrind/fjalar/Makefile.am from Memcheck's by mernst · Pull Request #113 · codespecs/fjalar

mernst · 2026-10-17T18:44:55Z

Almost every line of Fjalar's build file valgrind/fjalar/Makefile.am renames Memcheck to Fjalar, so most upstream changes to valgrind/memcheck/Makefile.am conflict when memcheck.patch patches them in.

With this pull request, generate-fjalar-makefile.sh generates valgrind/fjalar/Makefile.am from valgrind/memcheck/Makefile.am by renaming Memcheck to Fjalar, and Fjalar's changes to the build are in the new valgrind/fjalar/fjalar.am, which the generated file includes. Fjalar builds every Memcheck source file, so a new upstream source file is built automatically. The script lists any line of the generated file that still mentions Memcheck or docs, or that is a conflict marker, and then exits with status 2.

fjalar.am contains the fjalar_core_decls.h consistency check. It lists that check's stamp file in MOSTLYCLEANFILES rather than CLEANFILES, so that make mostlyclean, which removes the object files, also removes the stamp (and make clean still does).

generate-patch-files.sh omits Makefile.am from memcheck.patch. doc/valgrind-merge.texinfo and plan-upgrade-valgrind.md say to run the script after the Memcheck merge. plan-upgrade-readelf.md, make-fjalar-release.sh, the release notes, and the Fjalar manual (fjalar_manual.html, "Creating Fjalar tools") now refer to fjalar.am. The manual says to remove the -O0 from EXTRA_CFLAGS in fjalar.am to enable optimizations.

Split out of #103, which depends on this pull request. In #103, apply-patch-files.sh runs the script.

Almost every line of Fjalar's Makefile.am renames Memcheck to Fjalar, so most upstream changes to memcheck/Makefile.am conflict when they are patched in. generate-fjalar-makefile.sh instead generates it from valgrind/memcheck/Makefile.am, and Fjalar's changes to the build are in valgrind/fjalar/fjalar.am, which the generated file includes. That includes the fjalar_core_decls.h consistency check. memcheck.patch omits Makefile.am, and the merge doc and the upgrade plans say to run the script after the Memcheck merge. Split out of codespecs#103.

coderabbitai · 2026-10-17T18:45:05Z

Warning

Review limit reached

Run on-demand review

_{This review includes 9 billable files and costs up to $2.25.}

Or wait 25 minutes for your next included review.

Check out review usage here.

View limit details

Limit details: You’ve used the included review currently available.

Learn how review limits work.

Review configuration:

⚙️ Run configuration

Configuration used: Repository: codespecs/fjalar/.coderabbit.yaml
Review profile: ASSERTIVE
Plan: Advanced
Run ID: 69d8abba-9f7d-453e-a86e-76c923fd6525

📥 Commits

Reviewing files that changed from the base of the PR and between 56e2000 and dd45210.

📒 Files selected for processing: 9

doc/valgrind-merge.texinfo
generate-fjalar-makefile.sh
generate-patch-files.sh
make-fjalar-release.sh
plan-upgrade-readelf.md
plan-upgrade-valgrind.md
valgrind/fjalar/Makefile.am
valgrind/fjalar/fjalar.am
valgrind/fjalar/notes/fjalar-release-notes.txt

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: 2e07f345-78af-4697-b1fc-482e07b4de9c

📥 Commits

Reviewing files that changed from the base of the PR and between d962317 and 56e2000.

📒 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 changes add fjalar.am as the source of Fjalar-specific build configuration. A new script generates Fjalar’s Makefile.am from Memcheck’s Makefile, includes that configuration, and checks for remaining references and conflict markers. Patch generation excludes Makefile.am files. Merge, dependency, and release instructions now direct maintainers to update fjalar.am and regenerate the Makefile.

Priority: ➖ Normal

Change: Feature

Merge Risk: ⚪ Minimal · up to 56e20

Fjalar’s generated build configuration and maintainer instructions show no established merge-blocking behavior change. The Darwin debug-symbol declaration is preserved; proceed with normal build checks.

### Architecture Summary

Architecture risk: 🔵 Low · up to 56e20

The change affects 7 systems.

Changed systems: valgrind, doc, generate-fjalar-makefile.sh, generate-patch-files.sh, make-fjalar-release.sh, plan-upgrade-readelf.md, plan-upgrade-valgrind.md

Architecture concerns
No architecture-level concerns identified.

Review details

Systems and components

observed — valgrind (service) was modified; 3 changed files map to changed impact.
observed — doc (service) was modified; 1 changed file maps to changed impact.
observed — generate-fjalar-makefile.sh (service) was modified; 1 changed file maps to changed impact.
observed — generate-patch-files.sh (service) was modified; 1 changed file maps to changed impact.

Before / after behavior

observed — Modified behavior in generate-fjalar-makefile.sh: Adds script setup, usage and output defaults, and failure handling: command failures exit with status 1, and the temporary file is removed on exit. The output is generated in a temporary file before being replaced.
observed — Modified behavior in generate-fjalar-makefile.sh: Adds Makefile generation from Memcheck’s Makefile: renames memcheck- and memcheck_ references and MEMCHECK_ variables, removes the EXTRA_DIST assignment for docs/ including its continuations, and appends the Fjalar include. The generated content replaces the selected output.
observed — Modified behavior in generate-fjalar-makefile.sh: Adds validation of the generated output, skipping comment lines and removing memcheck.h before checking for remaining Memcheck or docs/ references and merge-conflict markers. Matching lines are printed and cause exit status 2.
observed — Modified behavior in make-fjalar-release.sh: The build instructions now refer to fjalar/fjalar.am instead of fjalar/Makefile.am, add removal of kvasir/dyncomp_wrappers.c from VGPRELOAD_FJALAR_SOURCES_COMMON, and identify fjalar/Makefile.am as generated.

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 1 functions across 3 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 1 functions across 3 files. (2 skipped: 2 unsupported.)

✨ Finishing Touches: 💡 1

⚔️ Resolve merge conflicts: 💡

Resolve merge conflict in branch generate-fjalar-makefile

🧪 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.}

…ts of pull requests; remove the 3-way-merge alternative The text that codespecs#98, codespecs#99, codespecs#101, codespecs#103, and codespecs#113 change in the plans is left as in codespecs#96, so that those pull requests' versions apply when they land. plan-pre-upgrade.md points to those pull requests rather than restating them. The "Alternative: a 3-way merge for Coregrind" section is removed, because codespecs#103's apply-patch-files.sh performs the same 3-way merge with git apply --3way.

…e path, since the step's shell is in valgrind/fjalar

mernst · 2026-10-19T02:38:57Z

@coderabbitai review

coderabbitai · 2026-10-19T02:39:05Z

✅ 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.

…akefile # Conflicts: # generate-patch-files.sh

…s, describe patch -b

…kefile.am

…ating Makefile.am

mernst mentioned this pull request Oct 17, 2026

Valgrind merge: create patches with git diff and apply them with git apply #103

Open

Use MOSTLYCLEANFILES for the stamp; clarify the generator's exit status

1026663

mernst added a commit to mernst/fjalar that referenced this pull request Oct 18, 2026

Merge codespecs#113 (generate-fjalar-makefile) into git-apply-merge

863e1d9

mernst mentioned this pull request Oct 18, 2026

Upgrade plans: preparation plan and review fixes #105

Open

Valgrind plan, step 6: run generate-fjalar-makefile.sh by its absolut…

d962317

…e path, since the step's shell is in valgrind/fjalar

Merge remote-tracking branch 'upstream/master' into generate-fjalar-m…

56e2000

…akefile # Conflicts: # generate-patch-files.sh

mernst requested a review from markro49 October 19, 2026 16:39

mernst assigned markro49 Oct 19, 2026

markro49 requested changes Oct 19, 2026

View reviewed changes

Comment thread doc/valgrind-merge.texinfo

Comment thread doc/valgrind-merge.texinfo

Comment thread doc/valgrind-merge.texinfo

markro49 assigned mernst and unassigned markro49 Oct 19, 2026

mernst added 2 commits October 19, 2026 11:51

Merge doc: set DAIKONDIR when cloning, explain removing memcheck file…

e44758a

…s, describe patch -b

Merge branch 'master' into generate-fjalar-makefile

dd45210

mernst assigned markro49 and unassigned mernst Oct 19, 2026

mernst added 2 commits October 19, 2026 12:34

Fjalar manual: to create a tool, edit fjalar.am, not the generated Ma…

5a4ec06

…kefile.am

Revert the merge doc changes of e44758a, which are unrelated to gener…

ee4b2c1

…ating Makefile.am

mernst mentioned this pull request Oct 19, 2026

Merge doc: set DAIKONDIR when cloning; explain the .orig files of patch -b #127

Open

Navigation Menu

Repository navigation

Generate valgrind/fjalar/Makefile.am from Memcheck's - #113

Generate valgrind/fjalar/Makefile.am from Memcheck's#113
mernst wants to merge 8 commits into
codespecs:masterfrom
mernst:generate-fjalar-makefile

mernst commented Oct 17, 2026 •

edited

Loading

Uh oh!

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

Review limit reached

Uh oh!

mernst commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

mernst commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

coderabbitai Bot commented Oct 17, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Review limit reached

Uh oh!

mernst commented Oct 19, 2026

Uh oh!

coderabbitai Bot commented Oct 19, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

This branch has not been deployed

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

mernst commented Oct 17, 2026 •

edited

Loading

coderabbitai Bot commented Oct 17, 2026 •

edited

Loading

coderabbitai Bot commented Oct 19, 2026 •

edited

Loading