Page MenuHomePhabricator

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

Authored by Fabien on Mon, Apr 8, 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.Mon, Apr 8, 15:32
Herald added a reviewer: Restricted Project. · View Herald TranscriptMon, Apr 8, 15:32
deadalnix requested changes to this revision.Mon, Apr 8, 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.Mon, Apr 8, 20:05
Fabien requested review of this revision.Tue, Apr 9, 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.Wed, Apr 10, 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.Wed, Apr 10, 22:05
Fabien updated this revision to Diff 8037.Thu, Apr 11, 14:32

Hack the CMAKE_CURRENT_BINARY_DIR variable as suggested.

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

Add a comment and set the directory relative to the CMAKE_CURRENT_BINARY_DIR

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

Add double quotes

This revision was automatically updated to reflect the committed changes.