depends: keep local source archives in WORK

willcl-ark commented at 2:40 PM on July 21, 2026: member

Local package tarballs are generated from the current checkout, but depends caches them alongside downloaded archives in SOURCES_PATH. When checkouts share that directory, a cached local tarball can be newer than another checkout's source files, causing depends to build the wrong sources with a valid checksum and build ID.

Store local package tarballs and their checksum stamps in WORK_PATH/local-sources instead. WORK_PATH is per-checkout by default, so generated snapshots stay with their checkout while downloaded sources remain shareable through SOURCES_PATH. Existing mtime-based invalidation is unchanged.

This also handles contrib/guix/guix-build, which mounts each checkout at /bitcoin. Distinguishing local tarballs by absolute source path cannot isolate those builds when SOURCES_PATH is shared.

Validation: focused checks reproduced the same-path collision with the path-hash approach and verified isolation with this change. Host/container source checksums and build IDs match; archive reuse, refresh after an edit, and downloaded-source paths were also checked. No full Guix build was run.

Addresses #35764.

DrahtBot added the label Build system on Jul 21, 2026

DrahtBot commented at 2:40 PM on July 21, 2026: contributor

The following sections might be updated with supplementary metadata relevant to reviewers and maintainers.

Code Coverage & Benchmarks

For details see: https://corecheck.dev/bitcoin/bitcoin/pulls/35765.

Reviews

See the guideline and AI policy for information on the review process.

Type	Reviewers
Stale ACK	ryanofsky

If your review is incorrectly listed, please copy-paste <code></code> into the comment that the bot should ignore.

Conflicts

No conflicts as of last run.

willcl-ark commented at 2:41 PM on July 21, 2026: member

Not sure if it might be better to set depends mtime to 2000-01-01 12:00:00 UTC as is done by default_build_TOUCH already? Perhaps it could be...

willcl-ark commented at 7:34 AM on July 22, 2026: member

Build of this PR:

