Details

Reviewers

Group Reviewers

Restricted Project

Commits

rSTAGING71191d04adaa: [CMAKE] Only change obj/build.h if the content has changed
rABC71191d04adaa: [CMAKE] Only change obj/build.h if the content has changed

Summary

This diff creates a temporary build.h.tmp file, then copies it to
obj/build.h only if it differs from it.
This avoid rebuilding due to the obj/build.h being touched while the
content remains unchanged.

In order to generate the intermediate file every time git HEAD is
updated, the .git/log/HEAD file is set as a dependency for the custom
command.

Test Plan

mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja util

Check the buildcmake/obj/build.h file is generated and the
buildcmake/obj/build.h.tmp is removed.

ninja util

Should return no work to do.
Amend the commit summary, then

ninja util

Check the buildcmake/obj/build.h file is generated again and the
buildcmake/obj/build.h.tmp is still not there.

Diff Detail

Repository

rABC Bitcoin ABC

Branch

cmake_build_h_copy_if_different

Lint

Lint Passed

Unit

No Test Coverage

Build Status

Buildable 7549
Build 13138: Bitcoin ABC Buildbot (legacy)
Build 13137: arc lint + arc unit

Event Timeline

Fabien created this revision.Sep 20 2019, 13:24

Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 20 2019, 13:24

Herald added a reviewer: deadalnix. · View Herald Transcript

Build Bitcoin-ABC / Diffs / Diff Testing started.

deadalnix requested changes to this revision.Sep 20 2019, 13:39

deadalnix added inline comments.

src/CMakeLists.txt
176 ↗	(On Diff #13032)	Please wrap this in a funcction. First, this file is not guaranteed to exist, second you need to actualy locate the root of the git repo. This now has backed in assumption about the structure of thing outside the scope of this module.

This revision now requires changes to proceed.Sep 20 2019, 13:39

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7523: Diff 13032.Sep 20 2019, 13:45

Don't assume git.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Enquote path to git executable.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Build Bitcoin-ABC / Diffs / Diff Testing passed.

Harbormaster completed remote builds in B7524: Diff 13033.Sep 20 2019, 14:33

Harbormaster completed remote builds in B7525: Diff 13034.Sep 20 2019, 14:44

Build Bitcoin-ABC / Diffs / Diff Testing passed.

deadalnix requested changes to this revision.Sep 20 2019, 16:09

deadalnix added inline comments.

src/CMakeLists.txt
180 ↗	(On Diff #13034)	This assume the build folder is in git's subtree, which it doesn't have to be.
184 ↗	(On Diff #13034)	Create it if it doesn't exist.
190 ↗	(On Diff #13034)	What if the file wasn't found?

This revision now requires changes to proceed.Sep 20 2019, 16:09

Address feedback.

Build Bitcoin-ABC / Diffs / Diff Testing started.

Fabien added inline comments.Sep 23 2019, 09:47

src/CMakeLists.txt
190 ↗	(On Diff #13034)	Since last diff update, the file is created when not found. 2 cases remain: Git is not found The project is not managed by git (`rev-parse` returns an error) In these cases `GIT_HEAD_LOGS_FILE` is empty and no dependency is added to the `custom_command`. The `obj/build.h` will still be created if it doesn't exist but then will not update until the project get some git management, which I think is the expected behavior (the suffix will be `-unk`).