fix(ebuild): recreate static archives to remove stale objects

[alz458]

fix(ebuild): recreate static archives to remove stale objects

Summary

Removing a source from a static-library target could leave its compiled object in the existing archive. Applications could still link to deleted functions even though a clean build failed. Recreate the archive when Ninja rebuilds it so its members match the current inputs.

Type of Change

• fix — Bug fix
• test — Add tests
• build — Build-system change

Changes

• Remove the old archive at execution time, then invoke the configured archiver through Python and propagate its exit status.
• Preserve Ninja's unchanged-build behavior and quote the Python executable for the platform and Ninja syntax.
• Add real compiler/archive/linker regression tests for source removal, retained functions, unchanged builds, project paths containing spaces, and archiver failure.
• Document the regression and record unrelated baseline findings as required by AGENTS.md.

Testing

• Source-removal regression failed against the original rule in both path variants, leaving removed.o in the archive.
• Focused backend tests: 23 passed.
• Full suite after change: 672 passed, 10 failed, 2 skipped.
• Untouched base in a separate directory: 669 passed, the same 10 failed, 2 skipped.
• Nine failures concern the existing missing PackageRecipe.to_dict; one is a sandbox denial accessing the home cache.
• Changed Python files pass Ruff. Repository-wide Ruff has four existing findings; mypy has one existing missing-method error. Both were reproduced on the untouched base.
• Wheel build and git diff --check pass.
• Independent review found no blocking issues and additionally exercised interpreter-path quoting and archiver failure.
• Validation ran on macOS with Python 3.13. Windows execution has not been verified.

Pre-Submission Checklist

• Regression tests added and run
• Testing documentation updated
• Based on current master as checked during preparation
• All existing tests pass — baseline failures documented above
• DCO-signed conventional commit created
• Linked issue added

Related Issue

Fixes #136

Additional Notes

[srpatcha]

Review — ebuild#137 "fix(ebuild): recreate static archives to remove stale objects"

head: 11f94de author: alz458 ci: pass (policy / Policy / Linked Issue)

Verdict: Correct fix for a real and nasty defect — ar r replaces named members and silently keeps omitted ones, so a deleted source kept linking. The regression test is the good kind: it builds with a real compiler, deletes a source, and then asserts that the deleted function no longer links while the retained one still does, so it cannot pass against a merely-broken archive. Every claim in the body reproduces here. Two things to fix before merge, both small: the wrapper turns a missing archiver into a Python traceback, and build.ninja now embeds the absolute path of whichever interpreter generated it.

This PR is also what makes open PR #133 safe — see "Interaction with #133" below.

Findings

#	Severity	File:line	Finding	Recommended fix
1	Medium	ebuild/build/ninja_backend.py:213-215	subprocess.call raises FileNotFoundError when the archiver is missing, and nothing catches it, so the developer gets a nine-line Python traceback ending in FileNotFoundError: [Errno 2] No such file or directory: 'no-such-ar-binary' where the previous rule produced sh: no-such-ar-binary: not found. Verified by generating with ar="no-such-ar-binary" and running ninja — full traceback reproduced, including frames from CPython's subprocess.py. §9.2 requires "Actionable diagnostics with remediation guidance", and .ai/tooling.md is explicit: "Raw tracebacks and internal exception strings do not surface as the primary message." A missing or misconfigured ar is the single most likely way this rule fails in the field — a half-installed cross toolchain — so this is the diagnostic that matters most.	Wrap the call. Inside the -c script: try: sys.exit(subprocess.call(sys.argv[2:])) / except OSError as e: sys.exit("ebuild: cannot run archiver %s: %s" % (sys.argv[2], e)). One line, keeps the exit status contract, and sys.exit(str) prints to stderr and exits 1. Your test_archive_failure_is_reported_by_ninja already covers the nonzero-status path; add the missing-binary path next to it and assert the traceback is absent.
2	Low	ebuild/build/ninja_backend.py:192-195	The generated build.ninja hardcodes sys.executable as an absolute path. Verified — the emitted rule reads command = /tmp/ar137.cQ1H/.venv/bin/python -c "…". If ebuild is reinstalled at a different prefix (pipx upgrade, venv recreated, image layer rebuilt, tree bind-mounted into a container), an existing build.ninja fails with no indication that the interpreter is what moved. It also makes build.ninja differ between two developers who generated it from the same sources. ebuild build regenerates the manifest first, so this only bites someone invoking ninja directly — which is why this is Low and not Medium — but that is a supported thing to do and the TESTING.md section this PR adds tells people to do exactly that.	Cheapest honest fix is to say so: note in the TESTING.md paragraph you are already adding that build.ninja is tied to the generating interpreter and must be regenerated if the environment changes. If you would rather make it robust, emit sys.executable but have generate() compare against the recorded value and rewrite unconditionally when it differs.
3	Low	tests/ebuild/test_ninja_backend.py vs tests/unit/test_ninja_backend.py	Two files with the same name, in the same repo, both testing NinjaBackend. This PR adds its regression to tests/ebuild/; open PR #133 adds its regression for the same class to tests/unit/. Neither is wrong, but a contributor has no rule telling them which one a new backend test belongs in, and the class's coverage is now split across two modules that neither imports nor references the other. Brief item 10.	Not this PR's to fix, and not worth blocking on. Worth one line in TESTING.md stating the split rule (tests/unit/ = manifest-text assertions, tests/ebuild/ = tests that shell out to a real compiler/ninja), since you are editing that file anyway. That is the distinction the two files actually observe today.

