Unwrap Qt 5 packages #26336

ttuegel · 2017-06-02T22:59:57Z

Motivation

The Qt 5 wrappers are difficult to maintain and prevent runtime interoperability between packages. The latter is an important goal for desktop environments such as KDE. Fortunately, the wrappers are also unnecessary.

This pull request removes makeQtWrapper and kdeWrapper. Instead of wrapping programs, required runtime dependencies are propagated through propagatedUserEnvPkgs. However, this does mean that some applications may not work correctly if invoked directly from the Nix store. This necessitates other changes, most notably: Qt plugins are now installed in a different prefix for each minor version to prevent conflicts.

Testing

Tested using sandboxing
(nix.useSandbox on NixOS,
or option build-use-sandbox in nix.conf
on non-NixOS)
Built on platform(s)
- NixOS
- macOS
- Linux
Tested compilation of all pkgs that depend on this change using nix-shell -p nox --run "nox-review wip"
Tested execution of all binary files (usually in ./result/bin/)
Fits CONTRIBUTING.md.

I have also run the relevant NixOS tests.

FRidh · 2017-06-03T10:49:46Z

The Qt 5 wrappers are difficult to maintain and prevent runtime interoperability between packages.

Could you refer to examples? Can nix-shell (or a future nix run) deal with propagatedUserEnvPkgs?

ttuegel · 2017-06-03T15:19:23Z

Could you refer to examples?

Maintainability

Whenever an old package gets a new dependency, I have to go through the whole dependency graph and prune the $dev outputs (the wrappers tend to be a little zealous about retaining outputs).

Interoperability

In another branch, I am packaging the KDE PIM suite (KMail, KOrganizer, Kontact, etc.). All the KDE packages are designed to detect optional runtime dependencies, but the PIM suite takes this to an extreme. With the wrappers, we either need to package the PIM suite as one monolith or tolerate large duplicate symlink farms for each package.

I don't like the monolith approach because it requires that we fight upstream's design for their software. It is designed to be installed into an environment and cooperate with other packages in that environment, and I don't like subverting that goal. Also, I imagine many users do not need or want the entire PIM suite; for example, they may only use KMail and do not want to install gigabytes of unrelated software.

We had a solution in kdeWrapper for dealing with large symlink farms. This works fairly well, but only if you don't use nix-env. I have some personal feelings about nix-env (I don't like it) but it is part of the Nix ecosystem and we need it to work right. Ultimately, kdeWrapper creates an environment to exist alongside the environments created by nix-env or nixos-rebuild. It duplicates the functionality of those environments, adding another layer of complexity, and it breaks nix-env... I think these are good reasons to seek another solution.

Can nix-shell (or a future nix run) deal with propagatedUserEnvPkgs?

nix-shell cannot do so by itself, but I could modify the Qt tooling slightly to automatically generate an environment for each package that could be used with nix-shell. I think we can borrow a trick from cabal2nix to detect if we're running in nix-shell and switch to the environment automatically, instead of the package's default output.

FRidh · 2017-06-03T15:39:53Z

Thanks for the explanation @ttuegel. We should indeed not fight upstream's design. I thought it be good to have the argumentation you gave now because its otherwise hard to track why certain design choices were made and abandoned.

AndersonTorres · 2017-06-03T16:36:16Z

We can then eliminate any occurrence of qt?Wrappers? 2017-06-03 12:39 GMT-03:00 Frederik Rietdijk <notifications@github.com>:

…

Thanks for the explanation @ttuegel <https://github.com/ttuegel>. We should indeed not fight upstream's design. I thought it be good to have the argumentation you gave now because its otherwise hard to track why certain design choices were made and abandoned. — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#26336 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AFrc9iFRWY5dhFviCAbndeT_J-IK4PtRks5sAX5LgaJpZM4Nu4D2> .

olejorgenb · 2017-06-04T08:31:05Z

However, this does mean that some applications may not work correctly if invoked directly from the Nix store.

What does that mean exactly? Some apps must be launched through a .desktop file? Simply typing the command name in a shell wont work?

ttuegel · 2017-06-04T23:17:30Z

What does that mean exactly? Some apps must be launched through a .desktop file? Simply typing the command name in a shell wont work?

It means that nix-build -A foo; ./result/bin/foo may not work. nix-env -iA foo; foo will always work. I am taking steps to see that nix-shell -p foo also works.

olejorgenb · 2017-06-05T04:20:47Z

Thanks for the clarification. That doesn't sound too bad. As a non-desktop-suite user I got a bit worried there for a second :)

A bit off topic, but just in case these changes make it simpler/possible to fix the icon issue in kde apps installed/run as standalone apps:

