build: scripted-diff: drop config/ subdir for bitcoin-config.h

theStack commented at 11:13 am on September 20, 2024: contributor

This PR is a follow-up to #30856, as suggested in comment #30856 (comment). With the scripted diff, review should be fairly trivial, but it could still be seen as controversial due to the large number of files (78 in total) being touched.

DrahtBot commented at 11:13 am on September 20, 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	hebasto
Concept ACK	theuni

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:

#30860 (test: autogenerate bash completion by BrandonOdiwuor)

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.

DrahtBot added the label Refactoring on Sep 20, 2024

theStack renamed this:
~~scripted-diff: drop config/ subdir for bitcoin-config.h~~
build: scripted-diff: drop config/ subdir for bitcoin-config.h
on Sep 20, 2024

fanquake removed the label Refactoring on Sep 20, 2024

DrahtBot added the label Build system on Sep 20, 2024

hebasto commented at 9:16 am on September 23, 2024: member

Concept ACK, given the early stage of the current release cycle and the presence of only a single conflicting PR.

fanquake requested review from theuni on Sep 23, 2024

fanquake added the label DrahtBot Guix build requested on Sep 23, 2024

DrahtBot commented at 5:37 pm on September 23, 2024: contributor

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

File	commit 33adc7521cc8bb24b941d959022b084002ba7c60(master)	commit 6fbeea9f8b3cdad6d6def1eeb5a884e6446a207e(master and this pull)
SHA256SUMS.part	`89575c9e7a506f16...`	`7d9eb64ae40835ea...`
*-aarch64-linux-gnu-debug.tar.gz	`83ab33a38bceb4ee...`	`41bc30119e8bc20b...`
*-aarch64-linux-gnu.tar.gz	`a232fbbe572e68ac...`	`614efa1d09263812...`
*-arm-linux-gnueabihf-debug.tar.gz	`a84d1b299dbf3220...`	`e4507ed5811fcdca...`
*-arm-linux-gnueabihf.tar.gz	`652caee91e88f2d5...`	`9a31cde0c8b9e7d5...`
*-arm64-apple-darwin-unsigned.tar.gz	`27cc00b2fdbc1374...`	`5e705ebce9c06c56...`
*-arm64-apple-darwin-unsigned.zip	`eddb4c85626a1831...`	`840d26cd3693c7f4...`
*-arm64-apple-darwin.tar.gz	`715c86a8f85d1d53...`	`4d64b0740d685f52...`
*-powerpc64-linux-gnu-debug.tar.gz	`55377f5b1dc78cf2...`	`0f9a98213352e252...`
*-powerpc64-linux-gnu.tar.gz	`120675f298ddd59f...`	`57ad0789c19df2be...`
*-riscv64-linux-gnu-debug.tar.gz	`57a64f32ea69220f...`	`4b40f2c8a64eb511...`
*-riscv64-linux-gnu.tar.gz	`a3a52fd585611a4d...`	`6dd7f9f8ef783a46...`
*-x86_64-apple-darwin-unsigned.tar.gz	`4d86d57f70cd9645...`	`1da632e8e3f95017...`
*-x86_64-apple-darwin-unsigned.zip	`d71ea11af306a495...`	`1fd0de4ac63ce56f...`
*-x86_64-apple-darwin.tar.gz	`e8792a381d01b322...`	`805a46c5e705a00f...`
*-x86_64-linux-gnu-debug.tar.gz	`e3f5849a37744ea5...`	`6be83655793a7da6...`
*-x86_64-linux-gnu.tar.gz	`8a648bbea3d3cf2a...`	`23e1ce0b57def6da...`
*.tar.gz	`30492a8d47e39455...`	`9d87da8057c0911a...`
guix_build.log	`a1cc625d9bfe4c01...`	`bc87f4ce5784f4d4...`
guix_build.log.diff		`9f924f275b69d8bb...`

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

theuni commented at 8:27 pm on September 23, 2024: member

Concept ACK, and scripted-diff lgtm ACK.

Bikeshed: How about bitcoin-build-config.h for symmetry with bitcoin-build-info.h?

scripted-diff: drop config/ subdir for bitcoin-config.h, rename to bitcoin-build-config.h

Follow-up for PR #30856, commit 0dd66251.

-BEGIN VERIFY SCRIPT-
sed -i "s|config/bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l config/bitcoin-config\.h)
sed -i "s|bitcoin-config\.h|bitcoin-build-config.h|g" $(git grep -l "bitcoin-config\.h" ./src ./test ./cmake)
git mv ./cmake/bitcoin-config.h.in ./cmake/bitcoin-build-config.h.in
-END VERIFY SCRIPT-

09b0161c4a

theStack force-pushed on Sep 23, 2024

theStack commented at 10:52 pm on September 23, 2024: contributor

Bikeshed: How about bitcoin-build-config.h for symmetry with bitcoin-build-info.h?

Sounds good to me, done. Both the scripted diff and the patch are now a bit larger, as also some comments/strings have to be adapted, and the .h.in file needs to be renamed as well, but review should still be simple.

in test/lint/test_runner/src/main.rs:432 in 09b0161c4a

428@@ -429,7 +429,7 @@ fn lint_includes_build_config() -> LintResult {
429                 ])
430                 .args(get_pathspecs_exclude_subtrees())
431                 .args([
432-                    // These are exceptions which don't use bitcoin-config.h, rather the Makefile.am adds
433+                    // These are exceptions which don't use bitcoin-build-config.h, rather the Makefile.am adds

hebasto commented at 10:26 am on September 24, 2024:

Not directly related to this PR change, but mentioning “Makefile.am” is outdated.

hebasto approved

hebasto commented at 10:27 am on September 24, 2024: member

ACK 09b0161c4a2502564f50fa2b3ce9b19d8fc40d8b.

DrahtBot requested review from theuni on Sep 24, 2024

build: scripted-diff: drop config/ subdir for bitcoin-config.h #30937

Code Coverage

Reviews

Conflicts

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