I did not raise the extra interpreter spawn per archive step as a finding. It is real — roughly one process launch per static library per build — but the portability reason given in the comment ("Python also works on Windows, unlike rm") is sound, the cost is bounded by the number of static_library targets rather than by source count, and the alternative would be a POSIX/Windows fork in the generator. Correct trade.

Evidence check

Reproduced against a git archive export of head 11f94de2, in a clean venv from pyproject.toml, with host cc, ar and the ninja Python package present:

Claim in the PR	Result here
"Source-removal regression failed against the original rule in both path variants, leaving removed.o in the archive"	Confirmed. Reverting the rule to $ar rcs $out $in fails both test_removing_source_removes_archive_member[project] and [project with spaces], each with assert 'removed.o' not in ['keep.o', 'removed.o']. 2 failed, 5 passed.
"Focused backend tests: 23 passed"	23 passed — 7 in tests/ebuild/test_ninja_backend.py + 16 in tests/unit/test_ninja_backend.py.
"Nine failures concern the existing missing PackageRecipe.to_dict"	Confirmed independently while reviewing #133 and #124: those 9 reproduce on plain master and are what #124 fixes. Correctly scoped out.
"Windows execution has not been verified"	Correct to say, and the reason this is not a finding. Windows is the one platform where the rm alternative was rejected and the cmd.exe quoting path is untested; stating that plainly is what .ai/reviewer.md asks for. Note that the CI matrix's windows-2022 legs will skip both new tests anyway — shutil.which("cc") returns None there — so CI will not close this gap for you.

The tests will genuinely run in CI on the ubuntu and macOS legs: ninja>=1.11 is a runtime dependency in pyproject.toml and ci.yml:54 does pip install -e ., ci.yml:88 runs the whole tests/ tree. I checked this specifically because a pytest.skip guarded by shutil.which is the classic way a new test quietly never executes.

Interaction with #133

The two merge cleanly — git merge-tree over the common base 8b623d57 reports both files "changed in both" with no conflict markers, because #133 touches _object_path and this PR touches _write_ninja. But they are related, and the order matters:

#133 renames objects from src/util.o to src/util.c.o. Under the old ar rcs rule that rename strands the old member in every existing archive — I verified on #133 that the result is one archive containing both util.o and util.c.o with the symbol defined twice. This PR removes that hazard entirely, because the archive is recreated rather than updated. So: land #137 first, then #133, and #133 needs no migration note. Landing them the other way round leaves a window where a pull-and-rebuild produces duplicate-symbol link errors. I have said the same on #133.

Architecture conformance

Conforms.

• §21 / Tier 1. ebuild is Tier 1 — Foundation, and the ninja generator is eBuild's own concern. §9.1 puts Configure/Build inside the eBuild engine; emitting the archive rule is that responsibility.
• §5.1. Not engaged. The diff adds two stdlib imports (shlex, subprocess) to a build-time module. Nothing points up a tier, and nothing here is a runtime dependency — "eBuild understands the complete graph but is not a runtime dependency" holds.
• Brief item 8 (API/wire compatibility). No public signature, struct, manifest field or serialized format changes. The ar_rule text in a generated build.ninja is not a declared contract in §23.2 — that table covers the eBuild project format, package format, firmware format and board definitions, not the build directory. Nothing to migrate, so the body's silence is correct. Finding 2 is about robustness, not compatibility.
• Brief item 11 (documentation). Met, and better than most: TESTING.md gains the reason the rule exists ("updating an existing archive with ar rcs alone retains members removed from the source list"), which is the sentence that stops someone reverting this in six months. TASKS.md records the unrelated baseline failures rather than hiding them.
• One note on TASKS.md: this PR appends a prose section and does not claim a T-00x ID, so unlike #133 and #135 — which have both independently claimed T-006 — it creates no ID collision. Fine as is.

Proposed changes

1. Catch OSError around subprocess.call in the -c script and exit with a one-line message (finding 1).
2. Add the sentence about build.ninja being tied to the generating interpreter to the TESTING.md section you are already writing (finding 2).
3. Optional: one line in TESTING.md stating the tests/unit/ vs tests/ebuild/ split (finding 3).
4. Ask for #137 to merge before #133.

Not checked

• Windows — NOT RUN. No Windows host available. The subprocess.list2cmdline branch at :193, and how cmd.exe parses the -c "…" string containing ;, [ and ], are unverified. The author says the same; I am confirming, not contradicting. CI will not cover it either (see above).
• macOS — NOT RUN. The body's results are from macOS with Python 3.13; mine are Linux with the venv's CPython 3.12. llvm-ar's and Apple ar's behaviour on unlink + recreate was not tested here, only GNU ar.
• Full suite — NOT RUN. I ran the two test_ninja_backend.py modules (23 tests). The "672 passed, 10 failed, 2 skipped" and "untouched base 669 passed" comparison is unverified in this review, though the 9 to_dict failures within it were reproduced separately.
• mypy — NOT RUN. Not installed on this host. The "one existing missing-method error" claim is unverified.
• Wheel build and git diff --check — NOT RUN.
• Cross toolchains — NOT RUN. Only host cc/ar were exercised. A $ar that is a multi-word command (e.g. ar --plugin=…) still works in principle — ninja substitutes it into the shell command line, the shell splits it, and subprocess.call(sys.argv[2:]) receives the pieces as separate argv entries — but I did not test that configuration.
• I did not examine whether any packaging or SBOM step reads build.ninja and would now record an interpreter path as a build input.

---

Automated architecture review of 11f94de2790d — scheduled, model claude-opus-5, checked against the EmbeddedOS Master Design v2.0. Advisory only: this reviewer never approves, requests changes, or merges. Reply here to discuss or push back — a wrong finding is a bug worth reporting.