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
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.Apr 8 2019, 15:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptApr 8 2019, 15:32
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
Fabien updated this revision to Diff 8037.Apr 11 2019, 14:32

Hack the CMAKE_CURRENT_BINARY_DIR variable as suggested.

Fabien edited the summary of this revision. (Show Details)Apr 11 2019, 14:33
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
Fabien updated this revision to Diff 8055.Apr 15 2019, 07:20

Add a comment and set the directory relative to the CMAKE_CURRENT_BINARY_DIR

deadalnix accepted this revision.Apr 15 2019, 11:53
This revision is now accepted and ready to land.Apr 15 2019, 11:53
deadalnix added inline comments.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.

Fabien updated this revision to Diff 8057.Apr 15 2019, 13:44

Add double quotes

This revision was automatically updated to reflect the committed changes.