Separate version info into bitcoin-version.h from bitcoin-config.h
ClosedPublic
Actions

Authored by Fabien on May 16 2019, 14:17.

Details

Reviewers

deadalnix

Group Reviewers

Restricted Project

Commits

rSTAGING181dea75e1ea: Separate version info into bitcoin-version.h from bitcoin-config.h
rABC181dea75e1ea: Separate version info into bitcoin-version.h from bitcoin-config.h

Summary

This would avoid rebuilding the whole source tree at each version
change.

Test Plan

./autogen.sh
mkdir build && cd build

../configure
make check

rm -rf *
CONFIG_SITE=../depends/i686-w64-mingw32/share/config.site ../configure \
--with-seeder=false
make

rm -rf *
CONFIG_SITE=../depends/x86_64-w64-mingw32/share/config.site \
../configure --with-seeder=false
make

cd .. && mkdir buildcmake && cd buildcmake

cmake -GNinja ..
ninja check

rm -rf *
cmake -GNinja .. -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win32.cmake \
-DBUILD_BITCOIN_SEEDER=OFF
ninja

rm -rf *
cmake -GNinja .. -DCMAKE_TOOLCHAIN_FILE=../cmake/platforms/Win64.cmake \
-DBUILD_BITCOIN_SEEDER=OFF
ninja

Diff Detail

Repository

rABC Bitcoin ABC

Lint

Lint Not Applicable

Unit

Tests Not Applicable

Event Timeline

Fabien created this revision.May 16 2019, 14:17

Herald added a reviewer: Restricted Project. · View Herald TranscriptMay 16 2019, 14:17

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

Harbormaster completed remote builds in B5871: Diff 8696.May 16 2019, 14:33

config contains allt he confgure generated files, so config/version.h would make much more sense.

src/CMakeLists.txt
22 ↗	(On Diff #8696)	Unless this is needed anywhere in here, I think it can stay in config.
src/bitcoin-cli-res.rc
2 ↗	(On Diff #8696)	Considering this patterns repeats itself a bazillion times, it may make sense to either include this from version.h or to also move the copyright infos in version.h .
src/bitcoin-version.h.in
12 ↗	(On Diff #8696)	There should be one template for cmake and one for configure. These define should be removed from the other configure template.

This revision now requires changes to proceed.May 16 2019, 14:56

Will move to src/config/ and include copyright defines in it.

src/bitcoin-version.h.in
12 ↗	(On Diff #8696)	The defines are removed from the CMake template. The autotools one is generated by `autoheader`.

Move to src/config/bitcoin-version.h, separate templates, keep resource files unchanged.

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing started.

Harbormaster completed remote builds in B5878: Diff 8708.May 17 2019, 09:58

Build Bitcoin-ABC / Diffs / Bitcoin-ABC Diff Testing passed.

deadalnix requested changes to this revision.May 17 2019, 11:11

deadalnix added inline comments.

configure.ac
1200 ↗	(On Diff #8708)	Just version
src/config/bitcoin-version.h.cmake.in
5 ↗	(On Diff #8708)	The guard doesn't match naming convention.
src/config/bitcoin-version.h.in
5 ↗	(On Diff #8708)	dito
12 ↗	(On Diff #8708)	Presumably, this should be removed from somewhere and moved there.

This revision now requires changes to proceed.May 17 2019, 11:11

Fabien added inline comments.May 17 2019, 11:25

src/config/bitcoin-version.h.cmake.in
5 ↗	(On Diff #8708)	Good catch
src/config/bitcoin-version.h.in
12 ↗	(On Diff #8708)	This is removed from `configure.ac`. The `bitcoin-config.h.in` template is generated by `autoheader`, there is nothing else to remove.