cmake: Revamp handling of data files for `{test,bench}

hebasto commented at 11:44 am on September 14, 2024: member

This PR leverages the approach from the https://github.com/chaincodelabs/libmultiprocess project and introduces a new target_data_sources() function, which minimizes code needed to assign a *.json or *.raw data file to the test_bitcoin or bench_bitcoin target.

cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets

This change introduces a new `target_data_sources()` function.

0c35be69cf

hebasto added the label Build system on Sep 14, 2024

hebasto added the label Tests on Sep 14, 2024

DrahtBot commented at 11:44 am on September 14, 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	fjahr

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:

#31144 (optimization: change XOR obfuscation key from std::vector<std::byte>{8} to uint64_t by l0rinc)
#30911 (build: speed up by flattening the dependency graph by theuni)

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.

hebasto commented at 11:45 am on September 14, 2024: member

Friendly ping @ryanofsky :)

in src/test/CMakeLists.txt:139 in 0c35be69cf

134+    data/sighash.json
135+    data/tx_invalid.json
136+    data/tx_valid.json
137+  RAW_FILES
138+    data/asmap.raw
139+  RAW_NAMESPACE test::data

maflcko commented at 8:25 am on September 16, 2024:

not sure this is going in the right direction. The namespace just happens to be a single value for all raw files. However, in theory, one could imagine several raw files having a different namespace each. This would then again increase the verbosity and complexity.

hebasto commented at 11:41 am on September 17, 2024:

Once such a scenario happens, we can use another call:

0target_data_sources(test_bitcoin
1  RAW_FILES
2    data/some_file_with_another_namespace.raw
3  RAW_NAMESPACE somewhere::another_namespace
4)

And I think it is better than (1) specifying data/some_file_with_another_namespace.raw to generate a header than (2) adding ${CMAKE_CURRENT_BINARY_DIR}/data/some_file_with_another_namespac.raw.h to the sources.

maflcko commented at 8:22 am on September 18, 2024:

Ok, fair enough. I just wanted to mention it. Seems fine either way.

fjahr commented at 9:42 am on September 20, 2024: contributor

tACK 0c35be69cf2a18a7a9173d0f9f88116b4417c892

I have based #28792 on this now. It works and seems like a nicer way to do what I need there than the previous approach.

Take it with a grain of salt though because I don’t think I am experienced enough with CMake to tell if there are any hidden issues with this approach or if there are even better ways to achieve this.

maflcko removed the label Tests on Sep 20, 2024

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

DrahtBot commented at 4:54 pm on September 20, 2024: contributor

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

File	commit 197aa249551ea7b2280b6e187d2ad5378687beff(master)	commit 0bc2b979eacf67662631c50e67d952450903d8b3(master and this pull)
SHA256SUMS.part	`521f76d55c6d0681...`	`82df3d90acea09eb...`
*-aarch64-linux-gnu-debug.tar.gz	`d65768ddaa01b1e2...`	`b55abed2172b7cf8...`
*-aarch64-linux-gnu.tar.gz	`7979cd76ea72491b...`	`a1ea6ffd72a57c52...`
*-arm-linux-gnueabihf-debug.tar.gz	`f10a6f5c23d9591a...`	`18b49aeb3bb57391...`
*-arm-linux-gnueabihf.tar.gz	`243389753c7ad79a...`	`eff66b351f885633...`
*-arm64-apple-darwin-unsigned.tar.gz	`4de1c5efbfb6983a...`	`2fb0cc3e11743ea0...`
*-arm64-apple-darwin-unsigned.zip	`56768c5b2e77aa44...`	`5e9fdb3271af18d5...`
*-arm64-apple-darwin.tar.gz	`45f83ddd7d034ced...`	`6b9a11453016515c...`
*-powerpc64-linux-gnu-debug.tar.gz	`dbd3fd28edfe8346...`	`3dcb12423b002833...`
*-powerpc64-linux-gnu.tar.gz	`b30ca46023d94314...`	`6e6caa5f3db63e9a...`
*-riscv64-linux-gnu-debug.tar.gz	`3496ab1a2d69f873...`	`eb08343fd8aaff84...`
*-riscv64-linux-gnu.tar.gz	`e9fd9140d0c75276...`	`4185a04618d7f424...`
*-x86_64-apple-darwin-unsigned.tar.gz	`2d80ca5c684f9c3a...`	`0747b84d884272a7...`
*-x86_64-apple-darwin-unsigned.zip	`8d9c51153100b0a9...`	`f31f6e03d65beb2d...`
*-x86_64-apple-darwin.tar.gz	`2a6c00e8b0e9e53b...`	`780ded24e6d27356...`
*-x86_64-linux-gnu-debug.tar.gz	`0d6d2b136adbc951...`	`8cf9d88b75d6268d...`
*-x86_64-linux-gnu.tar.gz	`c8522c785d8bade4...`	`1b9e51de65d24364...`
*.tar.gz	`fdfcfc0072ad02f3...`	`f46aa53f989f4832...`
guix_build.log	`c3f3df05bf999a2d...`	`8f0c9f28397fb3eb...`
guix_build.log.diff		`f922aa1b8ac7af94...`

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

DrahtBot added the label CI failed on Oct 13, 2024

DrahtBot removed the label CI failed on Oct 17, 2024

cmake: Revamp handling of data files for {test,bench}_bitcoin targets #30901

Code Coverage

Reviews

Conflicts

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

cmake: Revamp handling of data files for `{test,bench}_bitcoin` targets #30901