Page MenuHomePhabricator

[cmake] Refactor native build cmake generation
ClosedPublic

Authored by deadalnix on Jan 19 2020, 22:08.

Details

Summary

This adds a native_add_cmake_flags function for tools to add flags they want to pass to the native build. The flags are collected during the cmake execution process and _gen_native_cmake_target is run at the very end so that all the flags are used by watching when CMAKE_CURRENT_LIST_DIR is set to empty.

Test Plan

Run cmake build from scratch.
Delete the native folder. Check that it is regenrated when rebuilding.
Run the build a second time to ensure that dependecies for native target are tracked properly.

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cmakenativecustomargs
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 9075
Build 16112: Default Diff Build & Tests
Build 16111: arc lint + arc unit

Event Timeline

Fabien requested changes to this revision.Jan 20 2020, 11:39
Fabien added a subscriber: Fabien.
Fabien added inline comments.
cmake/modules/NativeExecutable.cmake
14 ↗(On Diff #15677)

The if is not necessary. Only INTERNAL cache variables are overridden unless FORCE is specified.

26 ↗(On Diff #15677)

Nit: symlink to because => symlink because

27 ↗(On Diff #15677)

Nit: ecutble => executable

34 ↗(On Diff #15677)

You need to add ${ARGN} to the dependencies, otherwise the native target won't rebuild if there source files are changed.

76 ↗(On Diff #15677)

Can you point to some documentation regarding this behavior ? I couldn't find anything.

cmake/templates/NativeCmakeRunner.cmake.in
3 ↗(On Diff #15677)

The bin dir is not used.

This revision now requires changes to proceed.Jan 20 2020, 11:39
cmake/modules/NativeExecutable.cmake
14 ↗(On Diff #15677)

Obviously...

34 ↗(On Diff #15677)

Good catch. This isn't exact, but probably good enough.

76 ↗(On Diff #15677)

From https://cmake.org/cmake/help/v3.10/variable/CMAKE_CURRENT_LIST_DIR.html

When CMake finishes processing commands from the file it restores the previous value.

Which ends up being nothing for the last one. Let's be clear, this is an horrible hack, but it doesn't looks like there is a better way to do this.

Fabien added inline comments.
cmake/modules/NativeExecutable.cmake
14 ↗(On Diff #15677)

Yes it's weird...

76 ↗(On Diff #15677)

Indeed, that's hacky !

But I can't this at any good solution: PRE-BUILD custom command attached to target will only work with VS, and calling the generation at the end of the top level file will break secp256k1 build...

cmake/templates/NativeCmakeRunner.cmake.in
3 ↗(On Diff #15677)

The bin dir is still unused.

This revision is now accepted and ready to land.Jan 20 2020, 15:07