From b010e46fba57019326cf6cedcf79230bddb6dbc3 Mon Sep 17 00:00:00 2001 From: Christopher Wellons Date: Sat, 11 Apr 2026 16:24:27 -0400 Subject: [PATCH] ninja: Fix deps log recovery In DepsLog::Load(), the file offset used for truncation-based recovery was advanced past each record immediately after reading its bytes from disk, but before validating the record's semantic correctness (e.g. the path checksum or node-id range checks). When a record was structurally complete but semantically invalid, recovery truncated the file to an offset that included the bad record, so it was never removed. This created an infinite cycle: every build would load the deps log, hit the corrupt record, "recover" by truncating everything after it (but not the record itself), then append new dependency records that would be destroyed by the next build's recovery. The result was spurious rebuilds on every invocation. Move the offset update to the end of the loop body, after all validation has passed. Now when a record fails validation, the offset still points before it, and truncation correctly removes it. (This patch has been submitted upstream.) --- Dockerfile | 4 +++- src/ninja-fix-recovery.patch | 29 +++++++++++++++++++++++++++++ 2 files changed, 32 insertions(+), 1 deletion(-) create mode 100644 src/ninja-fix-recovery.patch diff --git a/Dockerfile b/Dockerfile index 0b855ff..e12f145 100644 --- a/Dockerfile +++ b/Dockerfile @@ -721,9 +721,11 @@ RUN $ARCH-gcc -DEXE=ccache.exe -DCMD=gcc \ FROM cross AS build-ninja COPY --from=dl-ninja /dl/ /dl/ +COPY src/ninja-*.patch $PREFIX/src/ WORKDIR /ninja -RUN cmake -DCMAKE_BUILD_TYPE=Release \ +RUN cat $PREFIX/src/ninja-*.patch | patch -d/dl/ninja -p1 \ + && cmake -DCMAKE_BUILD_TYPE=Release \ -DCMAKE_SYSTEM_NAME=Windows \ -DCMAKE_CXX_COMPILER=$ARCH-g++ \ -DCMAKE_EXE_LINKER_FLAGS="-s" \ diff --git a/src/ninja-fix-recovery.patch b/src/ninja-fix-recovery.patch new file mode 100644 index 0000000..1a467f9 --- /dev/null +++ b/src/ninja-fix-recovery.patch @@ -0,0 +1,29 @@ +Fix deps log recovery failing to remove corrupt records + +In DepsLog::Load(), the file offset used for truncation-based recovery +was advanced past each record immediately after reading its bytes from +disk, but before validating the record's semantic correctness (e.g. the +path checksum or node-id range checks). When a record was structurally +complete but semantically invalid, recovery truncated the file to an +offset that included the bad record, so it was never removed. + +This created an infinite cycle: every build would load the deps log, hit +the corrupt record, "recover" by truncating everything after it (but not +the record itself), then append new dependency records that would be +destroyed by the next build's recovery. The result was spurious rebuilds +on every invocation. + +Move the offset update to the end of the loop body, after all validation +has passed. Now when a record fails validation, the offset still points +before it, and truncation correctly removes it. + +--- a/src/deps_log.cc ++++ b/src/deps_log.cc +@@ -205,3 +205,2 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) { + } +- offset += size + sizeof(size); + +@@ -270,2 +269,3 @@ LoadStatus DepsLog::Load(const string& path, State* state, string* err) { + } ++ offset += size + sizeof(size); + }