#35765 depends: keep local source archives in WORK_PATH
| Reviewability | Reviews | Agreement | Size | ||||
|---|---|---|---|---|---|---|---|
| #35765 depends: keep local source archives in WORK_PATH | Ready | 1 | Strong | 10 | |||
|
|
|
|
| |||
Reviewability: ReadyStalePaused Agreement: StrongPositivePositive w/ caveatsNeutralMildDisputedBlockedCrickets Reviews: current code-review ACKs, then (+stale ACKs) and -NACKs; greener = more ACKs. Size: lines added or modified outside tests, then in tests; greener = smaller. | |||||||
Goal
Stores local package tarballs and their download stamps in `WORK_PATH/local-sources` instead of `SOURCES_PATH`. This ensures generated snapshots remain isolated to each checkout's work directory while downloaded dependency archives continue to be shared.
Problem: When multiple checkouts share a `SOURCES_PATH` cache, depends can reuse a local package tarball generated by a different checkout if that tarball has a newer mtime. In environments like Guix (`contrib/guix/guix-build`), where every checkout is mounted at `/bitcoin` and file mtimes are clamped to 2000-01-01, depends reliably reuses the wrong checkout's tarball without detecting source file changes.
Evidence of importance:
- Commit message: 'Local packages... are built from a tarball of the local directory... stored in SOURCES_PATH next to downloaded sources. But unlike downloaded sources, it has no fixed name or hash identifying its contents, so when SOURCES_PATH is shared between checkouts, a checkout can find another checkout's tarball, see no newer files, and build from the wrong sources. This happens reliably with contrib/guix/guix-build...'
- PR description notes focused reproduction of the collision and verification of isolation.
Risk of a mistake: low (build). A bug in local package path resolution could break builds of packages configured with local_dir or fail to properly cache and rebuild them during development.
Changed since the previous assessment: Replaced the previous approach (which appended a hash of the local checkout path to the archive filename in SOURCES_PATH) with moving local package archives and stamps entirely into per-checkout WORK_PATH directories, resolving collisions where checkouts share identical absolute paths like /bitcoin inside Guix containers.
Reviewability
Ready
Nothing known that would invalidate a review now.
Waiting on (from the thread read):
- reviewer: review and Guix testing from another contributor since ryanofsky authored the WORK_PATH commit 2026-10-15
Agreement
Strong
substantive support, no open objection (ryanofsky, ryanofsky, ryanofsky)
Thread notes: ryanofsky approved the updated push adopting the WORK_PATH approach, marking their earlier suggestion accepted. Additional review and Guix testing from another contributor was requested.
Objections, suggestions, and questions:
| Reviewer | Type | Harm, request, or question | Status | Must be | Author replied | Quote |
|---|---|---|---|---|---|---|
| ryanofsky (member) | suggestion, maintenance | Drop or simplify the packages.md addition since focusing on internal implementation details makes visible behavior harder to understand | fixed · fix pushed · author dropped the doc changes in a push | answered | yes | 2026-09-21: 'I do find the PR description and the `packages.md` addition a bit confusing though because they both seem focused on internal implementation details, instead of describing visible behavior.' Status from: 2026-09-23: willcl-ark: 'Dropped the doc comment in latest push.' |
| ryanofsky (member) | suggestion, approach | Store local package tarballs in WORK_PATH instead of SOURCES_PATH so they are automatically not shared across checkouts without needing path hashes | accepted · fix pushed · author adopted the WORK_PATH approach and ryanofsky approved | answered | no | 2026-09-30: 'I think a simpler alternative could be to store local package tarballs in `WORK_PATH` instead of `SOURCES_PATH`' Status from: 2026-10-15: ryanofsky: 'Thanks for picking up the `WORK_PATH` approach' |
Support:
- ryanofsky (member) (ACK, 2026-09-21) [read]: Code changes look right, matching the patch from #35764
- ryanofsky (member) (ACK, 2026-09-30) [read]: Documentation change was dropped; looks good as is
- ryanofsky (member) (ACK, 2026-10-15) [read]: Approves the adoption of the WORK_PATH approach and commit message update
Participants:
- ryanofsky (member, support, 5 statements 2026-09-21 to 2026-10-15): ACKed the initial code, suggested and ACKed the WORK_PATH approach
Files
0 lines under test/bench/ci.
- depends/packages.md +5/-4
- depends/funcs.mk +4/-2
- depends/Makefile +1/-0
Card
The short text the ranking pass compares across PRs.
This pull request moves depends local package archives and download stamps from SOURCES_PATH to WORK_PATH/local-sources. Previously, checkouts sharing a SOURCES_PATH cache could reuse another checkout's local package archive if its timestamp was newer, causing silent builds from stale or foreign source trees. This issue reliably affected contrib/guix/guix-build, where container paths and file timestamps are normalized. Storing local archives in the per-checkout work directory eliminates collisions while keeping downloaded external dependencies shared across builds. It addresses #35764.
Data
thread read through 2026-10-15 (9 statements), head 01491f22b2
labels: Build system
Categories
Build and CI
P1 · bug fix
Prevents builds from silently compiling the wrong source tree when checkouts share a cache. As noted in the commit message, when `SOURCES_PATH` is shared across checkouts, depends can reuse another checkout's package tarball, an issue that 'happens reliably with contrib/guix/guix-build' where mtimes are clamped to 2000-01-01, threatening release integrity and build correctness.
Membership: Matches the 'Build system' label and modifies `depends/` Makefiles to isolate local source packages.
Factors: security/stability 2, bug 2, performance 0, user value 0, leverage 0