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

qt: bump dependencies + modernize #9337

Merged
merged 4 commits into from
Feb 21, 2022

Conversation

SpaceIm
Copy link
Contributor

@SpaceIm SpaceIm commented Feb 10, 2022

  • CMakeDeps support
  • compiler=msvc support

  • I've read the guidelines for contributing.
  • I've followed the PEP8 style guides for Python code in the recipes.
  • I've used the latest Conan client version.
  • I've tried at least one configuration locally with the
    conan-center hook activated.

@@ -1227,6 +1271,8 @@ def _create_plugin(pluginname, libname, type, requires):
self.cpp_info.components[component].exelinkflags.extend(obj_files)
self.cpp_info.components[component].sharedlinkflags.extend(obj_files)

self.cpp_info.set_property("cmake_build_modules", build_modules)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why isn't this done per-component ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In the meantime, since 1.44.1 cmake_build_modules per component works properly again: conan-io/conan#10326

Comment on lines 371 to 372
if not tools.cross_building(self, skip_x64_x86=True):
self.requires("xkbcommon/1.3.0")
self.requires("xkbcommon/1.3.1")
Copy link
Contributor Author

@SpaceIm SpaceIm Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Out of scope of this PR, but requirements() shouldn't have logic based on tools.cross_building(), but only options and host settings.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ahah:

ERROR: 
	ConanException: qt/6.2.2 package_info(): Package require 'xkbcommon' declared in components requires but not defined as a recipe requirement

@conan-center-bot

This comment has been minimized.

@@ -368,8 +368,7 @@ def requirements(self):
self.requires("libalsa/1.2.5.1")
if self.options.gui and self.settings.os in ["Linux", "FreeBSD"]:
self.requires("xorg/system")
if not tools.cross_building(self, skip_x64_x86=True):
self.requires("xkbcommon/1.3.1")
self.requires("xkbcommon/1.3.1")
Copy link
Contributor

@ericLemanissier ericLemanissier Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this will break all cross compilation scenarii (involving Linux/FreeBSD and GUI), because xkbcommon is built with meson, which conan is not yet able to configure for cross-compilation

Copy link
Contributor Author

@SpaceIm SpaceIm Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it's already broken since xkbcommon is missing, while it's required. It doesn't make sense to have logic based on tools.cross_building in requirements().
Moreover, xorg recipe can't be cross-compiled either, so you can consider that cross-build to Linux is broken anyway.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's been a long time since I tested it, but it worked at least partially. Requirements can depend on settings, so I don't get why tools.cross_building would be forbidden in requirements(self)

Copy link
Contributor Author

@SpaceIm SpaceIm Feb 10, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because requirements are host dependencies, they don't care of build machine, what would it mean to not require a dependency if cross-building, then require it at consume time if someone use the package on its machine?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Well, you surely don't end up with the same package if you cross-compile and if you compile natively, but it's better than having no possibility to cross compile at all. For qt6, It's fine because cross-compilation is already broken, but for qt5 if you want to go down this road, then we need to add a with_xkbcommon option, and force this option to false when cross compiling

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think you have reached the agreement, as Eric approved it, so I am comfortable approving it as well (I am not very much in Qt nowadays, but from the generic CCI perspective it looks fine)

@conan-center-bot

This comment has been minimized.

@SpaceIm SpaceIm closed this Feb 10, 2022
@SpaceIm SpaceIm reopened this Feb 10, 2022
@conan-center-bot

This comment has been minimized.

@conan-center-bot

This comment has been minimized.

@SpaceIm SpaceIm closed this Feb 12, 2022
@SpaceIm SpaceIm reopened this Feb 12, 2022
@conan-center-bot
Copy link
Collaborator

All green in build 5 (85056dee8ead8f537a071986c02696ebb54e9e2f):

  • qt/5.15.2@:
    All packages built successfully! (All logs)

  • qt/6.1.3@:
    All packages built successfully! (All logs)

  • qt/6.0.4@:
    All packages built successfully! (All logs)

  • qt/6.2.1@:
    All packages built successfully! (All logs)

  • qt/6.2.2@:
    All packages built successfully! (All logs)

  • qt/6.2.0@:
    All packages built successfully! (All logs)

  • qt/6.2.3@:
    All packages built successfully! (All logs)

@conan-center-bot conan-center-bot merged commit bd12ff6 into conan-io:master Feb 21, 2022
@SpaceIm SpaceIm deleted the qt-modernize branch February 21, 2022 16:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

6 participants