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

allow dual build of qt5 and qt6 #147

Merged
merged 17 commits into from Apr 2, 2024
Merged

Conversation

carlosdem
Copy link
Contributor

this work is based on apol's pull request #143. it tries to support both qt5 and qt6 builds through some meson trickery inspired by the latest libappstream release.

@carlosdem carlosdem mentioned this pull request Mar 19, 2024
@Conan-Kudo
Copy link
Contributor

Conan-Kudo commented Mar 19, 2024

This is a bad idea and in general probably unnecessary, since the only consumer is Plasma Discover (which is now Qt6 only).

@robert-ancell
Copy link
Contributor

The value in this is we can ship these updated versions to older releases which use the old version of Qt5 version of Plasma discover. If we drop Qt5 support completely then we'd have to start a branch.

@Conan-Kudo
Copy link
Contributor

Conan-Kudo commented Mar 19, 2024

But do we need to be able to build both at once? And do they need to be parallel installable? I don't think so.

@robert-ancell
Copy link
Contributor

Right, I don't think there's any need to be parallel installed.

@soumyaDghosh
Copy link

Hi, @carlosdem Can some ETA be given on this, considering, we'll be using this library to upgrade the discover backend. Please let me know, if I can help somehow.

@Conan-Kudo
Copy link
Contributor

This makes a lot of unnecessary changes, including changing the names of the pkgconfig and cmake files, which breaks building things.

@soumyaDghosh
Copy link

This makes a lot of unnecessary changes, including changing the names of the pkgconfig and cmake files, which breaks building things.

I was going to work on the Snap backend of discover in this upcoming GSOC under the mentorship of @ScarlettGatelyMoore, so, if possible, can we also add this migration under that umbrella? We can pick up things where it's currently now. Considering this PR almost builds perfectly, because I did built it from source today. Can a plan be made to add this under GSOC, so, that we can work. All of our work will depend on this. So, this port is really needed.

@carlosdem
Copy link
Contributor Author

I'll finish it tomorrow (AEST time) and make it single build qt5 or qt6 only as requested.

@soumyaDghosh
Copy link

I'll finish it tomorrow (AEST time) and make it single build qt5 or qt6 only as requested.

Thanks a lot!

make qt bindings default to qt6 and only build one qt major version at a time
@carlosdem
Copy link
Contributor Author

I'll finish it tomorrow (AEST time) and make it single build qt5 or qt6 only as requested.

Thanks a lot!

@soumyaDghosh sorry life got in the way a bit. new version is up.

@robert-ancell
Copy link
Contributor

Thanks @carlosdem! I've updated the CI so it now builds on the older releases.

@robert-ancell robert-ancell merged commit 5e8db3b into snapcore:master Apr 2, 2024
5 checks passed
@robert-ancell robert-ancell mentioned this pull request Apr 2, 2024
@carlosdem carlosdem deleted the dual_build branch April 2, 2024 08:53
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.

None yet

5 participants