x86_64
eb548f6197dee88a0e125c04f7c43b7e3360f963c5b62d6dd7096def4d25ae82  guix-build-b1f79c9fa6dd/output/aarch64-linux-gnu/SHA256SUMS.part
30109d042a7a31893422649b11b5ac54fc2702d27dabedb9e286dc740e475cc4  guix-build-b1f79c9fa6dd/output/aarch64-linux-gnu/bitcoin-b1f79c9fa6dd-aarch64-linux-gnu-debug.tar.gz
04c50689d60f5f2d17631a7443cbd4ae5084f31de1934f22d488cdc630721816  guix-build-b1f79c9fa6dd/output/aarch64-linux-gnu/bitcoin-b1f79c9fa6dd-aarch64-linux-gnu.tar.gz
0a7a24fdf440a63382f1abf83c4a4cd4700bc68e14dbbb2d51ea98c8a0a65ac6  guix-build-b1f79c9fa6dd/output/arm-linux-gnueabihf/SHA256SUMS.part
9d1383085f267a8bf57842330ef856c4e1f0fd4efa899705a0e6b8e0f82e7144  guix-build-b1f79c9fa6dd/output/arm-linux-gnueabihf/bitcoin-b1f79c9fa6dd-arm-linux-gnueabihf-debug.tar.gz
e5bc8d5e63a299e64434dce0bcd0d1a953de77d0c12ea87dea1fe4af529f20b9  guix-build-b1f79c9fa6dd/output/arm-linux-gnueabihf/bitcoin-b1f79c9fa6dd-arm-linux-gnueabihf.tar.gz
da82d78e01c0cd69e6033633dabf3f4ab0fcb5c9d746ea68607f4f4e56bcedb8  guix-build-b1f79c9fa6dd/output/arm64-apple-darwin/SHA256SUMS.part
13488634f71b849eea3f0ed70fcb96a4c08b7614ed22cd53c17a35030debbd01  guix-build-b1f79c9fa6dd/output/arm64-apple-darwin/bitcoin-b1f79c9fa6dd-arm64-apple-darwin-codesigning.tar.gz
f7cadfe8ec4e02a569751fefa1a0a257c25cf6edcf9af69dcb73f6c697495e4f  guix-build-b1f79c9fa6dd/output/arm64-apple-darwin/bitcoin-b1f79c9fa6dd-arm64-apple-darwin-unsigned.tar.gz
2a3f133c5ad11a657286c9f1b377fac100bfb6335f52a7ac0da565ebed7e8ce7  guix-build-b1f79c9fa6dd/output/arm64-apple-darwin/bitcoin-b1f79c9fa6dd-arm64-apple-darwin-unsigned.zip
0210d6fad3c23629dd44259a318a098ab1d40cd2f614211323da423e00dbd6e2  guix-build-b1f79c9fa6dd/output/dist-archive/bitcoin-b1f79c9fa6dd.tar.gz
4ff5751126abad7c077158cdc5eae9d8826f61752cb73254ce2d718a5bae841f  guix-build-b1f79c9fa6dd/output/powerpc64-linux-gnu/SHA256SUMS.part
4b587f04770cb8fb1676de1ac47b2ff7a19dbb28296b6c86e4ead11cca74c103  guix-build-b1f79c9fa6dd/output/powerpc64-linux-gnu/bitcoin-b1f79c9fa6dd-powerpc64-linux-gnu-debug.tar.gz
627972cee5ff34f41d04fa051a171e9674b28bcc18ec8c2a3224e0aab8265c35  guix-build-b1f79c9fa6dd/output/powerpc64-linux-gnu/bitcoin-b1f79c9fa6dd-powerpc64-linux-gnu.tar.gz
54cd74d473f49f8bda4b198a8db4bb239b9b2b31dcd604072c9067e15e7676cd  guix-build-b1f79c9fa6dd/output/riscv64-linux-gnu/SHA256SUMS.part
0309b1be6f6a95e4a577ce4706bc658b150a6114341b260d6239833909f0a2d3  guix-build-b1f79c9fa6dd/output/riscv64-linux-gnu/bitcoin-b1f79c9fa6dd-riscv64-linux-gnu-debug.tar.gz
d3df748ea3e86ca0a60763955469b92ef496a6f1355c57618aee6ae32f421b4d  guix-build-b1f79c9fa6dd/output/riscv64-linux-gnu/bitcoin-b1f79c9fa6dd-riscv64-linux-gnu.tar.gz
20b826775a5bc1076e7dbb7b2d3025a98442463642e8e3ca22b736bd048feb14  guix-build-b1f79c9fa6dd/output/x86_64-apple-darwin/SHA256SUMS.part
8771f090263129505d2bc2aa333ba243439532713b54bfa792b79172513ca2fc  guix-build-b1f79c9fa6dd/output/x86_64-apple-darwin/bitcoin-b1f79c9fa6dd-x86_64-apple-darwin-codesigning.tar.gz
9fe8f90b64df1a86a0eeda61d3ef25c30c752ff3f3b41f00bf526c181c07339e  guix-build-b1f79c9fa6dd/output/x86_64-apple-darwin/bitcoin-b1f79c9fa6dd-x86_64-apple-darwin-unsigned.tar.gz
18b6ad45aed36993f4c296e79b04de8591bf325b793b7e797ce1819ae9fa6205  guix-build-b1f79c9fa6dd/output/x86_64-apple-darwin/bitcoin-b1f79c9fa6dd-x86_64-apple-darwin-unsigned.zip
06bfebb2f8d63caa53587e4612bb75da416740fb52b9cec7aeac75b8378718b1  guix-build-b1f79c9fa6dd/output/x86_64-linux-gnu/SHA256SUMS.part
bd5a7d537ee343f37eac47aa4b6ce2b762ba11564a8f455993f2ddf673c06e27  guix-build-b1f79c9fa6dd/output/x86_64-linux-gnu/bitcoin-b1f79c9fa6dd-x86_64-linux-gnu-debug.tar.gz
aa4b4dd950669df51fdb0b0cf0ed555cacc13878416390cc8a6a0258eb1490d3  guix-build-b1f79c9fa6dd/output/x86_64-linux-gnu/bitcoin-b1f79c9fa6dd-x86_64-linux-gnu.tar.gz
8f5052762e1f72be1c98b334a8cfdcc91a4f96e9fb4468259225347aec6f44f4  guix-build-b1f79c9fa6dd/output/x86_64-w64-mingw32/SHA256SUMS.part
a534e9c6bf9165c1c093fd9e2a55c2cda1e6a8cfd076beba6ee8717ea61d9168  guix-build-b1f79c9fa6dd/output/x86_64-w64-mingw32/bitcoin-b1f79c9fa6dd-win64-codesigning.tar.gz
783ab6083576c93e74301574d326a07d36953249e17a90bcebb46d436c99d618  guix-build-b1f79c9fa6dd/output/x86_64-w64-mingw32/bitcoin-b1f79c9fa6dd-win64-debug.zip
2a5f744b0eeaccf29ad367b5b1cb32c75f5013792a9ca264894569fce6b3c6e5  guix-build-b1f79c9fa6dd/output/x86_64-w64-mingw32/bitcoin-b1f79c9fa6dd-win64-setup-unsigned.exe
1d2450834b8e53295b9e1317a817e9dddf7a48a5a3352b70fa2e92158a3a8d69  guix-build-b1f79c9fa6dd/output/x86_64-w64-mingw32/bitcoin-b1f79c9fa6dd-win64-unsigned.zip

