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.

Sign up for GitHub

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

Jump to bottom

kwallet-pam: just provide plugin path for qtbase #84689

Merged
merged 1 commit into from Apr 9, 2020
Merged

kwallet-pam: just provide plugin path for qtbase #84689

merged 1 commit into from Apr 9, 2020

Conversation

matthewbauer
Copy link
Member

@matthewbauer matthewbauer commented Apr 8, 2020 •
edited

kwallet sets a limit of 1000 for a single characters for environment
variables read from the socket[1]. wrapQtApps gives us a huge value
for QT_PLUGIN_PATH (up to 13000 bytes on my system!) Since this was
overflowing, the Qt plugin loading mechanism was hitting a segfault
when it was trying to parse the truncated QT_PLUGIN_PATH.

So for now, we can just unset QT_PLUGIN_PATH in the pam_kwallet_init
script. kwalletd5 has its own QT_PLUGIN_PATH which it can use.

This problem occured on 20.03, but not 19.09. It’s unclear what
changes were made in that time, but likely that previously we weren’t
getting a QT_PLUGIN_PATH set in the plasma5 startup at all. This means
that in 19.09 our QT_PLUGIN_PATH value must have been small enough to
fit into the 1000 char limit.

Fixes #77290

/cc @ttuegel

@matthewbauer matthewbauer mentioned this pull request Apr 8, 2020
Closed
@matthewbauer matthewbauer requested a review from mrkkrp April 8, 2020 05:19
@ttuegel
Copy link
Member

ttuegel commented Apr 8, 2020

So for now, just get a QT_PLUGIN_PATH value for qtbase, providing the
xcb plugin which is the one we need. XDG_DATA_DIRS appears to be
unnecessary.

Let's also add qtwayland, otherwise I expect it will break for our Wayland users.

@mrkkrp
Copy link
Member

mrkkrp commented Apr 8, 2020

The build fails for me:

Builder called die: makeWrapper doesn't understand the arg QT_PLUGIN_PATH
Backtrace:
105 makeWrapper /nix/store/akb1l64z5f8w0k473zzwyvc5y6jlaqks-hook/nix-support/setup-hook
145 wrapProgram /nix/store/akb1l64z5f8w0k473zzwyvc5y6jlaqks-hook/nix-support/setup-hook
81 _callImplicitHook /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
98 _eval /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
41 runHook /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
1189 fixupPhase /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
1299 genericBuild /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
2 main /nix/store/9krlzvny65gdc8s7kpb6lkx8cd02c25b-default-builder.sh

@worldofpeace
Copy link
Contributor

But please add a comment why we're doing this. Someone could easily look at this and be like "well this needs to be updated for wrapQtApp` and break it again because we have no testing.

@matthewbauer
Copy link
Member Author

The build fails for me:

Builder called die: makeWrapper doesn't understand the arg QT_PLUGIN_PATH
Backtrace:
105 makeWrapper /nix/store/akb1l64z5f8w0k473zzwyvc5y6jlaqks-hook/nix-support/setup-hook
145 wrapProgram /nix/store/akb1l64z5f8w0k473zzwyvc5y6jlaqks-hook/nix-support/setup-hook
81 _callImplicitHook /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
98 _eval /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
41 runHook /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
1189 fixupPhase /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
1299 genericBuild /nix/store/izwfky32fcnz4gg9vj6yl35x0x4x2g4v-stdenv-linux/setup
2 main /nix/store/9krlzvny65gdc8s7kpb6lkx8cd02c25b-default-builder.sh

Updated with a fix for this - I was just passing in wrong args to wrapProgram.

@matthewbauer matthewbauer force-pushed the fix-77290 branch 2 times, most recently from 33ba091 to 8021f0a Compare April 8, 2020 20:27
@worldofpeace worldofpeace added the 9.needs: port to stable A PR needs a backport to the stable release. label Apr 8, 2020
@Mindavi
Copy link
Contributor

Mindavi commented Apr 9, 2020 •
edited

This doesn't build for me.

The build error 
-- Could NOT find KF5Wallet (missing: KF5Wallet_DIR)
-- Could NOT find KF5Wallet: found neither KF5WalletConfig.cmake nor kf5wallet-config.cmake
CMake Error at /nix/store/cmriqbxnpcgmbll06pnl1h3va08x2xvr-cmake-3.16.5/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:146 (message):
  Could NOT find KF5 (missing: Wallet) (Required is at least version
  "5.62.0")
Call Stack (most recent call first):
  /nix/store/cmriqbxnpcgmbll06pnl1h3va08x2xvr-cmake-3.16.5/share/cmake-3.16/Modules/FindPackageHandleStandardArgs.cmake:393 (_FPHSA_FAILURE_MESSAGE)
  /nix/store/hxvvgjkvpp22g1rykzcb734649sgzjz0-extra-cmake-modules-5.66.0/share/ECM/find-modules/FindKF5.cmake:110 (find_package_handle_standard_args)
  CMakeLists.txt:38 (find_package)


