build: Unify `-logsourcelocations` format

hebasto commented at 7:02 am on September 4, 2024: member

0$ ./build/src/bitcoind -logsourcelocations -asmap=/tmp/no_file 2>&1 | head -1
12024-09-04T07:01:03Z [init/common.cpp:149] [LogPackageVersion] Bitcoin Core version v28.99.0-b94fe85fbe2f (release build)

hebasto added the label Build system on Sep 4, 2024

DrahtBot commented at 7:02 am on September 4, 2024: contributor

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

Code Coverage

For detailed information about the code coverage, see the test coverage report.

Reviews

See the guideline for information on the review process.

Type	Reviewers
ACK	maflcko

If your review is incorrectly listed, please react with 👎 to this comment and the bot will ignore it on the next update.

Conflicts

Reviewers, this pull request conflicts with the following ones:

#30861 (build: Improve ccache performance for different build directories by hebasto)

If you consider this pull request important, please also help to review the conflicting pull requests. Ideally, start with the one that should be merged first.

build: Unify `-logsourcelocations` format 93a0c2da34

in CMakeLists.txt:469 in b94fe85fbe outdated

465@@ -466,6 +466,11 @@ configure_file(contrib/filter-lcov.py filter-lcov.py COPYONLY)
466 # Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
467 try_append_cxx_flags("-fno-extended-identifiers" TARGET core_interface SKIP_LINK)
468 
469+# Unify -logsourcelocations format.

maflcko commented at 7:11 am on September 4, 2024:

This isn’t the only reason. The other is ccache, see commit 7abac98d3e3c1bc8ad66cb5c05184b9c5cc674d5. It would be good to mention this.

Ref: https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map

hebasto commented at 7:33 am on September 4, 2024:

Sure! The comment has been extended.

hebasto force-pushed on Sep 4, 2024

maflcko commented at 7:41 am on September 4, 2024: member

According to https://reproducible-builds.org/docs/build-path/ “-ffile-prefix-map=OLD=NEW is an alias for both -fdebug-prefix-map and -fmacro-prefix-map. (available since GCC 8 and Clang 10)”. Ref https://gcc.gnu.org/onlinedocs/gcc/Overall-Options.html#index-ffile-prefix-map and https://clang.llvm.org/docs/ClangCommandLineReference.html#cmdoption-clang-ffile-prefix-map

review ACK 93a0c2da34b45aaf0aab23c5f1b66cb32df06507

maflcko added the label DrahtBot Guix build requested on Sep 4, 2024

DrahtBot commented at 6:02 pm on September 4, 2024: contributor

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

File	commit 4835bba2cb1350259c20bf0faf852187203ba336(master)	commit 3c4b2350d8d9ca64a2cf67adff405a2bb3738735(master and this pull)
SHA256SUMS.part	`78d20f646eafaa88...`	`da70eef0f8609f86...`
*-aarch64-linux-gnu-debug.tar.gz	`d02ba5e3d8648cc9...`	`97837c402cecbf92...`
*-aarch64-linux-gnu.tar.gz	`a6ebfa98ce0de3da...`	`fd7aa68c21b1eb74...`
*-arm-linux-gnueabihf-debug.tar.gz	`af186067da838139...`	`d5b51832ca9fab7a...`
*-arm-linux-gnueabihf.tar.gz	`8298e07f334b9bee...`	`0eb791e19f395c75...`
*-arm64-apple-darwin-unsigned.tar.gz	`e41563ef32423f38...`	`ea92656fd016276a...`
*-arm64-apple-darwin-unsigned.zip	`6c1b25aef48f687c...`	`98668506c7ec700d...`
*-arm64-apple-darwin.tar.gz	`32b9cc93428696dd...`	`0472599e5127b7fe...`
*-powerpc64-linux-gnu-debug.tar.gz	`850bef88a9dc7b0a...`	`b911b01e58febfab...`
*-powerpc64-linux-gnu.tar.gz	`da72ffef016769a5...`	`a64af9eb695aed3b...`
*-riscv64-linux-gnu-debug.tar.gz	`ea775c037fadd66e...`	`8111475d4b0216dc...`
*-riscv64-linux-gnu.tar.gz	`c33f8b226ece66bf...`	`300f6c8f139000a5...`
*-x86_64-apple-darwin-unsigned.tar.gz	`d83e03380adf065a...`	`4a33849d4b10704e...`
*-x86_64-apple-darwin-unsigned.zip	`5b848a74156133ab...`	`17af57016e789b35...`
*-x86_64-apple-darwin.tar.gz	`3b3d9ba23f76ba21...`	`b2a453b325554ea2...`
*-x86_64-linux-gnu-debug.tar.gz	`e973d85c26ccfcbf...`	`7ead0e47e7b94693...`
*-x86_64-linux-gnu.tar.gz	`c5b7c0d81c3b4232...`	`e3a9507135ecfdb8...`
*.tar.gz	`fe44e94c04cb7dba...`	`737d25e54f8364f3...`
guix_build.log	`741268c610dc68eb...`	`862ebb09975b1090...`
guix_build.log.diff		`5e6264e6afb391cb...`

DrahtBot removed the label DrahtBot Guix build requested on Sep 4, 2024

maflcko commented at 9:52 am on September 5, 2024: member

Upgrading my review with a logsourcelocations test on Linux (compiled and guix) binaries. I did not test macOS/Windows (compiled and guix) binaries.

However, ccache does not hit across two different build dirs, compiling the same commit.

in CMakeLists.txt:471 in 93a0c2da34

465@@ -466,6 +466,14 @@ configure_file(contrib/filter-lcov.py filter-lcov.py COPYONLY)
466 # Don't allow extended (non-ASCII) symbols in identifiers. This is easier for code review.
467 try_append_cxx_flags("-fno-extended-identifiers" TARGET core_interface SKIP_LINK)
468 
469+# Make builds location independent. This helps to:
470+#  1. Unify the -logsourcelocations format.
471+#  2. Improve ccache efficiency across different directories.

maflcko commented at 9:54 am on September 5, 2024:

Can you explain this. How would this be tested?

I tried with cmake -B ./bld-1-cmake and cmake -B ./bld-2-cmake and it did not work.

fanquake commented at 10:14 am on September 5, 2024:

I also tried testing. It doesn’t seem to work, because the -I argument passed to the compiler contains the full path/to/the/src directory, which means the compiler invocations differ, and ccache wont hit?

maflcko commented at 10:44 am on September 5, 2024:

Ah, thanks for testing. I guess this is a possible cmake regression, but I haven’t ever used different dirs with autotools, so I can’t say for sure.

Seems fine to fix here, or in a follow-up, as this pull request is an improvement already and this bug already exists in current master.

DrahtBot added the label CI failed on Sep 7, 2024

hebasto commented at 11:57 am on September 10, 2024: member

However, ccache does not hit across two different build dirs, compiling the same commit.

Fixed in #30861.

hebasto marked this as a draft on Sep 10, 2024

DrahtBot removed the label CI failed on Sep 13, 2024

build: Unify -logsourcelocations format #30811

Code Coverage

Reviews

Conflicts

Guix builds (on x86_64) [untrusted test-only build, possibly unsafe, not for production use]

build: Unify `-logsourcelocations` format #30811