koga: depend the generated lisp-info on libclasp, not just iclasp - #1829
koga: depend the generated lisp-info on libclasp, not just iclasp#1829dg1sbg wants to merge 1 commit into
Conversation
generate-lisp-info runs iclasp, which loads libclasp at runtime via rpath and dumps the packages, functions, variables, classes and type map it finds there. Its output therefore depends on libclasp's contents. The edge declared only iclasp as an implicit input, and iclasp depends on libclasp order-only -- in ninja that means "build it first if missing, but changes to it do not make me dirty". Since iclasp is just main.o plus a link, it rarely changes on its own, so editing any Lisp or C++ source never refreshed the generated files. Before this change, touching libclasp.so and rebuilding left the generated files untouched and ninja reported "no work to do". Walking one build tree through eight branches in sequence left them at their original timestamp and byte sizes throughout, including across a branch that adds C++ sources and therefore new scraped symbols -- eight images built from another branch's lisp-info, with nothing to indicate it. Naming libclasp as an implicit input restores the dependency. lib-filename already returns the static archive under --static-linking, so one call covers both linkage modes. Verified on x86-64 Linux: the edge now lists libclasp.so, touching it marks the generator dirty, the files regenerate, and the regression suite is unchanged at 1963.
|
CI summary: all three macOS jobs pass; all three Linux jobs failed on known pre-existing flakes. Every one of them built successfully — the failures are in test steps, not compilation.
The checks that matter for this change specifically: no test loss anywhere (1962 is baseline minus the single flaky test; 1963 is baseline exactly), zero A correction to my note on #1826I said there that this PR "makes the generator run constantly, so every run is an opportunity for the truncation bug in #1826 to corrupt a file" and that #1826 should therefore land first. That is right for a reused build tree — local development, a CI cache, It is not a risk for CI as configured here, which builds from scratch in a fresh runner, so there is no pre-existing longer file to leave a tail. That is consistent with what these runs show: three clean builds and no sign of generated-file corruption. So the ordering preference stands for the benefit of everyone reusing a tree, but it is not a blocker for merging this on CI evidence, and I should have drawn that distinction the first time. |
|
Superseded by #1826, which now carries both this change and the They are opposite halves of the same failure and have to land together: today the truncation bug is rare because the generator seldom runs, so fixing this dependency alone would make it fire constantly, while fixing the truncation alone leaves the generated files stale. Combining them at review request. The commit here is preserved unchanged in #1826 ( |
1 participant
The generated lisp-info files never regenerate when the Lisp or C++ content changes, so a build can silently use package, symbol and class data belonging to a different tree state.
The broken chain
generate-lisp-inforunsiclasp, which loadslibclasp.soat runtime via rpath and dumps the packages, functions, variables, classes and type map it finds there. Its output therefore depends onlibclasp.so's contents. But the only path from the generator to that library runs throughiclasp, which depends on it order-only — in ninja that means "build it first if missing, but changes to it do not make me dirty". Andiclaspis justmain.oplus a link, so it rarely changes on its own.The declaration of
iclaspas an implicit input shows the dependency was intended; it just names the wrong artifact. All the content the generator reads lives in the library, not in the executable.Reproduction, before the fix
Worse in practice: walking a single build tree through eight branches in sequence, rebuilding at each, left the generated files at their original timestamp and byte sizes the whole way — including across a branch that adds new C++ sources and therefore new scraped symbols. Eight images, each built from another branch's lisp-info, with nothing to indicate it.
The change
lib-filenamealready returns.aunder--static-linking, so a single call is correct for both linkage modes.Verification, x86-64 Linux / LLVM 18
Full build and regression suite unaffected.
Relationship to #1823 / #1826
Separate bugs, similar damage, and they interact: #1823 is the generator leaving a stale tail when it runs; this is the generator not running when it should. Fixing #1823 alone is insufficient, since the regeneration that would apply that fix never happens.