-- Configuring incomplete, errors occurred!
See also "/build/kwallet-pam-5.17.5/build/CMakeFiles/CMakeOutput.log".
builder for '/nix/store/znxzdhm9hplgijnwgisj95khf5g88kpd-kwallet-pam-5.17.5.drv' failed with exit code 1
cannot build derivation '/nix/store/shahgnysbssvysmza9fvh5rrb75gg9ih-gdm.pam.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/rg1lr50hdf3i1pjih6phsiqazsyky3vb-kdm.pam.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/18swwxbj8hhdrpsydkk7gaplqnwd6vf4-lightdm.pam.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/0sbrpqyyw86zy3h4d38qy7v0c0axq6qx-sddm.pam.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/vhin09vlcsyqd1cw2s3bm01zd0hjsjd1-system-path.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/mi6dhdjrgl4nzmqigag91p4lspck371p-dbus-1.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/w6hk1qksyi43w55647m5wkppdkp8r13z-unit-polkit.service.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/3idhgmb8w406ksn93fg12m2v6kjjbfww-unit-systemd-fsck-.service.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/833injq3r8l3lgx33k9hn01aja76sm51-unit-dbus.service.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/bn5zxj0ndac4bpxaxln8pia969k98clc-unit-dbus.service.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/h9c7zhvw3r6z8y5g1s9bsc3052yli21y-system-units.drv': 3 dependencies couldn't be built
cannot build derivation '/nix/store/y8dl7l74qc50jb7k828vgy6q8l5rfwyn-user-units.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/iggcv5dx7vs4085hf131lqh72vrbsnv3-etc.drv': 8 dependencies couldn't be built
cannot build derivation '/nix/store/rk4pl3c261hn73rivvcknbi5dl8ghc75-nixos-system-nixos-testing-20.09pre-git.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/ckjsj74a7mdp8jjqcibfn7xxs7gscnv6-closure-info.drv': 1 dependencies couldn't be built
cannot build derivation '/nix/store/jxlfrs62yaaxv4x210wisiy5jrxnfphn-run-nixos-vm.drv': 2 dependencies couldn't be built
cannot build derivation '/nix/store/avcbdpw6byq55jw7pkwxs4zx2390xzph-nixos-vm.drv': 2 dependencies couldn't be built
error: build of '/nix/store/avcbdpw6byq55jw7pkwxs4zx2390xzph-nixos-vm.drv' failed

However, putting back the kwallet dependency in buildInputs helps resolving this, and also fixes the coredump. I'm not sure why it was removed here, but it seems like putting it back and leaving the rest alone resolves the linked issue.

FYI: I guess that's also why the grahamcofborg builds fail.

@matthewbauer
kwallet-pam: unset QT_PLUGIN_PATH
f0db4de 
kwallet sets a limit of 1000 for a single characters for environment
variables read from the socket[1]. wrapQtApps gives us a huge value
for QT_PLUGIN_PATH (up to 13000 bytes on my system!) Since this was
overflowing, the Qt plugin loading mechanism was hitting a segfault
when it was trying to parse the truncated QT_PLUGIN_PATH.

So for now, we can just unset QT_PLUGIN_PATH in the pam_kwallet_init
script. kwalletd5 has its own QT_PLUGIN_PATH which it can use.

This problem occured on 20.03, but not 19.09. It’s unclear what
changes were made in that time, but likely that previously we weren’t
getting a QT_PLUGIN_PATH set in the plasma5 startup at all. This means
that in 19.09 our QT_PLUGIN_PATH value must have been small enough to
fit into the 1000 char limit.

Fixes NixOS#77290

[1]: https://github.com/KDE/kwallet/blob/bc9713e2725ab1c4311866f751c674a38584bd92/src/runtime/kwalletd/main.cpp#L44

/cc @ttuegel
@matthewbauer matthewbauer merged commit 55e8250 into NixOS:master Apr 9, 2020
@nixos-discourse
Copy link

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

https://discourse.nixos.org/t/declarative-wrappers-another-idea/6661/1

@doronbehar doronbehar mentioned this pull request Apr 12, 2020
Closed
10 tasks
@doronbehar doronbehar mentioned this pull request Aug 7, 2021
Draft
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@worldofpeace worldofpeace worldofpeace approved these changes

@ttuegel ttuegel Awaiting requested review from ttuegel ttuegel is a code owner

@mrkkrp mrkkrp Awaiting requested review from mrkkrp

Assignees
No one assigned
Labels
9.needs: port to stable A PR needs a backport to the stable release. 10.rebuild-darwin: 0 10.rebuild-linux: 0
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

KDE wallet crashes on start (plasma5)
6 participants
@matthewbauer @ttuegel @mrkkrp @worldofpeace @Mindavi @nixos-discourse