Skip to content

keepassxc: add 'wrapGAppsHook' to fix FileChooser error (see also: https://github.com/NixOS/nixpkgs/issues/111368)#140470

Closed
balazs-lengyel wants to merge 2 commits intoNixOS:masterfrom
balazs-lengyel:KeePassXC
Closed

keepassxc: add 'wrapGAppsHook' to fix FileChooser error (see also: https://github.com/NixOS/nixpkgs/issues/111368)#140470
balazs-lengyel wants to merge 2 commits intoNixOS:masterfrom
balazs-lengyel:KeePassXC

Conversation

@balazs-lengyel
Copy link

Motivation for this change

KeePassXC failed creating a DB with GLib-GIO-ERROR **: 11:16:48.748: Settings schema 'org.gtk.Settings.FileChooser' is not installed.

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.

@ofborg ofborg bot requested review from jonafato and turion October 4, 2021 11:20
@ofborg ofborg bot added 10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. labels Oct 4, 2021
@sugar700
Copy link
Member

sugar700 commented Oct 4, 2021

Huh, that's interesting, KeepassXC is not a GTK application, however apparently it's possible to configure Qt applications to use GTK with QT_QPA_PLATFORMTHEME=gtk3. That has interesting implications for all Qt applications in Nixpkgs.

@sugar700
Copy link
Member

sugar700 commented Oct 4, 2021

Seems like #136071 could be related, cc @jtojnar.

'';

nativeBuildInputs = [ asciidoctor cmake wrapQtAppsHook qttools pkg-config ];
nativeBuildInputs = [ asciidoctor cmake wrapGAppsHook wrapQtAppsHook qttools pkg-config ];
Copy link
Member

Choose a reason for hiding this comment

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

This double wraps the binary if I am not mistaken. Please add the wrapper arguments to wrapperArgs.

Copy link
Author

Choose a reason for hiding this comment

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

Copy link
Member

Choose a reason for hiding this comment

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

wrapProgram $out/bin/tev \

@turion
Copy link
Contributor

turion commented Oct 4, 2021

What issue did this fix? I've never heard of this problem.

@balazs-lengyel
Copy link
Author

@turion keepassxc crashes on selecting the location for the new DB file under a fresh nixos+gnome install.

@turion
Copy link
Contributor

turion commented Oct 5, 2021

That explains why it doesn't have any problems in KDE.

'';

nativeBuildInputs = [ asciidoctor cmake wrapQtAppsHook qttools pkg-config ];
nativeBuildInputs = [ asciidoctor cmake wrapGAppsHook wrapQtAppsHook qttools pkg-config ];
Copy link
Member

Choose a reason for hiding this comment

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

wrapProgram $out/bin/tev \


preFixup = optionalString stdenv.isDarwin ''
preFixup = ''
qtWrapperArgs+=("''${gappsWrapperArgs[@]}")
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure if this is enough.

@turion
Copy link
Contributor

turion commented Oct 8, 2021

Why is double wrapping a problem? To me it looks like a more maintainable and clear solution.

@balazs-lengyel
Copy link
Author

I agree with @turion that double-wrapping should be the way to go, since it is the most maintainable solution. Should I change this back?

@jtojnar
Copy link
Member

jtojnar commented Oct 21, 2021

Double wrapping is a problem because Linux does not support passing setting argv[0] when execing a script. That means argv[0] will contain .program-wrapped, which will likely get used in --help at best, and prevent the program from finding its resources at worst.

@stale
Copy link

stale bot commented Apr 19, 2022

I marked this as stale due to inactivity. → More info

@stale stale bot added the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 19, 2022
@turion
Copy link
Contributor

turion commented Apr 22, 2022

Double wrapping is a problem because Linux does not support passing setting argv[0] when execing a script. That means argv[0] will contain .program-wrapped, which will likely get used in --help at best, and prevent the program from finding its resources at worst.

Ah, ok. Quite a few (~ 10) programs in nixpkgs do this, though. Since this seems like a popular thing to do, and they all do it wrong, allegedly, maybe there should be a standard "double wrapper" wrapQtAndGAppsHook containing both wrapping args.

@stale stale bot removed the 2.status: stale https://github.com/NixOS/nixpkgs/blob/master/.github/STALE-BOT.md label Apr 22, 2022
@Artturin
Copy link
Member

can't repro the error on master XDG_DATA_DIRS= QT_QPA_PLATFORMTHEME=gtk3 ./result/bin/keepassx

@Artturin Artturin closed this Apr 22, 2022
@turion
Copy link
Contributor

turion commented Apr 22, 2022

This is my fault. I added double wrapping in #163608, not realizing that there are downsides.

For keepassxc at least, everything works sort of as expected:

$ keepassxc --help
Usage: /nix/store/bsn1wir6x2jiin6jdp3jzl4ja98b2yxn-keepassxc-2.6.6/bin/.keepassxc-wrapped [options] [filename(s)]
KeePassXC - cross-platform password manager

Options:
  -h, --help                   Displays help on commandline options.
  --help-all                   Displays help including Qt specific options.
  -v, --version                Displays version information.
  --config <config>            path to a custom config file
  --localconfig <localconfig>  path to a custom local config file
  --lock                       lock all open databases
  --keyfile <keyfile>          key file of the database
  --pw-stdin                   read password of the database from stdin
  --debug-info                 Displays debugging information.

Arguments:
  filename(s)                  filenames of the password databases to open
                               (*.kdbx)

@Janik-Haag Janik-Haag added the 12.first-time contribution This PR is the author's first one; please be gentle! label Jun 12, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

10.rebuild-darwin: 1-10 This PR causes between 1 and 10 packages to rebuild on Darwin. 10.rebuild-darwin: 1 This PR causes 1 package to rebuild on Darwin. 10.rebuild-linux: 1-10 This PR causes between 1 and 10 packages to rebuild on Linux. 10.rebuild-linux: 1 This PR causes 1 package to rebuild on Linux. 12.first-time contribution This PR is the author's first one; please be gentle!

Projects

None yet

Development

Successfully merging this pull request may close these issues.

7 participants