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

qt6: init at 6.2.0 #141883

Closed
wants to merge 121 commits into from
Closed

qt6: init at 6.2.0 #141883

wants to merge 121 commits into from

Conversation

milahu
Copy link
Contributor

@milahu milahu commented Oct 16, 2021

closed in favor of #169296

Motivation for this change

add latest version of qt, compile from source

Status

all builds are passing
libraries are usable in cmake projects
qmake support is not implemented, but has low priority (qmake is deprecated, qmake2cmake)

estimated build times on 4x2.4GHz

 80 min qtdeclarative
 40 min qtbase
 20 min qtwayland
720 min qtwebengine = 12 hours, similar to chromium

 [todo] pyside - takes surprisingly long
        C++ is heavy to parse, maybe caching could be better -> upstream issue

build

cd path/to/nixpkgs
time nix-build . -A qt6.qtbase --keep-failed 2>&1 | tee qt6.qtbase.$(date +%s).log
ls -d /nix/store/*-qtbase-6.2.?{,-{dev,bin}}

qt6 apps

https://github.com/milahu/awesome-qt6

contributing

feel free to help me with this beast ...

common problems (and solutions) with packaging qt6 for nixos: packaging-qt6.md

related

todo

  • fix cycle error in qtvirtualkeyboard between $bin and $out (low priority)
    • qtvirtualkeyboard is currently disabled in qt-6/6.2/default.nix
  • dynamic qt.conf for qmake. this was partly implemented for qt5 in pkgs/development/libraries/qt-5/qt-env.nix. ideally, qt.conf should be derived from buildInputs
    • low priority, because qmake is deprecated in favor of cmake. we also have a conerter tool, cmake2qmake (in old versions this was called pro2cmake.py)
    • where is cmake2qmake currently? qtbase? qtbase.dev?
    • is this documented in the qt-6/*.md files?
  • libraries are usable in cmake projects. tested by compiling qimgv with qt6. qimgv needs qtbase, qtdeclarative, qtsvg
  • move non-qt6 changes to separate PRs? (low priority)
  • switch apps to qt6
  • fix pending reviews (thanks sandro)
  • squash all commits for qt6
  • fix sqlite in qtbase? -> strawberry config: CMake Warning at CMakeLists.txt:536 (message): The Qt sqlite driver test failed.
  • pyside6 = qt6 bindings for python
    • fix cmake/PySideHelpers.cmake to detect qt modules in QT_ADDITIONAL_PACKAGES_PREFIX_PATH, which is set in qtModule.nix setupHook
  • pyqt6 = qt6 bindings for python
    • last i checked, pyqt5 was bundled with qt *.so files. ideally link to qt packages
    • pyqt is a 3rd party library -> low priority
  • qtbase tests and examples. how to build these? configureFlags or cmakeFlags? where are the tests? how to run the tests?
  • build all modules from srcs.nix (maybe some are missing)
    • all except qtvirtualkeyboard -> cycle error
  • in wrapQtAppsHook, set QML2_IMPORT_PATH for qml apps (qtdeclarative)
    • already done in pkgs/development/libraries/qt-6/hooks/wrap-qt-apps-hook.sh
  • update readme.md for qt6
  • remove review code like qimgv-qt6
  • patch-cmake-files.sh
    • should run before configurePhase → throw early on error
    • add unit test
  • update to latest qt version. currently 6.3.0 see https://download.qt.io/official_releases/qt/
  • update readme
    • maybe move readme to /docs/language-frameworks
  • add version bools qtbase.isQt6 and qtbase.isQt5 etc
  • fix QT_PLUGIN_PATH, see qt6: init at 6.2.0 #141883 (comment)

Things done

  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@nixos-discourse
Copy link

This pull request has been mentioned on NixOS Discourse. There might be relevant details there:

https://discourse.nixos.org/t/attempting-to-build-qt6/10551/8

@milahu milahu marked this pull request as draft October 16, 2021 15:36
@peterhoeg
Copy link
Member

And just to add this - I understand that 6.2 is an LTS release, but KDE as eventually the main downstream consumer of Qt6 is presumably going to be requiring the latest and thus 6.3 is likely to be a requirement for that.

Of course, there is nothing preventing us from carrying both the latest LTS as well as the latest and greatest.

@@ -25,7 +25,8 @@
, p11-kit
, util-linux
, qtbase
, qtx11extras
, qtx11extras ? null # qt5
, qt5compat ? null # qt6
Copy link
Member

Choose a reason for hiding this comment

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

qt5compat isn't used

@@ -62,6 +63,8 @@ mkDerivation rec {
taglib
qtbase
qtx11extras
] ++ lib.optionals (lib.versions.major qtbase.version == "5") [
Copy link
Member

Choose a reason for hiding this comment

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

I wondering if we should introduce isQt5 and isQt6 helpers like what we have for Python as this check (or a variation of it) is most likely going to be littered all over the place.

@@ -0,0 +1,81 @@
# Qt 5 Maintainer's Notes
Copy link
Member

Choose a reason for hiding this comment

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

There's a few references to Qt 5 that should Qt 6.

@milahu
Copy link
Contributor Author

milahu commented Apr 13, 2022

@milahu are there any merge blockers?

"someone" (sorry, i have no energy for this) would need to squash all the qt6 commits
and separate them from the "switch app to qt6" (etc) commits
(also see my todo list in the first post)

apart from that ... not that i know of. it's "basically working"

I'd be glad to merge this before the feature freeze window closes if possible.

yepp. better than "death by perfectionism"

@Artturin
Copy link
Member

Adding qt6 isn't a breaking change so it can be merged anytime and backported

Comment on lines +3 to +27
## Minor Updates

Let `$major` be the major version number, e.g. `5.9`.

1. Change the version number in the `$major/fetch.sh`.
2. Run `./maintainers/scripts/fetch-kde-qt.sh pkgs/development/libraries/qt-5/$major`
from the top of the Nixpkgs tree.

See below if it is necessary to update any patches.

## Major Updates

Let `$major` be the new major version number, e.g. `5.10`.

1. Copy the subdirectory from the previous major version to `$major`.
2. Change the version number in `$major/fetch.sh`.
3. Run `./maintainers/scripts/fetch-kde-qt.sh pkgs/development/libraries/qt-5/$major`
from the top of the Nixpkgs tree.
4. Add a top-level attribute in `pkgs/top-level/all-packages.nix` for the new
major version.
5. Change the `qt5` top-level attribute to point to the new major version.
6. If the previous major version is _not_ a long-term support release,
remove it from Nixpkgs.

See below if it is necessary to update any patches.
Copy link
Member

Choose a reason for hiding this comment

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

All of this should change with Qt 6 because there are not LTS releases anymore. We should only track the latest version in Nixpkgs. (Aside from that, having multiple Qt 5 versions in Nixpkgs never worked correctly anyway.)


See below if it is necessary to update any patches.

## Patches
Copy link
Member

Choose a reason for hiding this comment

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

None of this is applicable anymore.

, qtbase
, qtdeclarative
, libiconv
# FIXME Configure summary: qt5compat is not using libiconv. bug in qt6?
Copy link
Member

Choose a reason for hiding this comment

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

To use libiconv in some module like qt5compat, we probably need to enable the corresponding feature in the qtbase build.

in

qtModule {
pname = "qtwebkit";
Copy link
Member

Choose a reason for hiding this comment

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

I really think we should drop qtwebkit from Qt 6.

Comment on lines +43 to +45
qt5 used `configure`. for example `./configure -system-ffmpeg`

qt6 uses cmake, so we need ...
Copy link
Member

Choose a reason for hiding this comment

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

Does this mean that all configureFlags should be removed from (for example) the qtbase build?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

iirc, some options go to configure, other options go to cmake

ive added a script to the readme to list the available cmake options

@oxalica
Copy link
Contributor

oxalica commented Apr 14, 2022

Note: tdesktop 3.6.3 requires Qt 6 to build now, thus is blocked by this PR.
https://github.com/telegramdesktop/tdesktop/releases/tag/v3.6.3

@K900
Copy link
Contributor

K900 commented Apr 18, 2022

Anything holding this back still? tdesktop 3.7.0 also requires Qt6 now, and the current version requires some jank to keep going (see #169180).

@NickCao
Copy link
Member

NickCao commented Apr 18, 2022

I think the biggest blocker is whether it builds.

@NickCao
Copy link
Member

NickCao commented Apr 18, 2022

Just picked this up https://github.com/NickCao/nixpkgs/tree/qt6 and bumped it to qt 6.3.0, surprisingly most of the things builds with minor fixes (with qtwebengine as the only exception). tdesktop 3.7.0 builds and works fine with this.

@NickCao
Copy link
Member

NickCao commented Apr 18, 2022

Still having some trouble in getting IME (fcitx5) to work. export QT_PLUGIN_PATH=/run/current-system/sw/lib/qt-6.3.0/plugins fixes the issue.
The lack of this particular patch should be the root cause: https://github.com/NickCao/nixpkgs/blob/qt6/pkgs/development/libraries/qt-5/5.12/qtbase.patch.d/0010-qtbase-qtpluginpath.patch

@Kranzes
Copy link
Member

Kranzes commented Apr 18, 2022

Just picked this up https://github.com/NickCao/nixpkgs/tree/qt6 and bumped it to qt 6.3.0, surprisingly most of the things builds with minor fixes (with qtwebengine as the only exception). tdesktop 3.7.0 builds and works fine with this.

I also tested tdesktop 3.7.0 from your branch, works fine.

@milahu
Copy link
Contributor Author

milahu commented Apr 18, 2022

@NickCao thx : )

im curious how 8b2eed21b7ecdb8f788810f21ea927c0cabea8c5 works ...

iirc, my goal was to make the qt libs usable in cmake projects
by adding the line

  INTERFACE_LINK_DIRECTORIES "\${_((QT_MODULE_NAME))_NIX_OUT}/lib"

before the line

  INTERFACE_INCLUDE_DIRECTORIES ...

in the generated *.cmake files
otherwise cmake would say "package found but files are missing"

so, in my remote diagnosis, your version should break the tdesktop build, but apparently it works?

maybe this was fixed between qt 6.2 and 6.3
how do your generated *.cmake files look when you grep -C10 for INTERFACE_INCLUDE_DIRECTORIES?

@ttuegel
Copy link
Member

ttuegel commented Apr 18, 2022

The lack of this particular patch should be the root cause: https://github.com/NickCao/nixpkgs/blob/qt6/pkgs/development/libraries/qt-5/5.12/qtbase.patch.d/0010-qtbase-qtpluginpath.patch

This patch caused a lot of headaches for Qt 5 and in the strongest way possible, I would encourage us not to make that mistake again. Maintainers should set up the correct wrappers for their applications.

@NickCao
Copy link
Member

NickCao commented Apr 18, 2022

im curious how 8b2eed2 works ...

Me myself don't know how either, I removed that line simply because it was patching too much, breaking unrelated cmake files specifically the one for libdrm vendoered in qtbase. And somehow, it just works.

@Artturin
Copy link
Member

Still needs lots of cleanup and resyncing the improvements to the qt5 packaging made during the time this pr has been open

The ? nulls should he replaced with something better too

@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

Tinkered a little bit around the cmake patching hacks, found that it seems all the patched files are named SomeQtComponentTargets{,-release}.cmake and is generated by cmake. Can we just patch the template from which the cmake files are generated and solve this once and for all?

@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

By applying a questionable and overfitting patch atop cmake, I managed to cut the hacky list of regex by half.
NickCao@8cd5854

@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

By further neglecting support for qmake, the final approach is now clean and clear:
NickCao@f5aa39f
After all cmake is not complaining anything about that.

@NickCao NickCao mentioned this pull request Apr 19, 2022
13 tasks
@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

Just noticed that the qt plugins depends on the main output in their rpaths, so what's the point of splitting them into another output?

@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

Also as the coexisting multiple minor qt versions approach has never really worked out, dropping that also looks like a nice choice for me.

@milahu
Copy link
Contributor Author

milahu commented Apr 19, 2022

most of the things builds with minor fixes (with qtwebengine as the only exception)

what's the issue with qtwebengine?

@NickCao
Copy link
Member

NickCao commented Apr 19, 2022

./third_party/webgpu-cts/src/tools/deno: unsupported interpreter directive "#!/usr/bin/env -S deno run --unstable --allow-read --allow-write --allow-env --no-check" (set dontPatchShebangs=1 and handle shebang patching yourself)

@milahu
Copy link
Contributor Author

milahu commented Apr 19, 2022

oh .. stupid patchShebangs is confused by the -S option #77539

if [[ "$oldPath" == *"/bin/env" ]]; then
# Check for unsupported 'env' functionality:
# - options: something starting with a '-'
# - environment variables: foo=bar
if [[ $arg0 == "-"* || $arg0 == *"="* ]]; then
echo "$f: unsupported interpreter directive \"$oldInterpreterLine\" (set dontPatchShebangs=1 and handle shebang patching yourself)" >&2
exit 1
fi

you could try

{
  postPatch = ''
    sed -i.bak 's|^#!/usr/bin/env -S deno|#! ${deno}/bin/deno|' ./third_party/webgpu-cts/src/tools/deno
    diff -u ./third_party/webgpu-cts/src/tools/deno{.bak,} || true; exit 1 # debug
  '';

@milahu
Copy link
Contributor Author

milahu commented May 5, 2022

closing in favor of #169296

keeping the branch qt-6-init

@milahu milahu closed this May 5, 2022
@milahu milahu changed the title qt-6: init 6.2.0 qt6: init at 6.2.0 Dec 30, 2022
@milahu milahu deleted the qt-6-init branch January 25, 2023 10:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Package request: Qt 6