Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

CMake not spotting missing qtbase5-private-dev #4273

Open
zapashcanon opened this issue Mar 23, 2018 · 14 comments
Open

CMake not spotting missing qtbase5-private-dev #4273

zapashcanon opened this issue Mar 23, 2018 · 14 comments
Assignees
Labels

Comments

@zapashcanon
Copy link
Contributor

zapashcanon commented Mar 23, 2018

Hi,

When trying to build LMMS on Debian Unstable, I got:

/home/zapashcanon/Programmation/C++/lmms/src/3rdparty/qt5-x11embed/src/X11EmbedContainer.cpp:40:10: fatal error: 'QtCore/private/qobject_p.h' file not found
#include <QtCore/private/qobject_p.h>
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [src/3rdparty/qt5-x11embed/CMakeFiles/qx11embedcontainer.dir/build.make:63: src/3rdparty/qt5-x11embed/CMakeFiles/qx11embedcontainer.dir/src/X11EmbedContainer.cpp.o] Error 1

I opened an issue against qt5-x11embed. Finally, @lukas-w told me I was missing qtbase5-private-dev, that was indeed the problem, which should have been spotted by CMake in LMMS.

@lukas-w
Copy link
Member

lukas-w commented Mar 23, 2018

I remember a discussion on Discord about this issue. We found that the problem is FIND_PACKAGE(Qt5 …) being called twice results in the second call being ineffective because of the way Qt's find modules work internally. One possible solution to this would be to use ExternalProject_Add instead of add_subdirectory to build qt5-x11embed.

@tresf
Copy link
Member

tresf commented Mar 23, 2018

Here's the archive of the conversation that @lukas-w is talking about:

tresf - 02/23/2018

@lukas-w is there a chance it's a regression caused by changing our syntax? https://github.com/LMMS/lmms/blob/master/CMakeLists.txt#L139
The submodule uses

find_package(Qt5Core COMPONENTS Private REQUIRED)
#               ^

whereas master uses

FIND_PACKAGE(Qt5 COMPONENTS Core Gui Widgets Xml REQUIRED)
#                           ^

This newer technique is needed for better Qt5 detection with the non-Linux platforms per https://blog.kitware.com/cmake-finding-qt5-the-right-way/

Lukas-W - 02/23/2018

@tresf I don't think so, find_package(Qt5 COMPONENTS Core) just calls find_package(Qt5Core) internally
@tresf The contents of Qt5CoreConfig.cmake are wrapped in a if (NOT TARGET Qt5::Core) condition, so it looks like my initial assumption is right

tresf - 02/23/2018

@lukas-w can we hack in a unset (<some_varialble> CACHE) to workaround the issue?

Lukas-W - 02/23/2018

@tresf I don't think so. There's no way to unset a target in CMake.
The "canonical" way of doing it would be to use ExternalProject_Add instead of add_subdirectory

tresf - 02/23/2018

@lukas-w Could we detect the private components are needed upfront and hack it in there?
I realize it would take it out of scope of the submodule, but it might help the users trying to build it.
@lukas-w I was able to compile qt5-x11embed fine with the CMAKE_PREFIX_PATH set to /opt/qt59 and without qtbase5-private-dev installed.
Qt59 standalone ships with the private headers, but from what I'm reading we shouldn't necessarily expose them to the entire project.
@lukas-w On my system, Ubuntu 14.04 qt-x11embed, it completely ignores CMAKE_PREFIX_PATH for some reason. I'm going to do a fresh clone from lmms/lmms/master and see if the syntax is an influencing factor. (Edit: It's not)
My theory is that this is why the additional Ubuntu package qtbase5-private-dev is incorrectly needed on master even when building with a custom Qt5 version, such as from /opt/qt59 (Edit: I did confirm it shouldn't be needed if the above bug were fixed)

@tresf
Copy link
Member

tresf commented Mar 23, 2018

At the end, my proposal was to hack in a way to pick up the variables introduced by the parent project and not call FIND_PACKAGE(Qt5) unless it wasn't already found. (e.g. anticipate the private-dev on the LMMS-side) @lukas-w can we hack something like that in or do you feel we need to switch it over to ExternalProject_Add ?

@lukas-w
Copy link
Member

lukas-w commented Mar 25, 2018

@tresf We can certainly anticipate the need for private components LMMS-side. There shouldn't be a need to change anything within qt5-x11embed because the second call to FIND_PACKAGE(Qt5) will be ineffective.

@lukas-w lukas-w added the bug label Mar 25, 2018
@PhysSong
Copy link
Member

PhysSong commented Oct 5, 2018

Can we work around this bug like this?

IF(LMMS_BUILD_LINUX)
	FIND_PACKAGE(Qt5Core COMPONENTS Private REQUIRED)
	FIND_PACKAGE(Qt5 COMPONENTS X11Extras REQUIRED)
	LIST(APPEND QT_LIBRARIES Qt5::X11Extras)
ELSE()
	FIND_PACKAGE(Qt5 COMPONENTS Core REQUIRED)
ENDIF()

FIND_PACKAGE(Qt5 COMPONENTS Gui Widgets Xml REQUIRED)

We should find the Core component before any others because they depend on Qt5::Core.

@PhysSong
Copy link
Member

PhysSong commented Oct 5, 2018

One possible solution to this would be to use ExternalProject_Add instead of add_subdirectory to build qt5-x11embed.

I also think we should eventually do that. If we do so, how about other submodules? Should we switch them to ExternalProject_Add as well?

@tresf
Copy link
Member

tresf commented Oct 5, 2018

@PhysSong both proposals make sense to me. The first one can probably be commented with a # TODO to be removed once the project has been switched to use ExternalProject_Add for all necessary submodules.

ExternalProject_Add may not be a good solution to all submodules though especially ones that don't have native CMake support like several of our plugins. Examples include OpulenZ, Xpressive, FreeBoy, LadspaEffect/tap, LadspaEffect/swh, LadspaEffect/calf. Most of the submodule candidates required some custom CMake logic on our side for compilation.

@PhysSong
Copy link
Member

PhysSong commented Oct 5, 2018

That's right. However, I think rpmalloc can also be a candidate.

@lukas-w lukas-w self-assigned this Mar 9, 2019
@claell
Copy link
Contributor

claell commented Mar 13, 2019

I just ran into this. Since this seems to be also caused by a missing package, should the corresponding Wiki pages with dependencies also updated?

@tresf
Copy link
Member

tresf commented Mar 13, 2019

If the environment your building for is missing the instructions please feel free to edit the wiki to fix them or feel free to propose the change here.

For example, in Ubuntu, it's in the VST section (not sure if that's the right place).

@claell
Copy link
Contributor

claell commented Mar 13, 2019

I used Ubuntu, but did not install the things from the VST section as they seemed optional to me. So shell I change the section for that package?

@tresf
Copy link
Member

tresf commented Mar 13, 2019

So shell I change the section for that package?

Sure, thanks. 🐚 Go ahead and bump it up to the main dependencies block if that's what you believe is required.

Note, Qt5-downstream (from Ubuntu/Debian) repos don't offer the private headers by default and require the additional package however Qt5-upstream from Qt's website does, but this message still occurs when using Qt5-upstream due to the discussion above. Because of this, this building issue needs to remain open until we permanently fix this. A quick explanation is here above: #4273 (comment)

@claell
Copy link
Contributor

claell commented Mar 13, 2019

Alright, done for Ubuntu.

@rdrpenguin04
Copy link
Contributor

Still an issue it seems, but, since this thread exists, I at least knew which package to install. Thank you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

6 participants