src/bench/CMakeLists.txt
22 ↗	(On Diff #9523)	It's better to add a custom target and set the dependencies on it so you don't run the command every time.
src/bench/data/convert-raw-to-header.sh
10 ↗	(On Diff #9523)	You may want to look into `src/test/data/generate_header.py` and how it is used.

This revision now requires changes to proceed.Jun 23 2019, 00:08

src/bench/CMakeLists.txt
22 ↗	(On Diff #9523)	I think a custom command is a better fit here, as there is no point in building the raw.h file apart from the whole bench executable (the same is used for `generate_header.py`, for the same reason I guess). The command won't run every time, only when there is a change on the dependencies (I'll make sure to improve this) or when the output is required by another target, which is how the command gets run in this case as the .h files are dependencies for the `bitcoin-bench` target.
src/bench/data/convert-raw-to-header.sh
10 ↗	(On Diff #9523)	Good idea ! Using a redirection will make the script much simpler.

Improve dependencies, use file redirection to make the script simpler.

deadalnix requested changes to this revision.Jun 25 2019, 15:33

deadalnix added inline comments.

src/bench/data/convert-raw-to-header.sh
6 ↗	(On Diff #9647)	There are no dependency on hexdump. I don't understand why the approach taken by `src/test/data/generate_header.py` is not preferable.

This revision now requires changes to proceed.Jun 25 2019, 15:33

Fabien planned changes to this revision.Jun 25 2019, 15:50

Fabien added inline comments.

src/bench/data/convert-raw-to-header.sh
6 ↗	(On Diff #9647)	The dependency exists as long as autotools is still supported, the script is just the direct translation of the Makefile.bench.include scipt (only special autotools variables names are adapted). That said, I don't think the `generate_headers.py` solution (translation of the bash script to a python script) is worst, it's even a better long-term solution as it effectively removes the hexdump dependency when autotools support gets dropped. I will do the translation.