diff --git a/TASKS.md b/TASKS.md index 09f6bcb..b382a39 100644 --- a/TASKS.md +++ b/TASKS.md @@ -109,3 +109,13 @@ These commands were derived from the manifests at the repository root. Confirm o [ORCHESTRATION.md](./ORCHESTRATION.md) is met and the verification commands were actually run. - `blocked` requires a note naming what it is blocked on and who can unblock it. + +## Baseline findings during static-archive validation + +Unchanged commit `76970c9` and the archive fix both have nine failing tests +caused by missing `PackageRecipe.to_dict`, plus one offline-index test denied +access to the home cache by the local sandbox. Mypy also reports the missing +method. Ruff reports four existing findings in `test_build_dir_resolution.py`, +`test_package_recipe.py`, and `test_ci_gate.py`. These are outside the archive +fix; the before/after full-suite results are respectively 669/672 passed, +10 failed, and 2 skipped on macOS with Python 3.13. diff --git a/TESTING.md b/TESTING.md index 7039b08..767795e 100644 --- a/TESTING.md +++ b/TESTING.md @@ -66,3 +66,16 @@ passed, failed and skipped. A skipped test is not a passing test. Any test that could not be run in this environment is named, with the reason, and marked `NOT RUN` or `UNKNOWN` per [VERIFY.md](./VERIFY.md). + +## Static-library source removal regression + +Run `python -m pytest tests/ebuild/test_ninja_backend.py -v` to check native +archive rebuilding. The source-removal test requires a host C compiler, `ar`, +and the Ninja Python package. It builds a library, removes a source, and +checks that the old object is absent and its function no longer links. It +also verifies that retained code still links, unchanged archives are not +rebuilt, and project paths containing spaces work. A separate test checks +that an archiver failure makes Ninja fail. + +Static archives are recreated when their build step runs: updating an existing +archive with `ar rcs` alone retains members removed from the source list. diff --git a/ebuild/build/ninja_backend.py b/ebuild/build/ninja_backend.py index de417bd..5bf2cec 100644 --- a/ebuild/build/ninja_backend.py +++ b/ebuild/build/ninja_backend.py @@ -9,6 +9,8 @@ from __future__ import annotations import json +import shlex +import subprocess import sys from dataclasses import dataclass, field from pathlib import Path @@ -187,6 +189,10 @@ def _object_path(self, target, src: str) -> Path: def _write_ninja(self) -> None: """Write the build.ninja file.""" ninja_path = self.build_dir / "build.ninja" + python_command = ( + subprocess.list2cmdline([sys.executable]) + if sys.platform == "win32" else shlex.quote(sys.executable) + ).replace("$", "$$") lines = [ f"# Generated by ebuild", f"cc = {self.toolchain.cc}", @@ -204,7 +210,12 @@ def _write_ninja(self) -> None: " description = LINK $out", "", "rule ar_rule", - " command = $ar rcs $out $in", + # ar replaces supplied members but keeps omitted ones. Recreate + # the archive when Ninja rebuilds it so removed sources cannot + # survive as stale code. Python also works on Windows, unlike rm. + f' command = {python_command} -c "from pathlib import Path; import subprocess, sys; ' + 'Path(sys.argv[1]).unlink(missing_ok=True); ' + 'sys.exit(subprocess.call(sys.argv[2:]))" $out $ar rcs $out $in', " description = AR $out", "", # A shared_library edge names this rule. Without the rule the diff --git a/tests/ebuild/test_ninja_backend.py b/tests/ebuild/test_ninja_backend.py index d19dd8e..1d97f30 100644 --- a/tests/ebuild/test_ninja_backend.py +++ b/tests/ebuild/test_ninja_backend.py @@ -194,3 +194,88 @@ def run_ninja(): "was reused and the build wrongly reported success" ) assert "header was recompiled" in (second.stdout + second.stderr) + + +@pytest.mark.parametrize("directory", ["project", "project with spaces"]) +def test_removing_source_removes_archive_member(tmp_path, directory): + """Incremental archives must contain only the current source objects.""" + cc = shutil.which("cc") or shutil.which("gcc") + ar = shutil.which("ar") + if not cc or not ar or importlib.util.find_spec("ninja") is None: + pytest.skip("host C compiler, ar, and ninja are required") + + source_dir = tmp_path / directory + source_dir.mkdir() + (source_dir / "keep.c").write_text("int keep(void) { return 1; }\n") + (source_dir / "removed.c").write_text("int removed(void) { return 42; }\n") + main = source_dir / "main.c" + main.write_text("int removed(void); int main(void) { return removed(); }\n") + library = TargetConfig( + name="helpers", target_type="static_library", + sources=["keep.c", "removed.c"], + ) + config = ProjectConfig( + name="archive-regression", version="1.0", source_dir=source_dir, + targets=[library], + ) + build_dir = source_dir / "build" + archive = build_dir / "libhelpers.a" + toolchain = SimpleNamespace(cc=cc, cxx="c++", ar=ar) + + def build(): + NinjaBackend(config, build_dir, toolchain).generate() + result = subprocess.run( + [sys.executable, "-m", "ninja", "-f", str(build_dir / "build.ninja")], + cwd=source_dir, capture_output=True, text=True, + ) + assert result.returncode == 0, result.stdout + result.stderr + + def link(): + return subprocess.run( + [cc, str(main), str(archive), "-o", str(build_dir / "app")], + capture_output=True, text=True, + ) + + build() + assert link().returncode == 0 + initial_mtime = archive.stat().st_mtime_ns + build() + assert archive.stat().st_mtime_ns == initial_mtime, "unchanged archive rebuilt" + + library.sources.remove("removed.c") + (source_dir / "removed.c").unlink() + build() + members = subprocess.check_output([ar, "t", str(archive)], text=True) + assert "removed.o" not in members.splitlines(), members + assert "keep.o" in members.splitlines(), members + assert link().returncode != 0, "deleted function still links from stale code" + + # The retained function still links, so this is not merely a broken archive. + main.write_text("int keep(void); int main(void) { return keep() - 1; }\n") + result = link() + assert result.returncode == 0, result.stdout + result.stderr + + +def test_archive_failure_is_reported_by_ninja(tmp_path): + """The Python wrapper must preserve the archiver's failure status.""" + cc = shutil.which("cc") or shutil.which("gcc") + if not cc or importlib.util.find_spec("ninja") is None: + pytest.skip("host C compiler and ninja are required") + (tmp_path / "lib.c").write_text("int value(void) { return 1; }\n") + config = ProjectConfig( + name="archive-failure", version="1.0", source_dir=tmp_path, + targets=[TargetConfig( + name="helpers", target_type="static_library", sources=["lib.c"], + )], + ) + # Python receives 'rcs' as a script name and fails: a portable stand-in + # for an archiver returning a nonzero status, without a shell script. + toolchain = SimpleNamespace(cc=cc, cxx="c++", ar=sys.executable) + build_dir = tmp_path / "build" + NinjaBackend(config, build_dir, toolchain).generate() + result = subprocess.run( + [sys.executable, "-m", "ninja", "-f", str(build_dir / "build.ninja")], + cwd=tmp_path, capture_output=True, text=True, + ) + assert result.returncode != 0, result.stdout + result.stderr + assert "rcs" in result.stdout + result.stderr