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.

Sign up for GitHub

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

Jump to bottom

[qt6] fix compilation of qt6 with_vulkan on MacOs #6835

Merged
merged 4 commits into from
Sep 3, 2021
Merged

[qt6] fix compilation of qt6 with_vulkan on MacOs #6835

merged 4 commits into from
Sep 3, 2021

Conversation

AndreyMlashkin
Copy link
Contributor

@AndreyMlashkin AndreyMlashkin commented Aug 13, 2021 •
edited
Loading

Specify library name and version: qt/6.1.2@

relates to #6812
This is also a good place to share with all of us why you are submitting this PR (specially if it is a new addition to ConanCenter): is it a dependency of other libraries you want to package? Are you the author of the library? Thanks!

  • 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.

@AndreyMlashkin
Copy link
Contributor Author

It's not possible to force build with some combination of flags, right?
Because I need MacOs and with_vulkan to be built

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 13, 2021

please change the title to refer to qt6 recipe

@AndreyMlashkin AndreyMlashkin changed the title fix compilation with_vulkan on MacOs fix compilation of qt6 with_vulkan on MacOs Aug 13, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@SSE4 SSE4 requested a review from uilianries August 14, 2021 08:32
SpaceIm
SpaceIm reviewed Aug 16, 2021
View reviewed changes
recipes/qt/6.x.x/conanfile.py Outdated
@@ -896,6 +899,8 @@ def _create_plugin(pluginname, libname, type, requires):
self.cpp_info.components["qtNetwork"].frameworks.append("GSS")
if self.options.gui and self.options.widgets:
self.cpp_info.components["qtPrintSupport"].system_libs.append("cups")
if self.options.gui and self.options.with_vulkan:
self.cpp_info.components["qtGui"].frameworks.append("MoltenVK")
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 already the Macos branch

This was referenced Aug 16, 2021
Closed
Merged
@ghost
Copy link

ghost commented Aug 16, 2021 •
edited by ghost
Loading

I detected other pull requests that are modifying qt/6.x.x recipe:

This message is automatically generated by https://github.com/ericLemanissier/conan-center-conflicting-prs so don't hesitate to report issues/improvements there.

@SSE4 SSE4 changed the title fix compilation of qt6 with_vulkan on MacOs [qt6] fix compilation of qt6 with_vulkan on MacOs Aug 17, 2021
@conan-center-bot

This comment has been minimized.

Sign in to view
@ghost ghost mentioned this pull request Aug 17, 2021
Merged
4 tasks
SpaceIm
SpaceIm reviewed Aug 18, 2021
View reviewed changes
recipes/qt/6.x.x/conanfile.py Outdated Show resolved Hide resolved
recipes/qt/6.x.x/conanfile.py Outdated Show resolved Hide resolved
@ghost ghost mentioned this pull request Aug 18, 2021
Merged
4 tasks
@AndreyMlashkin
Copy link
Contributor Author

It's a pity, that there is no gui build and test for qt...

@conan-center-bot

This comment has been minimized.

Sign in to view
@AndreyMlashkin
Copy link
Contributor Author

Is there any blocker for this PR? Do I miss something?

@AndreyMlashkin AndreyMlashkin mentioned this pull request Aug 19, 2021
Merged
4 tasks
@github-actions github-actions bot mentioned this pull request Aug 19, 2021
Open
uilianries
uilianries previously approved these changes Aug 19, 2021
View reviewed changes
@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 19, 2021 •
edited
Loading

This modification doesn't work, I've tested it locally on my mac. CMake configuration fails (Vulkan is not found).
Please test your modifications locally, specially for fixes related to non-tested options in CI. You haven't answered to #6835 (comment)

@AndreyMlashkin
Copy link
Contributor Author

This modification doesn't work, I've tested it locally on my mac. CMake configuration fails (Vulkan is not found).
Please test your modifications locally, specially for fixes related to non-tested options in CI. You haven't answered to #6835 (comment)

That works for my Mac just fine. However, I have a local vulkan installation.

@SpaceIm
Copy link
Contributor

SpaceIm commented Aug 19, 2021

That works for my Mac just fine. However, I have a local vulkan installation.

That's the reason, Qt is using your Vulkan SDK, not moltenvk provided by CCI.

ericLemanissier
ericLemanissier suggested changes Aug 19, 2021
View reviewed changes
Copy link
Contributor

@ericLemanissier ericLemanissier left a comment

Choose a reason for hiding this comment

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

we need to first make sure it works

@AndreyMlashkin
Copy link
Contributor Author

Conan indeed using local vulkan installation

-- Found Vulkan: /usr/local/lib/libvulkan.dylib

@conan-center-bot

This comment has been minimized.

Sign in to view
@AndreyMlashkin
Copy link
Contributor Author

@SpaceIm @ericLemanissier, could you please try my branch?
I could not build because of vulkan-headers conflict:

ERROR: Conflict in moltenvk/1.1.1:
'moltenvk/1.1.1' requires 'vulkan-headers/1.2.162.0' while 'vulkan-loader/1.2.172' requires 'vulkan-headers/1.2.172'.
To fix this conflict you need to override the package 'vulkan-headers' in your root package.

If I explicitly override it, qt recipe complains that it is not used

@ericLemanissier
Copy link
Contributor

Sorry I cannot help, because I don't have a mac. also, I don't know if vulkan-loader and vulkan-headers versions have to be identical

@AndreyMlashkin
Copy link
Contributor Author

I have found a way :)
override=True solved the issue

@conan-center-bot

This comment has been minimized.

Sign in to view
@ericLemanissier

This comment has been minimized.

Sign in to view
@ghost ghost mentioned this pull request Aug 26, 2021
Merged
4 tasks
@AndreyMlashkin
Copy link
Contributor Author

Relates to
#7064
#7055

@conan-center-bot

This comment has been minimized.

Sign in to view
@conan-center-bot
Copy link
Collaborator

All green in build 10 (1b62a59f96c8d6e3a8339c4aff0627428391c8fc):

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

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

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

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

@AndreyMlashkin
Copy link
Contributor Author

This PR works for me.

@prince-chrismc
Copy link
Contributor

sneak in some bump deps if possible?

@AndreyMlashkin
Copy link
Contributor Author

sneak in some bump deps if possible?

sure, you can do a pr to this pr :)

@ghost ghost mentioned this pull request Sep 1, 2021
Merged
4 tasks
@AndreyMlashkin
Copy link
Contributor Author

Can we merge this PR and make an improvement later?
Adding of MoltenVk for Mac is needed anyway.

@SSE4 SSE4 requested a review from uilianries September 3, 2021 11:53
@conan-center-bot conan-center-bot merged commit 66efdf1 into conan-io:master Sep 3, 2021
@neobrain neobrain mentioned this pull request May 20, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@SpaceIm SpaceIm SpaceIm left review comments

@prince-chrismc prince-chrismc prince-chrismc approved these changes

@ericLemanissier ericLemanissier ericLemanissier approved these changes

@SSE4 SSE4 SSE4 approved these changes

@memsharded memsharded Awaiting requested review from memsharded

@danimtb danimtb Awaiting requested review from danimtb

@jgsogo jgsogo Awaiting requested review from jgsogo

@uilianries uilianries Awaiting requested review from uilianries

Assignees
No one assigned
Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
7 participants
@AndreyMlashkin @SpaceIm @conan-center-bot @ericLemanissier @prince-chrismc @SSE4 @uilianries