-
Notifications
You must be signed in to change notification settings - Fork 2k
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
dev-qt: add version 6.3.1 #25635
dev-qt: add version 6.3.1 #25635
Conversation
Pull Request assignmentSubmitter: @Chiitoo dev-qt/qt5compat: @gentoo/proxy-maint (new package) Linked bugsNo bugs to link found. If your pull request references any of the Gentoo bug reports, please add appropriate GLEP 66 tags to the commit message and request reassignment. If you do not receive any reply to this pull request, please open or link a bug to attract the attention of maintainers. In order to force reassignment and/or bug reference scan, please append Docs: Code of Conduct ● Copyright policy (expl.) ● Devmanual ● GitHub PRs ● Proxy-maint guide |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Nice! Not tested yet. Some minor comments.
Also, we should tag the Qt 6 bump bug.
-DQT_FEATURE_qtwebengine_build=on | ||
-DQT_FEATURE_qtwebengine_quick_build=on | ||
-DQT_FEATURE_qtwebengine_widgets_build=on | ||
# -DQT_FEATURE_ssl=off |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why are all of these commented out?
Also, nit, but I usually prefer #-DFOO, rather than # -DFOO. But not a big deal.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
If I remember right, and maybe guess a little, I pulled all the 'qt_feature' strings out with grep, then added them there to try enabling and disabling things. Those that still are commented out probably need more work, or they couldn't be flipped for some other reason.
Hashtags justified to the right now.
dev-qt/qtbase/qtbase-6.3.0.ebuild
Outdated
QTSQL_IUSE="freetds mysql oci8 odbc postgres +sqlite" | ||
IUSE+=" ${QTGUI_IUSE} ${QTNETWORK_IUSE} ${QTSQL_IUSE} cups gtk icu systemd +udev" | ||
# QtPrintSupport = QtGui + QtWidgets enabled. | ||
# ibus = xkbcommon + dbus, and xkbcommon needs either libinput or X |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I don't see ibus mentioned anywhere else, is it just enabled automatically when these conditions are met?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks like it, pretty much:
https://code.qt.io/cgit/qt/qtbase.git/tree/src/plugins/platforminputcontexts/CMakeLists.txt?h=6.3.0
dev-qt/qtbase/qtbase-6.3.0.ebuild
Outdated
QTGUI_IUSE="accessibility egl eglfs evdev +gif gles2-only +ico +jpeg +libinput tslib tuio vulkan +X" | ||
QTNETWORK_IUSE="gssapi libproxy sctp +ssl vnc" | ||
QTSQL_IUSE="freetds mysql oci8 odbc postgres +sqlite" | ||
IUSE+=" ${QTGUI_IUSE} ${QTNETWORK_IUSE} ${QTSQL_IUSE} cups gtk icu systemd +udev" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I find this "incremental building" of IUSE (and REQUIRED_USE below) fairly hard to read/maintain. The separate variables QT*_IUSE are fine, but maybe declare IUSE and REQUIRED_USE just once.
And for consistency, why not moving gtk
to a QTWIDGETS_IUSE? and icu systemd
to QTCORE_IUSE and similar for the rest. This should alleviate my previous concern on readability as well.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Started this but similar to the Qt Tools split, this will take a bit more time so I'll try to at least get the other things done first.
dev-qt/qtbase/qtbase-6.3.0.ebuild
Outdated
gtk? ( widgets ) | ||
gui? ( || ( eglfs X ) || ( libinput X ) ) | ||
libinput? ( udev ) | ||
sql? ( || ( freetds mysql oci8 odbc postgres sqlite ) ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
reuse QTSQL_IUSE here?
dev-qt/qttools/qttools-6.3.0.ebuild
Outdated
|
||
inherit qt6-build | ||
|
||
DESCRIPTION="Qt Tools" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
TBH I'm not super happy about a monolithic qttools ebuild... most tools in this repo used to be independent and unrelated to each other. Has that changed? how difficult would it be to split them?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah, it was a very quickly put together (like most of the ones I made) just to fill the dependency up.
Looking at it a bit, it should probably be quite doable to split this one. I'll look into it, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On further thinking and looking, I'm not sure how to handle file collisions and preserve previously installed tools.
I'm not sure how often I can work on these things now that I started a job training thingy, and I already was sleepy most of the time before that...
Definitely should be able to add USE flags for the separate tools at the very least.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
USE-flags added.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to handle file collisions and preserve previously installed tools
not following, file collisions with what? Qt5? why would splitting the package change anything in this regard?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
but yeah, USE flags are good enough as a starting point, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm not sure how to handle file collisions and preserve previously installed tools
not following, file collisions with what? Qt5? why would splitting the package change anything in this regard?
Perhaps I'm missing something obvious, but let's say we install only assistant, and none of the other tools, setting them OFF, and then install some other tool(s) after that. I'm thinking we'd need to have a way to retain that assistant install and not install some of the duplicate files that would be installed with all the tools, similar to how we do with Qt5. I can't quite think of how to do that with CMake.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
A typical workaround for that situation is to create a qttools-common
package or similar that installs all common files. We did not have this problem with Qt5's tools though, there were no common files IIRC.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, that did come to mind. Will look into it as/when time allows.
fi | ||
|
||
DEPEND=" | ||
=dev-qt/qtbase-${PV}*[gui,network,opengl,sql,test,widgets] |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
a USE dependency on [test]
seems very fishy to me
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I guess it might refer to how configure fails if qtbase wasn't built with 'QT_FEATURE_testlib=ON'.
Will remove it here, and keep testlib on for the time being.
Trouble seems to start here: https://code.qt.io/cgit/qt/qtdeclarative.git/tree/src/CMakeLists.txt?h=6.3.0#n31
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh but that's the testing library (QtTest), it should have nothing to do with USE=test, which controls the unit tests of qtbase itself
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, that does make more sense.
I'll look more into it, thanks!
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It seems having testlib enabled will require network as well due to QuickControls2, if I'm looking at things right. Going with unconditionally enabling network dependency for the time being.
Will look into the 'QT_FEATURE_testlib=OFF' build issue more when time allows.
Thank you both. I'll try to get to these quicker rather than slacker.
Yeah, not sure how I forgot that existing. |
DESCRIPTION="Library for rendering dynamic web content in Qt6 C++ and QML applications" | ||
|
||
if [[ ${QT6_BUILD_TYPE} == release ]]; then | ||
KEYWORDS="~amd64" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
oh I should revisit ppc64 patchset =) but that's on me ofc. will check later after it lands.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yep, meant to poke you in the start too, but well, need to get these out there first.
Really hoping it will be soon now.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Not much to say and is just a quick surface check, I'm not super familiar with the Qt stack beside working on PyQt6 and maintaining qutebrowser (and these gave me no issues with the current ebuilds). I don't really use Qt for anything else.
media-libs/libglvnd | ||
x11-libs/libxkbcommon | ||
" | ||
RDEPEND="${DEPEND}" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Most likely missing BDEPEND on wayland-scanner
Also, ideally this probably(? haven't really looked at qt6 wayland) needs a split qtwaylandscanner like qt5 to make cross-compiling possible, and any package using qtwayland would also BDEPEND on that. This was a recent'ish qt5 ebuilds change which probably predates when you made these ebuilds similarly to checkreqs (may possibly be other things, but I haven't really been keeping up with qt ebuild changes).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will add the dependency, and leave splitting for later.
find "${S}" -type f -name "*.pr[fio]" | \ | ||
xargs sed -i -e 's|INCLUDEPATH += |&$${QTWEBENGINE_ROOT}_build/include $${QTWEBENGINE_ROOT}/include |' || die |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is how the qt5 ebuild did it, but I don't see the use for the pipe here, furthermore this || die
wouldn't check both commands (needs assert if using a pipe).
Can also do away with the ${S}, e.g.
find . -type f -name "*.pr[fio]" -exec \
sed -i -e 's|INCLUDEPATH += |&$${QTWEBENGINE_ROOT}_build/include $${QTWEBENGINE_ROOT}/include |' {} + || die
The find
in the sanity check below is missing || die
too but given there's 2>/dev/null I assume it's ignoring errors and maybe intended (it's just a sanity-check either way, so error checking is less important).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Adjusted as suggested the former, but left the die be missing for the time being still, since I'm really not sure either if it's intended to be missing.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
About the latter part, when originally contributed [1], it did not have the '2>/dev/null' (nor a '|| die'), but it was later modified as seen in the initial(?) commit with the change [2]. Arfrever did think that it is a good idea to switch to '|| die', however.
# Copyright 2021 Gentoo Authors | ||
# Distributed under the terms of the GNU General Public License v2 | ||
|
||
# @ECLASS: qt6-build.eclass |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Shouldn't qmake-utils.eclass be here too for the qt6_get_bindir stuff? (note meant to sync it with ::gentoo back in gentoo/qt@67fea44 but tossed that PR).
On a related note wrt bindir, qt5 is now installing /usr/bin/qmake5 (symlink), it may make sense to have qmake6 there too. Guess could potentially save from having to use qt*_get_bindir at times.
Also may want to bump qt6-build.eclass' copyright year, albeit I imagine haven't technically touched it since 2021 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
On a related note wrt bindir, qt5 is now installing /usr/bin/qmake5 (symlink), it may make sense to have qmake6 there too. Guess could potentially save from having to use qt*_get_bindir at times.
I could add that my PyQt6{,_WebEngine} ebuilds actually wouldn't need qmake-utils updates with that, could drop the qt6_get_bindir and call qmake6 from PATH 👀
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Mmmm, maybe yeah. Haven't really looked at the eclasses in a while myself. I'll leave this un-done for the time being and push the changes I've made now.
If you think it's a good idea, it's probably a good idea!
Hopefully didn't mess up anything while trying to get as much done today as possible... Things such as
did get into a Manifest. Surprising that I noticed it before pushing anywhere... Funny how nothing complained about it. Anyblue, I believe I did something to all the "resolved discussions" though I'm not sure I like hiding the comments like that (first time really testing that GitHub feature; used it kind of as a check-list of sorts). Plenty of stuff still to be done, but things should in general be in a much better shape. I do still want to check some of the ebuilds not mentioned here as well, and eventually I'll push the changes to the overlay, and they'll all be there nice and logged. |
Pull request CI reportReport generated at: 2022-06-09 19:31 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
REQUIRED_USE="designer? ( widgets )" | ||
|
||
BDEPEND="${PYTHON_DEPS} | ||
$(python_gen_any_dep 'dev-python/html5lib') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this might be missing a [${PYTHON_USEDEP}]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Well, it was there but... see: #25635 (comment)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Oh right. I'd still say it looks odd and is likely to get people asking every so often, but w/e.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I said PYTHON_DEPS was redundant, not PYTHON_USEDEP
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Please re-read #25635 (comment), and re-add USEDEP.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And for reference:
https://projects.gentoo.org/python/guide/any.html#dependencies
${PYTHON_DEPS} gets replaced by a call to python_gen_any_dep
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah, right, I don't think I read the 'BDEPEND="${PYTHON_DEPS}' line at, focusing on the line you commented on. :]
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
More specifically I selected both lines to comment on given they were related (aka any_dep provides python_deps), albeit the email version doesn't make that clear and make it look like I just picked the last line. Ah well, no biggie.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yeah. It may have mostly just been me not physically really seeing the ${PYTHON_DEPS} part. Kind of blends in (at GitHub), and it was a long day etc., but it definitely makes more sense now (especially the part about you not specifically mentioning USEDEP).
But yeah, will fix.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you also add missing BDEPs on |
Or consider patching it out to |
The build system was rewritten in Qt6 so |
At least 'qtbase' seems to do fine still with 'sys-apps/which' unmerged. |
Time to go with 6.3.1 after all it seems. |
- add USE="brotli" - add USE="zstd" - adjust openssl dependency - always build with gif and ico support - fix formatting - flip 'testlib' with USE="gui" due to a quick dependency - remove 'virtual/opengl' dependency As requested at: gentoo/gentoo#25635 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
As requested at: gentoo/gentoo#25635 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
As requested at: gentoo/gentoo#25635 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
As requested at: gentoo/gentoo#25635 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
For qt6, as requested at: gentoo/gentoo#25635 Additionally adjust Chromium patch version, and remove dependency on 'dev-util/gn' since system GN wasn't actually being used even before the option got renamed [1]. 1. https://code.qt.io/cgit/qt/qtwebengine.git/commit/?h=6.3&id=9ea295eec31 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
As requested at: gentoo/gentoo#25635 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Small change to 'dev-qt/qtquick3d':
|
Pull request CI reportReport generated at: 2022-06-22 06:49 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
dev-qt/qtwebengine/metadata.xml
Outdated
@@ -14,6 +14,7 @@ | |||
<flag name="designer">Install the QWebEngineView plugin used to add widgets in <pkg>dev-qt/designer</pkg> forms that display web pages.</flag> | |||
<flag name="geolocation">Enable physical position determination via <pkg>dev-qt/qtpositioning</pkg></flag> | |||
<flag name="jumbo-build">Combine source files to speed up build process.</flag> | |||
<flag name="pipewire">Enable PipeWire support for WebRTC.</flag> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In 5.15, this is 'screencast' IUSE flag instead which is probably more accurate.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done more better now...
geolocation? ( =dev-qt/qtpositioning-${PV}* ) | ||
kerberos? ( virtual/krb5 ) | ||
pipewire? ( media-video/pipewire:= ) | ||
pulseaudio? ( media-sound/pulseaudio:= ) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Topic came up given plasma hard-switched to libpulse, may as well switch it here too. qtwebengine:5 will update once get a chance (probably be good to start updating these on big packages so don't do revbumps when want to cleanup).
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.
Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Version 6.3.1 qtbase in non-split form. Bug: https://bugs.gentoo.org/838970 Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Version 6.3.1 of qttools in non-split form. Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Version 6.3.1 of the Qt APIs and tools for graphics pipelines. Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Version 6.3.1 of the Qt module and API for defining 3D content in Qt QuickTools. Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Version 6.3.1 of the Qt module containing the unsupported Qt 5 APIs. Package-Manager: Portage-3.0.30, Repoman-3.0.3 Signed-off-by: Jimi Huotari <chiitoo@gentoo.org>
Pull request CI reportReport generated at: 2022-07-21 14:14 UTC There are existing issues already. Please look into the report to make sure none of them affect the packages in question: |
Merged after discussion with asturm and Chiitoo again on IRC. I'd promised to do this a week or two ago but didn't get to it. Still masked, refreshed Qt wiki page, and sent email to -dev making clear things may change still. |
Thanks! |
Hopefully I didn't mangle anything up while bringing it over from the overlay.