qt5base: fix dependencies and improve packaging resilience - #105
qt5base: fix dependencies and improve packaging resilience#105titobrasolin wants to merge 1 commit into
Conversation
| $(call Package/qt5base/Default) | ||
| TITLE+=platform linuxfb (plugin) | ||
| DEPENDS+=+qt5base-core +qt5base-gui +libfreetype +PACKAGE_qt5base-input-autodiscovery:libudev +PACKAGE_qt5base-plugin-input-libinput:libinput | ||
| DEPENDS+=+qt5base-core +qt5base-gui +libfreetype +libdrm +PACKAGE_qt5base-input-autodiscovery:libudev +PACKAGE_qt5base-plugin-input-libinput:libinput |
There was a problem hiding this comment.
Just double checking: libdrm is now unconditionally considered a dependency, because we do not (want to) support the legacy-framebuffer interface anymore, correct?
There was a problem hiding this comment.
Dug into the qtbase 5.15.16 sources to get a definitive answer instead of guessing.
It's not a deliberate move away from legacy fbdev — linuxfb's plain, non-DRM fbdev path (qlinuxfbscreen.cpp) is untouched and still the default rendering path. The libdrm dependency comes from Qt5 itself:
src/plugins/platforms/linuxfb/linuxfb.pro#L20-L24conditionally compiles in an additional DRM-accelerated screen backend (qlinuxfbdrmscreen.cpp) alongside the plain fbdev one, wheneverkms_support-privateis available — it's baked into the samelibqlinuxfb.so.src/platformsupport/kmsconvenience/kmsconvenience.pro#L15— thekms_supportmodule — is a static internal module withQMAKE_USE += drm.- It only gets built when
qtConfig(kms)is true (src/platformsupport/platformsupport.pro#L28-L29), and that feature is itself gated onlibs.drminsrc/gui/configure.json#L1276-L1279.
We configure with --kms=$(if $(CONFIG_BUILD_qt5base-gui_DRM),yes,no), and BUILD_qt5base-gui_DRM defaults to y. So with the default config, the DRM linkage ends up baked into libqlinuxfb.so's NEEDED list regardless of which fbdev mode a board actually uses at runtime — same class of issue as the libatomic case in #74, just Qt-internal this time rather than toolchain-internal.
Given that, unconditional +libdrm is correct as long as BUILD_qt5base-gui_DRM defaults to y — but it's more precise to tie it to that same symbol instead of leaving it always-on:
- DEPENDS+=+qt5base-core +qt5base-gui +libfreetype +libdrm +PACKAGE_qt5base-input-autodiscovery:libudev +PACKAGE_qt5base-plugin-input-libinput:libinput
+ DEPENDS+=+qt5base-core +qt5base-gui +libfreetype +BUILD_qt5base-gui_DRM:libdrm +PACKAGE_qt5base-input-autodiscovery:libudev +PACKAGE_qt5base-plugin-input-libinput:libinputso a build that explicitly disables DRM support won't carry it either.
I'd like to look further into the other points raised in this PR before amending the commit and updating it, so I'll hold off pushing until I've gone through those as well.
| $(call Package/qt5base/Default) | ||
| TITLE+=libinput (plugin) | ||
| DEPENDS+=+qt5base-core +qt5base-gui +libinput | ||
| DEPENDS+=+qt5base-core +qt5base-gui +libinput @PACKAGE_qt5base-input-autodiscovery |
There was a problem hiding this comment.
This feels kind of odd - why would libinput support depend on the input-discovery package being present?
There was a problem hiding this comment.
Hi Mirko, thanks for the feedback!
I added this dependency because, as far as I can tell, Qt5's libinput support has a strict pre-condition requiring libudev to be detected during configure.
Since udev is currently hard-disabled in this Makefile (see lines 35-36):
# hard disable udev support for now
CONFIG_PACKAGE_qt5base-input-autodiscovery:=
It seems that libinput fails to meet its requirements. In fact, a CI test I ran without the @PACKAGE_qt5base-input-autodiscovery constraint consistently fails with:
ERROR: Feature 'libinput' was enabled, but the pre-condition 'features.libudev && libs.libinput' failed.
I used the @ dependency as a sort of safety switch to keep the build green, but I'd be happy to hear your thoughts on a better way to handle these implicit Qt5 requirements!
There was a problem hiding this comment.
All info I can gather around this "hard disable" for now is this:
While it was both me creating the commit and the Qt-ticket, I don't recall the details anymore, other than "behaviour changed".
As a matter of fact, I never saw the reason for using libinput anyway.
evdev works fine with /dev/event* devices.
Maybe just purging the libinput-support/-package for good?
There was a problem hiding this comment.
Thanks for the feedback, Mirko — and sorry for the slow follow-up.
Looking into it a bit more: libinput's device discovery relies on udev's ID_INPUT_* tagging to classify devices (keyboard, mouse, touchpad, etc.) — it doesn't have another mechanism for that. So as long as qt5base-input-autodiscovery (udev) stays disabled here, a libinput plugin wouldn't really have a working discovery path regardless of how the configure precondition is handled.
Given that, the @PACKAGE_qt5base-input-autodiscovery:libinput dependency seems correct as a reflection of an actual (currently unmet) requirement, rather than an arbitrary workaround. I'd keep it as-is for this PR, and treat a possible removal of libinput in favor of evdev as a separate follow-up issue/PR, so it doesn't hold up #73/#74/#104. Happy to open that follow-up if it sounds reasonable to you.
| $(if $(wildcard $(PKG_INSTALL_DIR)/usr/lib/qt5/plugins/egldeviceintegrations/libqeglfs-kms-egldevice-integration.so),$(call Build/Install/Plugins,$(1),egldeviceintegrations,libqeglfs-kms-egldevice-integration)) \ | ||
| $(if $(wildcard $(PKG_INSTALL_DIR)/usr/lib/qt5/plugins/egldeviceintegrations/libqeglfs-kms-integration.so),$(call Build/Install/Plugins,$(1),egldeviceintegrations,libqeglfs-kms-integration)) \ | ||
| $(call Build/Install/Plugins,$(1),platforms,libqeglfs) | ||
| ) |
There was a problem hiding this comment.
This feels like a huge pitfall and debug hell, where the build completes, but potentially with a different set of included files than expected.
As in: with the same qt5-config options, but different qt5-unrelated options (e.g. related to target), the qt5 build results in a different set of files.
That might be fine, if it would explicitly depend on a config option (e.g. "if not mips"), but simply not failing when files didn't get created/generated, sounds like the wrong approach and there's no way to tell why that happened, except files were not present for whatever reason.
There was a problem hiding this comment.
Thanks for raising this, Mirko.
Looking at how Qt handles this upstream: EGLFS device integration backends have been designed as autodetected/optional since this commit introduced the plugin architecture — when no preferred backend is configured, Qt tries available plugins in unspecified order and falls back to the built-in integration if none succeed. Current Qt docs describe the same logic still applying: backend selection depends on what's available (drm/gbm, etc.), not a fixed switch.
So which egldeviceintegrations/*.so files get produced for a given target depends on what Qt's configure detects in that target's sysroot, which can vary across architectures even with identical qt5base-related OpenWrt config — matching what #104 describes. In that sense, the wildcard check reflects the same "optional by design" behavior Qt applies at runtime, rather than masking an unrelated failure.
That said, the concern about silent skips still seems valid — there's no way from the log alone to tell "not applicable here" from "should have been built but wasn't." Adding a $(warning ...) when a plugin is skipped could keep the build tolerant while making the skip visible. Open to other approaches if there's a better fit.
mv refuses to move a directory onto an existing non-empty directory of the same name, so Build/Install/Default and Host/Install/Default fail with "Directory not empty" whenever PKG_INSTALL_DIR/host or /usr are already populated (e.g. a retried build). cp -a merges into the destination instead, matching the actual intent of moving the .owrttmp staging tree into place. Related to upstream openwrt#105.
7ce891b to
2844ef1
Compare
Formality Check: Suggestions AvailableWe checked this pull request against the contribution guidelines. Here is what needs your attention:
|
|
Pushed an update (force-push, amended
|
2844ef1 to
00865f7
Compare
This update strengthens the qt5base package for diverse architectures: - Add +libatomic to core dependencies to prevent linking errors. - Add +libdrm to linuxfb and eglfs platform plugins. - Add @PACKAGE_qt5base-input-autodiscovery to libinput plugin dependencies. - Introduce wildcard checks in installation macros to prevent 'cannot stat' errors when specific plugins are disabled by the Qt configuration. - Fix --qpa configuration logic in Build/Configure. - Clean up and refresh existing patches, removing the obsolete 200-header-dep-fix.patch. Fixes: openwrt#73 Fixes: openwrt#74 Fixes: openwrt#104 Signed-off-by: Tito Brasolin <tito.brasolin@gmail.com>
00865f7 to
e87fced
Compare
This update strengthens the qt5base package for diverse architectures:
Fixes: #73
Fixes: #74
Fixes: #104