DrahtBot added the label Needs rebase on Aug 25, 2026

willcl-ark force-pushed on Sep 21, 2026

willcl-ark renamed this:
~~depends: hash local source contents~~
depends: isolate local source archives by path
on Sep 21, 2026

DrahtBot removed the label Needs rebase on Sep 21, 2026

willcl-ark marked this as ready for review on Sep 21, 2026

ryanofsky approved

ryanofsky commented at 6:54 PM on September 21, 2026: contributor

Code review ACK 6ea85f57fbe33c047d951f7e940007771aeb4547. Thanks for the update! Code changes look right, matching the patch from #35764 (comment).

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. The existing Local Packages documentation already says the package is rebuilt when the local directory's contents change, which is what was originally intended and what's fixed by this change. The added text makes it harder to understand I think.

Might suggest PR title "depends: give local package tarballs unique filenames per source path" and description more like:

Problem: Depends Local Packages cache the sources for a package built from a local directory as a tarball in SOURCES_PATH, named only after the source directory's relative path. When multiple depends directories share one SOURCES_PATH, two different source directories can end up with the same tarball name. If one directory's tarball happens to be newer than every file in the other directory's sources, that directory silently builds from the wrong sources: the checksum and build ID are calculated correctly, but for the wrong content.

Solution: Include a hash of the absolute source directory path in the tarball filename, so each source directory gets its own tarball within a shared SOURCES_PATH. The existing mtime-based rebuild check is unchanged.

willcl-ark renamed this:
~~depends: isolate local source archives by path~~
depends: give local package tarballs unique filenames per source path
on Sep 22, 2026

willcl-ark commented at 9:22 AM on September 22, 2026: member

Thanks @ryanofsky I took your title and description wholesale.

I feel like the packages.md change is OK though? It's quite a developer-focused document.

ryanofsky commented at 2:58 PM on September 22, 2026: contributor

Thanks for the update!

I feel like the packages.md change is OK though? It's quite a developer-focused document.

Yeah it's fine. I just think it muddies the description and makes it less clear by describing a problem that the code does not have ("without reusing each others source archives"). The current paragraph ends with a clear takeaway: if you touch the source directory, the package will be rebuilt. Now there are more details about hashes and absolute paths and an interaction with a separate SOURCES_PATH feature which is not well documented itself. It's good for the local packages feature to be compatible with the SOURCES_PATH feature, but it should be a starting assumption that different features are compatible with each other. It is not reassuring for documentation to say if you use this one feature with this other feature the wrong thing won't happen.