Okular5 for instance works when installed standalone (lots of icons missing, but not that big a deal actually). BUT - startup time is affected due to how icon lookup works: okular makes 400k+ stat calls looking for icons.

This seems to be because QT_QPA_PLATFORMTHEME isn't set. Setting it to eg. "kde" reduces the stat calls to 9k and startup time is at least cut in half on my system.

Mic92 · 2017-06-06T07:28:05Z

doc/languages-frameworks/qt.xml


 </section>

 <section xml:id="ssec-qt-kde"><title>KDE</title>

-<para>The KDE Frameworks are a set of libraries for Qt 5 which form the basis of the Plasma desktop environment and the KDE Applications suite. Packaging a Frameworks-based library does not require any steps beyond those described above for general Qt-based libraries. Frameworks-based applications should not use <literal>makeQtWrapper</literal>; instead, use <literal>kdeWrapper</literal> to create the necessary wrappers: <literal>kdeWrapper { unwrapped = <replaceable>expr</replaceable>; targets = <replaceable>exes</replaceable>; }</literal>, where <replaceable>expr</replaceable> is the un-wrapped package expression and <replaceable>exes</replaceable> is a list of strings giving the relative paths to programs in the package which should be wrapped.</para>
+<para>The KDE Frameworks are a set of libraries for Qt 5 which form the basis of the Plasma desktop environment and the KDE Applications suite. Packaging a Frameworks-based package does not require any steps beyond those described above for general Qt-based packages.</para>


The manual should also mention, that the result of nix-build cannot be directly called
and suggest that nix-shell -p <command> can be used as an alternative.

Using a distinct prefix for plugins and QML libraries allows multiple Qt 5 minor versions to coexist in the same environment.

QStringLiteral stores strings in UTF-16 format, preventing Nix from detecting dependencies.

It is no longer necessary to wrap Qt packages

Plugin and QML import paths were previously determined by NIX_PROFILES. Using PATH instead allows Qt applications to work under nix-shell without further modification.

This seem to have been broken in NixOS#26336.

gleber · 2017-06-18T21:29:16Z

@ttuegel Looks like packages inited in #26524 were not covered by this PR. My understanding of changes in this PR is not good enough to fix it. Can you take a look?

Mic92 · 2017-06-18T21:36:55Z

ultrastar-creator fixed in 558b5fb

gleber · 2017-06-18T21:39:39Z

@Mic92 ultrastar-manager still has references to makeQtWrapper

Mic92 · 2017-06-18T21:40:28Z

9f14594

gleber · 2017-06-18T21:42:50Z

@Mic92 Awesome, you rock!

vcunat · 2017-07-04T06:29:45Z

Any immediate ideas about this seemingly non-sensical error of kdiff3? http://hydra.nixos.org/build/55590912

Could NOT find KF5 (missing: Crash) (found suitable version "5.34.0", minimum required is "5.16.0")

Bisection on staging narrowed-down the range into this PR's commits; bad: fc72335, good: 6beea32. Interestingly, it does build on master. There's also a similar error for kdevplatform: http://hydra.nixos.org/build/55596900

FRidh · 2017-07-04T07:02:02Z

@vcunat I've been adding the missing dependencies to the packages. Maybe some dependencies were not propagated anymore.

vcunat · 2017-07-05T08:12:18Z

Thanks, the related problems I know are fixed on master, after 43ca911 and a couple preceding commits.

ttuegel added 0.kind: enhancement 6.topic: qt/kde labels Jun 2, 2017

ttuegel self-assigned this Jun 2, 2017

ttuegel force-pushed the qt--unwrap branch from 6fef917 to c594ab5 Compare June 2, 2017 23:37

ttuegel mentioned this pull request Jun 5, 2017

QTTools' qcollectiongenerator broken by wrapper #26407

Closed

Mic92 reviewed Jun 6, 2017

View reviewed changes

ttuegel force-pushed the qt--unwrap branch 3 times, most recently from 10cc10e to ae4a628 Compare June 16, 2017 22:00

ttuegel added 14 commits June 18, 2017 08:41

qt5: use distinct lib/qt-5.x prefix for each minor version

3f3d33a

Using a distinct prefix for plugins and QML libraries allows multiple Qt 5 minor versions to coexist in the same environment.

qt5: rename qmakeHook to qmake

210f688

qt5: move kdeDerivation to qt5.mkDerivation

7b6f8dc

kdeApplications: rename kdeApp to mkDerivation

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

6beea32

plasma5: rename plasmaPackage to mkDerivation

eaa644a

kdeFrameworks: rename kdeFramework to mkDerivation

945758f

Remove kdeWrapper

Verified

This commit was created on GitHub.com and signed with GitHub’s verified signature. The key has expired.

GPG key ID: 4AEE18F83AFDEB23
Expired

