Page MenuHomePhabricator

[CMAKE] Only change obj/build.h if the content has changed
ClosedPublic

Authored by Fabien on Sep 20 2019, 13:24.

Details

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
Lint
Automatic diff as part of commit; lint not applicable.
Unit
Automatic diff as part of commit; unit tests not applicable.

Event Timeline

Fabien created this revision.Sep 20 2019, 13:24
Herald added a reviewer: Restricted Project. · View Herald TranscriptSep 20 2019, 13:24
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
Fabien updated this revision to Diff 13033.Sep 20 2019, 14:23

Don't assume git.

Fabien updated this revision to Diff 13034.Sep 20 2019, 14:26

Enquote path to git executable.

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
Fabien updated this revision to Diff 13076.Sep 23 2019, 09:41

Address feedback.

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).

deadalnix accepted this revision.Sep 24 2019, 14:12
This revision is now accepted and ready to land.Sep 24 2019, 14:12