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

Unwrap Qt 5 packages #26336

Merged
merged 45 commits into from Jun 18, 2017
Merged

Unwrap Qt 5 packages #26336

merged 45 commits into from Jun 18, 2017

Conversation

ttuegel
Copy link
Member

@ttuegel ttuegel commented Jun 2, 2017

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
Copy link
Member

FRidh commented Jun 3, 2017

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
Copy link
Member Author

ttuegel commented Jun 3, 2017

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
Copy link
Member

FRidh commented Jun 3, 2017

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
Copy link
Member

AndersonTorres commented Jun 3, 2017 via email

@olejorgenb
Copy link
Contributor

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
Copy link
Member Author

ttuegel commented Jun 4, 2017

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
Copy link
Contributor

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.


</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>
Copy link
Member

@Mic92 Mic92 Jun 6, 2017

Choose a reason for hiding this comment

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

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.

@ttuegel ttuegel force-pushed the qt--unwrap branch 3 times, most recently from 10cc10e to ae4a628 Compare June 16, 2017 22:00
@ttuegel 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
@gleber
Copy link
Contributor

gleber commented Jun 18, 2017

@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
Copy link
Member

Mic92 commented Jun 18, 2017

ultrastar-creator fixed in 558b5fb

@gleber
Copy link
Contributor

gleber commented Jun 18, 2017

@Mic92 ultrastar-manager still has references to makeQtWrapper

@Mic92
Copy link
Member

Mic92 commented Jun 18, 2017

9f14594

@gleber
Copy link
Contributor

gleber commented Jun 18, 2017

@Mic92 Awesome, you rock!

@vcunat
Copy link
Member

vcunat commented Jul 4, 2017

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
Copy link
Member

FRidh commented Jul 4, 2017

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

@vcunat
Copy link
Member

vcunat commented Jul 5, 2017

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

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.

None yet

9 participants