I do think the comment would be fine and potentially helpful as code comment in code where the hash is being constructed.

willcl-ark force-pushed on Sep 23, 2026

willcl-ark commented at 10:05 AM on September 23, 2026: member

OK you've persuaded me. Dropped the doc comment in latest push.

in depends/funcs.mk:112 in 22374481a7

 105 | @@ -106,6 +106,10 @@ $(1)_source_dir:=$(SOURCES_PATH)
 106 |  # If $(1)_file_name is empty and $(1)_local_dir is nonempty, set file name to a
 107 |  # .tar file with a friendly filename named after the directory path.
 108 |  $(if $($(1)_file_name),,$(if $($(1)_local_dir),$(eval $(1)_file_name:=$(call int_friendly_file_name,$($(1)_local_dir)).tar)))
 109 | +# Include the absolute source path's hash so worktrees sharing SOURCES_PATH
 110 | +# use separate archives. Source changes are still detected using mtimes.
 111 | +$(if $($(1)_local_dir),$(eval $(1)_local_dir_hash:=$(shell (cd $($(1)_local_dir) && pwd) | $(build_SHA256SUM) | cut -c1-$(HASH_LENGTH))))
 112 | +$(if $($(1)_local_dir_hash),$(eval $(1)_file_name:=$(basename $($(1)_file_name))-$($(1)_local_dir_hash).tar))

ryanofsky commented at 12:05 AM on September 30, 2026:

In commit "depends: isolate local archives by source path" (22374481a7e977bba5aa87462d097c49fb529a6c)

Looking into this more, I think a simpler alternative could be to store local package tarballs in WORK_PATH instead of SOURCES_PATH, which I implemented in fec2762f18a5020b16db84907b5b12e5fb5f9945 (tag).

Files in SOURCES_PATH are downloaded sources identified by names and hashes fixed in the package definitions, which is what makes it safe to share the directory between checkouts. Local package tarballs are different and depend on files in the current checkout so don't actually make sense to share. If local tarballs are stored in WORK_PATH instead of SOURCES_PATH they are automatically not shared and there is no need to uniquely identify different checkouts with hashes.

Other benefits of this approach: This would also mean that building does not need to write to SOURCES_PATH when sources are downloaded beforehand, so a shared SOURCES_PATH could be mounted read-only in build containers instead of read-write. And old tarballs would be removed along with their checkouts instead of accumulating in SOURCES_PATH.

The current PR is still an improvement and looks fine to merge as is, so this is just a suggestion.

ryanofsky approved

ryanofsky commented at 12:15 AM on September 30, 2026: contributor

Code review ACK 22374481a7e977bba5aa87462d097c49fb529a6c. Since last review, just dropped the documentation change. Left a comment below suggesting an alternative approach, but this looks good as is.

depends: store local package tarballs in WORK_PATH instead of SOURCES_PATH

Local packages (packages with $(package)_local_dir set) are built from a
tarball of the local directory, which is regenerated at parse time when any
file in the directory is newer than the tarball. The tarball was 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, which shares SOURCES_PATH
with its build container and mounts every checkout at /bitcoin inside it, so
all checkouts use the same tarball path. The host `make download-*` step it
runs first also sets the mtimes of local directory files to 2000-01-01, so no
checkout ever sees files newer than a tarball another checkout created.

Store local package tarballs and their download stamps in
WORK_PATH/local-sources instead. WORK_PATH defaults to the checkout's
depends/work directory, so each checkout gets its own tarball.

A side benefit is that when sources have been downloaded beforehand, as
guix-build does, building no longer writes to SOURCES_PATH, so a shared
SOURCES_PATH can be mounted read-only in build containers.

This change was written with Claude Opus 5.5 (1M context).

01491f22b2

willcl-ark force-pushed on Oct 1, 2026

willcl-ark renamed this:
~~depends: give local package tarballs unique filenames per source path~~
depends: keep local source archives in WORK_PATH
on Oct 1, 2026

depends: keep local source archives in WORK_PATH #35765

Code Coverage & Benchmarks

Reviews

Conflicts