NixOS / nixpkgs

Nix Packages collection & NixOS
MIT License
18.42k stars 14.36k forks source link

Tracking issue for wrapQtAppsHook #65399

Closed ttuegel closed 4 years ago

ttuegel commented 5 years ago

This issue is to track the open pull requests for Qt applications broken by the addition of wrapQtAppsHook.

Maintainers, please refer to the manual for instructions to update your packages. Packages that conformed to the prior version of the manual should not need to be updated.

Pull requests

bjornfor commented 5 years ago

@ttuegel: Can you please clarify this. In the manual you linked to the example expression starts with

{ mkDerivation, lib, qtbase }:

and the text says

Import mkDerivation and Qt (such as qtbase modules directly. Do not import Qt package sets; [...].

This confuse me, since in this comment you seem to say the correct fix is to use qt5.mkDerivation (which AFAICT uses the qt5 package set). So, is using { fetchurl, qt5 }: qt5.mkDerivation { ... } ok?

Packages that conformed to the prior version of the manual should not need to be updated.

This also confuse me:

ttuegel commented 5 years ago

So, is using { fetchurl, qt5 }: qt5.mkDerivation { ... } ok?

I'm sorry for being unclear. What I mean is, you should write

{ mkDerivation }: mkDerivation { ... }

and call the package with qt5.callPackage or libsForQt5.callPackage.

If the manual changed (new recommended way to write expressions for Qt apps), why shouldn't every expression be updated to follow the latest guideline?

The prior version of the manual is still on the NixOS website. The recommended way to write expressions for Qt applications has not changed. I updated the manual to include information about wrapQtAppsHook because that is new. Using stdenv.mkDerivation with Qt applications has always been unsupported. Expressions that already conformed to the manual do not need to be updated. Expressions that did unsupported things before are probably broken now.

Thra11 commented 5 years ago

I think the lxqt desktop may have been broken. lxqt-session complains that it can't find the "xcb" plugin in "", which probably means its platform plugin path is wrong or not set.

My NixOS xserver config in case it's relevant:

services.xserver = {
      enable = true;
      videoDrivers = [ "vesa" "modesetting" ];
      displayManager.sddm.enable = true;
      desktopManager.lxqt.enable = true;
};
worldofpeace commented 5 years ago

@Thra11 It appears that all of the expressions for lxqt use stdenv.mkDerivation instead of mkDerivaiton.

Edit: PR incoming to fix this.

samueldr commented 5 years ago

Hmmm, I'm hitting some weirdness on master right now, from a nixos-19.03 system.

Tell me if it's not related.

~/tmp/nixpkgs/nixpkgs $ git checkout master
Already on 'master'
Your branch is up to date with 'origin/master'.

~/tmp/nixpkgs/nixpkgs $ git rev-parse HEAD
a7d6390804c1f18737f71be62a02799f1b23e9a6

~/tmp/nixpkgs/nixpkgs $ nix-build -A cool-retro-term
/nix/store/3laxbqbiwclcvf6iir0rw9d391yjyd6a-cool-retro-term-1.1.1

~/tmp/nixpkgs/nixpkgs $ result/bin/cool-retro-term
Cannot mix incompatible Qt library (version 0x50c00) with this library (version 0x50c03)
Aborted

~/tmp/nixpkgs/nixpkgs 134 $ env -i DISPLAY="$DISPLAY" XAUTHORITY="$XAUTHORITY" result/bi
n/cool-retro-term
[... basically it works ...]

The fact that it works with env -i makes me think there may be contamination from my env.

What's peculiar is that from not too far back, it worked, so I'm bisecting.

eb4e067686d1121d2d4a3d7ac2ed080339125eeb # bad
70eae830431368d04abc387069a30450220e9247 # good

The bad commit being the merge of the good to master.

(Though I cannot test using cool-retro-term for bisecting.)


EDIT: it started after #64598. (The first commit itself won't build, but the second commit will. The parent commit is fine.)

This regression seems to coincide with, if not come from, the upgrade to 5.12.3.

ttuegel commented 5 years ago

It seems like there is some part of Qt 5.12.0 (0x50c00) is sticking around after the upgrade to Qt 5.12.3 (0x50c03), but I'm at a complete loss as to what. I do not think this is related to wrapQtAppsHook, at least not directly.

samueldr commented 5 years ago

The reason I think it may be related, is that when clearing the environment with env -i it ends up working. It may be that something needs to be added to the wrapper that wasn't already.


Digging more, now that I have slept, I think I traced the issue.

16510 openat(AT_FDCWD, "/nix/store/9q4gqkxpzfy0sl37wsnqw4mycyv53pi5-qtbase-5.12.0-bin/lib/qt-5.12/plugins/platforminputcontexts/libibusplatforminputcontextplugin.so", O_RDONLY|O_CLOEXEC) = 7
16510 read(7, "\177ELF\2\1\1\3\0\0\0\0\0\0\0\0\3\0>\0\1\0\0\0\220\216\0\0\0\0\0\0"..., 832) = 832

libibusplatforminputcontextplugin

unset QT_IM_MODULE and it crashes later. So, there is at least an input method impurity...

(Following up in another comment for the other impurity.)

samueldr commented 5 years ago

(Ensure you're not on a fresh master as the upgrade has been reverted.)

Without the input method impurity, I have multimc, and cool-retro-term somehow loading from the system path.

1603 16752 readlink("/nix/store/2d365d3hrp4h9ybpr60dxv74hbwyb0df-system-path/lib/qt-5.12/plugins/platforms", "/nix/store/9q4gqkxpzfy0sl37wsnqw"..., 4095) = 91
1606 16752 lstat("/nix/store/9q4gqkxpzfy0sl37wsnqw4mycyv53pi5-qtbase-5.12.0-bin", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
[...]
8149 17520 openat(AT_FDCWD, "/nix/store/9q4gqkxpzfy0sl37wsnqw4mycyv53pi5-qtbase-5.12.0-bin/lib/qt-5.12/plugins/bearer/libqconnmanbearer.so", O_RDONLY|O_CLOEXEC <unfinished ...>
 1307 18954 readlink("/nix/store/2d365d3hrp4h9ybpr60dxv74hbwyb0df-system-path/lib/qt-5.12/plugins/platforms", "/nix/store/9q4gqkxpzfy0sl37wsnqw"..., 4095) = 91
1310 18954 lstat("/nix/store/9q4gqkxpzfy0sl37wsnqw4mycyv53pi5-qtbase-5.12.0-bin", {st_mode=S_IFDIR|0555, st_size=4096, ...}) = 0
[...]
14675 18954 openat(AT_FDCWD, "/nix/store/9q4gqkxpzfy0sl37wsnqw4mycyv53pi5-qtbase-5.12.0-bin/lib/qt-5.12/plugins/sqldrivers/libqsqlite.so", O_RDONLY|O_CLOEXEC) = 14

I have unset QT_PLUGIN_PATH and QTWEBKIT_PLUGIN_PATH, I have no other Q* environment variables; unsetting XDG_DATA_DIRS is not the solution either.

Though, unsetting PATH helps.


Quoting myself from #44047

What is going on currently is that for any Qt plugin, they will be loaded according to the environment, and according to some hard-coded paths relative to components of PATH. This will not work properly when the environment is cleared or manipulated. (With non-NixOS platforms, using nix-shell to start a Qt application when none have been installed using nix may cause such failure.)

The current wrapper does not handle adding components to PATH so they will be searched.

ttuegel commented 5 years ago

So, there is at least an input method impurity...

This is intentional: we don't want for users to rebuild the world to change input methods. :slightly_frowning_face:

The current wrapper does not handle adding components to PATH so they will be searched.

Searching PATH components is another intentional impurity which predates the wrapQtAppsHook changes. However, this improves with the wrapper because fewer things need to have their plugins propagated into the system environment.

samueldr commented 5 years ago

This is intentional: we don't want for users to rebuild the world to change input methods. :slightly_frowning_face:

I figured, sorry if it seemed to imply otherwise, I was just stating the fact. Though I think the impurity is not "the input method is giving a full path to use as a library", but instead that it has to be resolved with the libraries path the software is given to work with. Sorry if that sentence is hard to parse, I'm still testing one final thing related to this.

Searching PATH components is another intentional impurity which predates the wrapQtAppsHook changes. However, this improves with the wrapper because fewer things need to have their plugins propagated into the system environment.

Yeah, I understand, but in real world use, it is part of the main issue, having mismatched Qt versions in the (overall) environment will break Qt apps.

After my test is done building, I'll share a, hopefully, good fix for the problem.

samueldr commented 5 years ago

65526 is likely to solve the mismatched minor versions for good.

peterzky commented 5 years ago

65543 fix qt5ct

teto commented 5 years ago

I've tried 2 things to make the problem disappear with wireshark but I still have the issue Cannot mix incompatible Qt library (version 0x50c00) with this library (version 0x50c03) when I run result/bin/wireshark. see the last 2 commits at https://github.com/teto/nixpkgs/tree/nixos-unstable

I have 2 other questions:

I believe fcitx might be the culprit; strace log https://transfer.sh/EsQHH/log , rebuilding...

teto commented 5 years ago

seems like it got fixed with my changes and that PR https://github.com/NixOS/nixpkgs/pull/65526

tadfisher commented 5 years ago

The logic in wrapQtAppsHook() seems iffy: the elif [ -h "$file" ] branch will never run for symlinks to files, because if [ -f "$file" ] returns success for these. A simple fix is to swap these branches so the -h test runs first.

skallinen commented 5 years ago

not sure if related, but qutebrowser stopped working a couple of days ago for me on unstable.

08:45:38 CRITICAL: This application failed to start because no Qt platform plugin could be initialized. Reinstalling the application may fix this problem.

Fatal Python error: Aborted

Current thread 0x00007f1c78a9ab80 (most recent call first):
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/lib/python3.7/site-packages/qutebrowser/misc/earlyinit.py", line 87 in _die
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/lib/python3.7/site-packages/qutebrowser/misc/earlyinit.py", line 222 in _check_modules
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/lib/python3.7/site-packages/qutebrowser/misc/earlyinit.py", line 238 in check_libraries
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/lib/python3.7/site-packages/qutebrowser/misc/earlyinit.py", line 296 in early_init
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/lib/python3.7/site-packages/qutebrowser/qutebrowser.py", line 190 in main
  File "/nix/store/jmy3swaz7ma3f3w6vfjcjm914rdlkqwn-qutebrowser-1.6.3/bin/..qutebrowser-wrapped-wrapped", line 11 in <module>
Aborted
MasseR commented 5 years ago

65710 fix for flameshot

worldofpeace commented 5 years ago

@FRidh I think it's good if this is pinned? There is a very large amount of reports coming in daily.

teto commented 5 years ago

it totally makes sense to pin it since the longterm fix broke - in the short term - many things.

worldofpeace commented 5 years ago

it totally makes sense to pin it since the longterm fix broke - in the short term - many things.

Indeed, I think this is an issue that needs as much attention as a ZHF one would need. Almost blocking release in fact.

xaverdh commented 5 years ago

An approximate list of packages called with qt5.callPackage that use stdenv.mkDerivation, generated by grepping through nixpkgs master, is: ../applications/misc/doomseeker/default.nix ../applications/graphics/fstl/default.nix ../applications/networking/browsers/otter/default.nix ../development/tools/analysis/snowman/default.nix ../development/libraries/gecode/3.nix ../development/libraries/gecode/default.nix ../os-specific/linux/i7z/default.nix ../os-specific/linux/v4l-utils/default.nix ../applications/audio/carla/default.nix ../applications/video/clipgrab/default.nix ../applications/radio/gqrx/default.nix ../applications/networking/instant-messengers/spectral/default.nix ../applications/radio/qsstv/default.nix ../applications/misc/cura/stable.nix ../applications/misc/cura/lulzbot.nix ../applications/misc/cura/plugins.nix ../applications/misc/vym/default.nix ../applications/radio/wsjtx/default.nix ../applications/graphics/scantailor/advanced.nix ../development/tools/minizinc/ide.nix Maybe the maintainers of these should be pinged?

vbgl commented 5 years ago

It seems that the wrap-qt-apps-hook only wraps ELF executables. How to fix a python program?

worldofpeace commented 5 years ago

@vbgl To wrap manually.

worldofpeace commented 5 years ago

wrapQtApp in postFixup.

knedlsepp commented 5 years ago

I'm using the following command to find the remaining packages that need fixing:

find . -type f -exec grep -q 'qt5' {} \; -exec grep -l 'stdenv.mkDerivation' {} +
knedlsepp commented 5 years ago

Something that is still not completely clear to me: Should all Qt applications be changed to not use stdenv.mkDerivation or only those that are broken by the most recent changes? I did try to run a few Qt-applications that still use stdenv.mkDerivation but I couldn't get them to crash, so I'm wondering if they are fine already or if my NixOS machine is in a state that by chance has the right qt versions? Do I need to first install some older version of qt to make them reliable crash?

samueldr commented 5 years ago

All Qt applications not wrapped should end up crashing in those situations. Some had ad-hoc fixes added with custom wrappers, though, which could mask issues. It's possible, though, that it doesn't end up crashing because there's no mismatched Qts or other environmental non-issues.

mmlb commented 5 years ago

I'm trying to update flent (a python application using pyqt) to latest release. I've got the output working by running wrapQtApp in postFixup, but this new version wants to try running the gui as part of the tests. I've skipped that test to verify postFixup thing works but I'd rather not drop that test. How can I do the equivalent for running buildPythonApplication tests?

FRidh commented 5 years ago

I think we should avoid custom builders when we can, because they don't allow for composition when different frameworks/languages are used together. This is also the reason why I am splitting buildPythonPackage into hooks.

@mmlb you might be able to move it to postInstall. That way it is wrapped before tests start.

mmlb commented 5 years ago

I think we should avoid custom builders when we can, because they don't allow for composition when different frameworks/languages are used together. This is also the reason why I am splitting buildPythonPackage into hooks.

makes sense to me

@mmlb you might be able to move it to postInstall. That way it is wrapped before tests start.

This didn't work. I'm only wrapping the packaged programs, but need to make python setup.py test be wrapped. I'm thinking about writing a custom checkPhase that runs a wrapped script.

xaverdh commented 5 years ago

I tried to fix notepadqq, but apparently the wrapper does not pick up the necessary plugins (no QT_PLUGIN_PATH). Any ideas what I am doing wrong? Resolved: The executable was a bash script.

mmlb commented 5 years ago

I'm thinking about writing a custom checkPhase that runs a wrapped script.

This worked. https://github.com/NixOS/nixpkgs/pull/66323

worldofpeace commented 5 years ago

Documented wrapQtAppsHook ignoring files that are not ELF headers #66325

ghost commented 5 years ago

@ttuegel

Do not import Qt package sets; the Qt versions of dependencies may not be coherent, causing build and runtime failures.

What does it mean? Could you provide some examples?

ttuegel commented 5 years ago

I updated the manual to show how to do this. I was uncomfortable demonstrating what not to do, but since you asked, the import line of packages should look like this:

{ qtbase, qtdeclarative }: /* et cetera */

and not like this:

{ qt5 }: /* or qt59, qt512, or anything else like that */

Therefore, your buildInputs should look like this:

{
  buildInputs = [ qtbase ];
}

and not like this:

{
  buildInputs = [ qt5.qtbase ];
}
ghost commented 5 years ago

@ttuegel Got it, thank you!

mixmixmix commented 5 years ago

@AndersonTorres: cutegram seems to suffer from the same issue.

oxij commented 5 years ago

Anki fixed in #66796.

jtojnar commented 5 years ago

Introducing wrapQtAppHook into qt5.mkDerivation pollutes a closure of non-Qt variants of packages using that deriver: https://github.com/NixOS/nixpkgs/pull/67134

aanderse commented 5 years ago

@MP2E dolphinEmuMaster is broken in master currently, requiring fixes as described by this issue. cc @marius851000 @ashkitten @delroth

ashkitten commented 5 years ago

taking a look, @aanderse

Ma27 commented 5 years ago

67244 bumps teamviewer, builds with the latest QT and uses the new QT hook.

ttuegel commented 5 years ago

Introducing wrapQtAppHook into qt5.mkDerivation pollutes a closure of non-Qt variants of packages using that deriver:

I would recommend selecting the deriver based on if the package is using Qt, i.e. (if withQt then mkDerivation else stdenv.mkDerivation) { ... }.

peterhoeg commented 5 years ago

There is an issue with variables added to qtWrapperArgs not being expanded properly.

In case of a gstreamer application, this doesn't work:

  qtWrapperArgs = [
    "--prefix GST_PLUGIN_SYSTEM_PATH_1_0 : $GST_PLUGIN_SYSTEM_PATH_1_0"
  ];

Where as this does:


postInstall = ''
  qtWrapperArgs+=(--prefix GST_PLUGIN_SYSTEM_PATH_1_0 : $GST_PLUGIN_SYSTEM_PATH_1_0)
'';
Shados commented 5 years ago

@ttuegel

Using stdenv.mkDerivation with Qt applications has always been unsupported.

Why, though? The reasoning behind this doesn't seem to be documented anywhere that I can see, and I'm wondering what the benefit is versus using a hook. The manual merely says:

mkDerivation is a wrapper around stdenv.mkDerivation which applies some Qt-specific settings.

And as far as I can tell from reading the Nix expression for it, the only thing it does aside from adding wrapQtAppsHook is configuring some debug flags based on an argument to the Qt package set. So I have some questions:

  1. Have I missed something that the custom deriver is doing?
  2. Is the per-set debug flag configuration something that can only be done by using a custom deriver...?
  3. Is the deriver approach more composable than the hook approach?
  4. Why is it called mkDerivation rather than something like mkQtDerivation? The difference between stdenv.mkDerivation and mkDerivation is non-obvious to someone who is reading a derivation expression without having also read the call site, and even then they still need to know that qt5.mkDerivation exists a priori.
lightbulbjim commented 5 years ago

Why is it called mkDerivation rather than something like mkQtDerivation? The difference between stdenv.mkDerivation and mkDerivation is non-obvious to someone who is reading a derivation expression without having also read the call site, and even then they still need to know that qt5.mkDerivation exists a priori.

Agree that the naming isn't ideal. It would be a bit of a pain to rename everything now though.

worldofpeace commented 5 years ago

Why is it called mkDerivation rather than something like mkQtDerivation? The difference between stdenv.mkDerivation and mkDerivation is non-obvious to someone who is reading a derivation expression without having also read the call site, and even then they still need to know that qt5.mkDerivation exists a priori.

Agree that the naming isn't ideal. It would be a bit of a pain to rename everything now though.

Indeed. Though if someone wanted to submit a proposal to change this it would be considered. (obviously after the branch off)

ttuegel commented 5 years ago

There is an issue with variables added to qtWrapperArgs not being expanded properly.

This may not be desirable, but it is expected that shell variables will not expand. Off hand, I don't know how (or when!) to expand them during the build.

ttuegel commented 5 years ago

And as far as I can tell from reading the Nix expression for it, the only thing it does aside from adding wrapQtAppsHook is configuring some debug flags based on an argument to the Qt package set. So I have some questions:

1. Have I missed something that the custom deriver is doing?

No, but the custom deriver reserves the right to claim more behaviors in the future.

2. Is the per-set debug flag configuration something that can only be done by using a custom deriver...?

3. Is the deriver approach more composable than the hook approach?

4. Why is it called `mkDerivation` rather than something like `mkQtDerivation`? The difference between `stdenv.mkDerivation` and `mkDerivation` is non-obvious to someone who is reading a derivation expression without having also read the call site, and even then they still need to know that `qt5.mkDerivation` exists a priori.

The answer to all these questions is roughly, "Because the largest package set I'm familiar with in Nixpkgs is haskellPackages, and this is how it works." That's not a great answer, but I figure we could do worse than to imitate a large, mature set of packages.

I'm open to switching to a hook-based architecture after NixOS 19.09. Whether we keep a custom deriver or switch to a hook, we should also start failing the build if the correct one isn't included.

vandenoever commented 5 years ago

I've a question regarding development of Qt applications on NixOS. I'll take the thing I'm attempting currently as an example.

I'd like to try out and possibly code on the current Kate master. To do so I've written a small nix expression:

{ pkgs ? import <nixpkgs> {} }:
with pkgs;
let
  kate = kdeApplications.kate.overrideAttrs (oldAttrs: rec {
    buildInputs = oldAttrs.buildInputs ++ [ ninja ripgrep vim strace ];
  });
in
  kate

This extends the buildInputs for Kate with some development tools. I enter the dev environment with nix-shell --pure. The --pure is there to avoid clashing with my installed version of kate.

However, the unit tests (ctest -V) fail with the infamous qt.qpa.plugin: Could not find the Qt platform plugin "xcb" in "".

In this environment, QT_PLUGIN_PATH, QT_QPA_PLATFORM and QT_QUICK_CONTROLS_STYLE are not set. HOME, DISPLAY, XDG_CONFIG_DIRS and XDG_CONFIG_DIRS are not set. Setting the QT_* variables does not solve the problem.

Is this the right way to enter a dev environment for developing Qt applications? I'm especially keen on reusing the NixPkgs definitions of the applications.

Edit: This command does work:

PATH=$PATH:/run/current-system/sw/bin ctest -V

Is there a way to add that to the environment in the above Nix expression?