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 5414
Build 8890: Bitcoin ABC Buildbot (legacy)
Build 8889: 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

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 ↗(On Diff #8037)

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

82 ↗(On Diff #8037)

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.