Page MenuHomePhabricator

[CMAKE] Move .h files transformed from .ui to the form subdirectory
ClosedPublic

Authored by Fabien on Apr 8 2019, 15:32.

Details

Summary

This diff is an alternative to D2750.

This is a prerequisite for PR11651, as all the qt headers will be
included with their path relative to src/.

Example: #include "ui_intro.h will become `#include
<qt/forms/ui_intro.h>`.

The ui_*.h files generated from the *.ui files are generated by
qt5_wrap_ui in the CMakeLists.txt file directory, wherever the
source *.ui files are located.

This diff updates then restores the CMAKE_CURRENT_BINARY_DIR in order
to make CMake behave as if the CMakeLists.txt file was located into
the qt/forms directory.

Test Plan
mkdir buildcmake && cd buildcmake
cmake -GNinja ..
ninja check

Diff Detail

Repository
rABC Bitcoin ABC
Branch
cmake_move_ui_files_forms_rewrite_qtwrapui
Lint
Lint Passed
Unit
No Test Coverage
Build Status
Buildable 5438
Build 8938: Bitcoin ABC Buildbot (legacy)
Build 8937: arc lint + arc unit

Event Timeline

deadalnix requested changes to this revision.Apr 8 2019, 20:05

You will end up using the ui generator from the system even when not wanted (for instance for reproducible builds) with that construct.

This revision now requires changes to proceed.Apr 8 2019, 20:05
Fabien requested review of this revision.Apr 9 2019, 18:41

The Qt5Widgets_UIC_EXECUTABLE variable is set by the find_package(Qt5Widget) which is searching for files according to the CMAKE_FIND_ROOT_PATH.
The platform files override the CMAKE_FIND_ROOT_PATH variable, so that cross compilation and gitian builds should be fine.

deadalnix requested changes to this revision.Apr 10 2019, 22:05
deadalnix added inline comments.
cmake/modules/QtHelper.cmake
37 ↗(On Diff #7993)

A simpler option would be to set CMAKE_CURRENT_BINARY_DIR and call QT5_WRAP_UI instead of duplicating its source code.

This revision now requires changes to proceed.Apr 10 2019, 22:05

Hack the CMAKE_CURRENT_BINARY_DIR variable as suggested.

deadalnix requested changes to this revision.Apr 14 2019, 14:25
deadalnix added inline comments.
src/qt/CMakeLists.txt
81

A comment as to why this is necessary would be useful, IMO

82

Defining this in term of the current binary dir is the correct thing to do here.

This revision now requires changes to proceed.Apr 14 2019, 14:25

Add a comment and set the directory relative to the CMAKE_CURRENT_BINARY_DIR

This revision is now accepted and ready to land.Apr 15 2019, 11:53
src/qt/CMakeLists.txt
87 ↗(On Diff #8055)

I think you need quotes in there in case there is a space in the path.

This revision was automatically updated to reflect the committed changes.