Verified
Learn about vigilant mode

be7b7d9

sddm: take themes from system environment

870c07c

qmake: unify Darwin and Linux setup hooks

1607f51

qt5: remove makeQtWrapper

c816bbc

qtbase: setup required environment in GTK3 platform theme plugin

5a8295e

qt5: separate plugins from libraries

6fa4178

qt5: use one output by default

d9f29af

qtbase: fix plugin path in CMake modules

dea7372

ttuegel added 15 commits June 18, 2017 08:44

plasma-desktop: pass path to hwclock through preprocessor macro

a4be1a6

kwin: pass path to Xwayland through preprocessor macro

7f95d48

plasma5: fixup inputs and outputs

8538d75

kservice: propagate shared_mime_info to environment

acc4576

libkscreen: use QLatin1String instead of QStringLiteral

d9094c0

QStringLiteral stores strings in UTF-16 format, preventing Nix from detecting dependencies.

kwindowsystem: use QLatin1String instead of QStringLiteral

b7f895d

qt58.qtbase: use QLatin1String instead of QStringLiteral

f392614

qt58.qtserialport: use QLatin1String instead of QStringLiteral

b1dcd27

qt58.qtwebkit: use QLatin1String instead of QStringLiteral

c546c26

qt56.qtwebkit: use QLatin1String instead of QStringLiteral

8cc09c9

qt56.qtserialport: use QLatin1String instead of QStringLiteral

99ed585

nixpkgs: remove comments from manual about wrapping Qt packages

f3ce852

It is no longer necessary to wrap Qt packages

nixpkgs: remark about running Qt applications

ce28d89

qt58: determine plugin and import paths from PATH

dab7700

Plugin and QML import paths were previously determined by NIX_PROFILES. Using PATH instead allows Qt applications to work under nix-shell without further modification.

qt56: determine plugin and import paths from PATH

e6cf9b9

Plugin and QML import paths were previously determined by NIX_PROFILES. Using PATH instead allows Qt applications to work under nix-shell without further modification.

ttuegel force-pushed the qt--unwrap branch from ae4a628 to e6cf9b9 Compare June 18, 2017 13:45

ttuegel merged commit f2a651a into NixOS:master Jun 18, 2017

gleber added a commit to gleber/nixpkgs that referenced this pull request Jun 18, 2017

qt: fix qmakeHook -> qmake in few packages.

2fb67d9

This seem to have been broken in NixOS#26336.

gleber mentioned this pull request Jun 18, 2017

qt: fix qmakeHook -> qmake in few packages. #26700

Merged

7 tasks

jerith666 mentioned this pull request Jun 25, 2017

krfb: add new qtx11extras dependency #26857

Merged

8 tasks

ghost mentioned this pull request Jul 28, 2017

Some KDE apps are broken on nixos-unstable #27334

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unwrap Qt 5 packages #26336

Unwrap Qt 5 packages #26336

ttuegel commented Jun 2, 2017

FRidh commented Jun 3, 2017

ttuegel commented Jun 3, 2017

FRidh commented Jun 3, 2017

AndersonTorres commented Jun 3, 2017 via email

olejorgenb commented Jun 4, 2017

ttuegel commented Jun 4, 2017

olejorgenb commented Jun 5, 2017

Mic92 Jun 6, 2017 •

edited

Loading

gleber commented Jun 18, 2017

Mic92 commented Jun 18, 2017

gleber commented Jun 18, 2017

Mic92 commented Jun 18, 2017

gleber commented Jun 18, 2017

vcunat commented Jul 4, 2017

FRidh commented Jul 4, 2017

vcunat commented Jul 5, 2017 •

edited

Loading

Unwrap Qt 5 packages #26336

Unwrap Qt 5 packages #26336

Conversation

ttuegel commented Jun 2, 2017

Motivation

Testing

FRidh commented Jun 3, 2017

ttuegel commented Jun 3, 2017

Maintainability

Interoperability

FRidh commented Jun 3, 2017

AndersonTorres commented Jun 3, 2017 via email

olejorgenb commented Jun 4, 2017

ttuegel commented Jun 4, 2017

olejorgenb commented Jun 5, 2017

Mic92 Jun 6, 2017 • edited Loading

Choose a reason for hiding this comment

gleber commented Jun 18, 2017

Mic92 commented Jun 18, 2017

gleber commented Jun 18, 2017

Mic92 commented Jun 18, 2017

gleber commented Jun 18, 2017

vcunat commented Jul 4, 2017

FRidh commented Jul 4, 2017

vcunat commented Jul 5, 2017 • edited Loading

Mic92 Jun 6, 2017 •

edited

Loading

vcunat commented Jul 5, 2017 •

edited

Loading