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

Default qt6 #1307

Draft
wants to merge 9 commits into
base: main
Choose a base branch
from
Draft

Default qt6 #1307

wants to merge 9 commits into from

Conversation

kelson42
Copy link
Collaborator

@kelson42 kelson42 commented Dec 26, 2024

Fixes #1300

Depends on kiwix/container-images#272
Depends on kiwix/kiwix-build#791
Depends on kiwix/kiwix-build#799
Depends on #1302

# requested, see README.md for more information).
QT_SELECT += $$(QT_SELECT)
lessThan(QT_MAJOR_VERSION, 6) {
!equals(QT_SELECT, "qt5") {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Do you want a kiwix-specific env var to control the build behavior? The QT_SELECT env var is not the only way to select which Qt installation to use, so someone might be intentionally using a qt5 toolchain yet the build would fail because the QT_SELECT env var was not set. Similarly, if someone already set QT_SELECT=qt5 as part of their default shell environment, the build wouldn't fail even though maybe it should have.

A kiwix-specific var like KIWIX_QT=qt5 would prevent this.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I have no strong opinion on this point... I mostly focus on simplicity. Just to be sure I understand your scenarios:

someone might be intentionally using a qt5 toolchain yet the build would fail because the QT_SELECT env var was not set.

OK, but then he clearly ignores the README right? If he ignores the README he won't find out how to set KIWIX_QT=qt5 either.

Similarly, if someone already set QT_SELECT=qt5 as part of their default shell environment, the build wouldn't fail even though maybe it should have.

You mean, he would have set QT_SELECT=qt5 but actually compiling with Qt6 for example? OK... but that sounds pretty weird!

Copy link
Collaborator

Choose a reason for hiding this comment

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

OK, but then he clearly ignores the README

I typically assume folks ignore the README, at least to some degree. They will usually follow any initial instructions but I would not expect beginners to dig into the details of the README. Even if they read it, they don't often know the significance of what they read, or what context it applies. Just saying there are limits to what beginners will pick up on.

With KIWIX_QT=qt5, my understanding was that you wanted a default Qt6 experience, where everyone compiles with qt6 by default, and only drops back to qt5 for certain cases like compiling on older distros. If that's what you want, using a kiwix-specific env var would accomplish that.

Regarding my original comment - someone might be intentionally using a qt5 toolchain - what I mean is that they could use a different avenue of configuring qtchooser, like via a config file. They could have intentionally setup their environment to build with Qt5, but the build would fail because they configured qtchooser via config instead of QT_SELECT. This is not a huge deal, but could trip up someone who doesn't understand the intent of the environment variables. This is why I think KIWIX_QT is more clear all around. If we use QT_SELECT to determine whether a Qt5 build is allowed, we're conflating the configuration of qtchooser ("which version of Qt does the system provide") with the requirements of kiwix ("which version of Qt does the project require").

he would have set QT_SELECT=qt5 but actually compiling with Qt6

No, I mean that a Qt developer with a preexisting default of QT_SELECT=qt5 in their environment could walk into the project and accidentally compile with Qt5. If I understand what you want - compile with Qt6 by default except some limited circumstances - this scenario seems to break your requirement. Also not a huge deal, but I bring it up because it seems contrary to the desired experience.

@@ -72,7 +72,7 @@
# with:
# args: --no-sign

- uses: legoktm/gh-action-build-deb@9114a536498b65c40b932209b9833aa942bf108d # pin@ubuntu-noble
- uses: legoktm/gh-action-build-deb@ubuntu-noble

Check warning

Code scanning / CodeQL

Unpinned tag for a non-immutable Action in workflow Medium

Unpinned 3rd party Action 'Packages' step
Uses Step: build-ubuntu-noble
uses 'legoktm/gh-action-build-deb' with ref 'ubuntu-noble', not a pinned commit hash
@kelson42 kelson42 force-pushed the default-qt6 branch 2 times, most recently from aeed8a3 to f6a2991 Compare January 2, 2025 14:21
@kelson42 kelson42 marked this pull request as ready for review January 2, 2025 14:25
@kelson42 kelson42 marked this pull request as draft January 2, 2025 14:25
@kelson42 kelson42 force-pushed the default-qt6 branch 5 times, most recently from 0ceb2f7 to 6d73c87 Compare January 2, 2025 15:18
@kelson42
Copy link
Collaborator Author

kelson42 commented Jan 2, 2025

@adamlamar Somehow I keep failing to get our PPA packages using Qt6... maybe you have an idea?!

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.

Requires Qt6
2 participants