make keeps redoing some of the earlier done work.

dgoncharov commented at 7:44 pm on June 2, 2021: none

This issue is about depends subsystem only.

Once everything is built, subsequent runs of make result in make redoing some of the earlier done work.

There are 2 files which are being rebuilt. config.site and .stamp_$(final_build_id).

config.site is rebuilt because

check-packages and check-sources are both phony and create no files.
A real file (config.site) depends on check-packages and check-sources. When a file depends on a phony prerequisite, the file is rebuilt every time. In this case, config.site is rebuilt every time.

.stamp_$(final_build_id) is rebuild because

every package (expat, boost, etc) is phony. .PHONY: $(1) See funcs.mk:239.
The rule for every package $(1): | $($(1)_cached_checksum) does not create any file. See funcs.mk:240.
A real file (.stamp_$(final_build_id)) depends on packages. When a file depends on a phony prerequisite, the file is rebuilt every time. In this case, .stamp_$(final_build_id) is rebuilt every time.

What can be done about this?

There are a few options.

Regarding config.site.

Given that subsequent runs of make do not download the tarball, then what is the purpose of computing the hash of the earlier downloaded tarball and comparing against the hash stored earlier in the stamp file? If some malicious person could substitute the local copy of the tarball, then that same person could store the new hash in the stamp file.

i see, that check-sources and check-packages were introduced in commit 235b3a789d6c54b042a241ffcfaba4404f6245fa. The comment says this is for travis. Given that travis is no longer used, we should figure out if this commit can be reverted.

Another option is to mark check-packages and check-sources as not phony and introduce sentinel files. E.g.

0check-packages: $(prereqs_of_check_packages)
1    $(foreach package,$(all_packages),$(call check_or_remove_cached,$(package));)
2    touch $@
3
4check-sources: $(prereqs_of_check_sources)
5    $(foreach package,$(all_packages),$(call check_or_remove_sources,$(package));)
6    touch $@

In addition, both of these rules will need to have prerequisites. In addition, the same trouble with .stamp_$(final_build_id) has to be fixed, because config.site depends on .stamp_$(final_build_id).

Or keep the makefile as is. Apparently, this does not bother anybody.

As for .stamp_$(final_build_id), either the rule can be modified to create a sentinel file and each package can be marked to be not phony or the keep the makefile as is.

0<!--- What behavior did you expect? -->
1"Nothing to be done for 'all'." message from make.
2
3<!--- How reliably can you reproduce the issue, what are the steps to do so? -->
4Every time.
5
6<!-- What version of Bitcoin Core are you using, where did you get it (website, self-compiled, etc)? -->
7Latest soucrce from git.

maflcko added the label Build system on Jun 3, 2021

dgoncharov commented at 1:33 pm on June 10, 2021: none

Rebuild of .stamp_$(final_build_id) is the expensive one. It copies all the cached tarballs and unpacks them sequentially, one by one. Takes about 27 seconds on my box.

dgoncharov referenced this in commit ed11f790a7 on Aug 12, 2021

dgoncharov commented at 3:58 pm on August 13, 2021: none

This is an example of the how a missing fetched stamp file causes make to skip checksum verification.

0$ ls -la /home/dgoncharov/src/bitcoin/depends/sources/download-stamps/.stamp_fetched-libevent*
1ls: cannot access '/home/dgoncharov/src/bitcoin/depends/sources/download-stamps/.stamp_fetched-libevent*': No such file or directory
2$ make  NO_QT=1 NO_QR=1 NO_ZMQ=1 NO_WALLET=1 NO_BDB=1 NO_SQLITE=1 NO_UPNP=1 NO_NATPMP=1
3copying packages: native_b2 boost libevent
4to: /home/dgoncharov/src/bitcoin/depends/x86_64-pc-linux-gnu
5$

This happens because check_or_remove_sources has the following shell code

0test -f $($(package)_fetched) && ( $(build_SHA256SUM) -c $($(package)_fetched) >/dev/null 2>/dev/null || \                      
1    ( echo "Checksum missing or mismatched for $(package) source. Forcing re-download."; \                                        
2      rm -f $($(package)_all_sources) $($(1)_fetched))) || true
3

Effectively, this shell code tests presence of the fetched stamp file. If the stamp file is missing no checksum verification is performed and the sources are considered good.

dongcarl commented at 9:34 pm on February 7, 2022: contributor

@hebasto, @theuni any thoughts on this? I think it’d be worthwhile to remove such a long, unnecessary step: #22134 (comment)

hebasto commented at 6:32 pm on February 9, 2022: member

@hebasto, @theuni any thoughts on this? I think it’d be worthwhile to remove such a long, unnecessary step: #22134 (comment)

Only a few developers in this repo really could concern about efficiency of consequential runs of make -C depends. And for me it is not an issue at all. And benefits of changing makefiles are significantly less in comparison to enabling ccache in depends.

Rebuild of .stamp_$(final_build_id) is the expensive one. It copies all the cached tarballs and unpacks them sequentially, one by one.

And I appreciate such behavior, because when using consequential runs of make -C depends with different NO_<PACKAGE>=1 variables, it guaranties that prefix directory does not contain any leftovers of a previous build.

dgoncharov commented at 11:20 pm on February 9, 2022: none

Keeping the behavior intact is an option, given that this apparently does not bother users. i think, we’d still want to fix skipped checksum verification when the stamp file is absent. The comment in the commit says this is needed for travis. Cory or Hennadii, can you please clarify why this copying is still needed?

willcl-ark commented at 3:05 pm on April 10, 2024: contributor

There doesn’t seem to be much interest in implementing this feature.

Please feel free to submit a pull request if this is something that you still want.

willcl-ark closed this on Apr 10, 2024

make keeps redoing some of the earlier done work. #22134