duckstation: unstable-2023-09-30 -> 0.1-6292

[SuperSamus]

Description of changes

Supersedes #273855.

Things done

• Built on platform(s)
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• For non-Linux: Is sandboxing enabled in nix.conf? (See Nix manual)
  • sandbox = relaxed
  • sandbox = true
• Tested, as applicable:
  • NixOS test(s) (look inside nixos/tests)
  • and/or package tests
  • or, for functions and "core" functionality, tests in lib/tests or pkgs/test
  • made sure NixOS tests are linked to the relevant packages
• Tested compilation of all packages that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage
• Tested basic functionality of all binary files (usually in ./result/bin/)
• 24.05 Release Notes (or backporting 23.05 and 23.11 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.

---

Add a 👍 reaction to pull requests you find important.

[AndersonTorres]

Now Wayland support is mandatory?

[SuperSamus]

Now Wayland support is mandatory?

I copied upstream:

https://github.com/NixOS/nixpkgs/blob/29972cdbc2ad70c7db8a1abf523c771aa8465f82/pkgs/applications/emulators/duckstation/default.nix#L107-L113

Same thing of PCSX2:

nixpkgs/pkgs/applications/emulators/pcsx2/default.nix

Lines 114 to 120 in b0d36bd

	# https://github.com/PCSX2/pcsx2/pull/10200
	# Can't avoid the double wrapping, the binary wrapper from qtWrapperArgs doesn't support --run
	postFixup = ''
	source "${makeWrapper}/nix-support/setup-hook"
	wrapProgram $out/bin/pcsx2-qt \
	--run 'if [[ -z $I_WANT_A_BROKEN_WAYLAND_UI ]]; then export QT_QPA_PLATFORM=xcb; fi'
	'';

I removed enableWayland because having both would be a mess.

[AndersonTorres]

Waiting mergers

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-ready-for-review/3032/3204

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/1353

[AndersonTorres]

Was this yet another mere hash update?
Next time it happens, please do not force-push, just keep the previous commit.

[SuperSamus]

Is there any reason why this hasn't been merged yet?

[AndersonTorres]

Mergers busy merging other things...

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/1387

[SuperSandro2000]

It should be possible to overwrite the makeWrapper used back to the none binary one

[SuperSamus]

Doing this would increase startup time, though.
So... which one is less bad? Double wrapping, or longer startup time?
(Also, if I do the latter, it should be changed on PCSX2 too for consistency)

[AndersonTorres]

I prefer no double-wrapping.

How much the startup would be affected?

[SuperSamus]

See #225881.
(Looking at the that... how am I supposed to revert it? Overriding propagatedBuildInputs?)

[AndersonTorres]

The speedup is huge, two orders of magnitude! Let's stay with the double wrapping, until someone writes the Best Wrappers Ever (using JSON or other eerie magic).

[SuperSandro2000]

Double wrapping is really bad as the program thinks that it was started as .name-wrapped which really is the name of the first wrapper after the other one was applied on top.

Also instead of adding a new magic variable which does things, why can't we tell people to just export QT_QPA_PLATFORM=xcb for duckstation? That would avoid the problem entirely.

[SuperSamus]

So, I'll reiterate the points:

• This wrapping is done to match upstream's behavior. The AppImages have Wayland as an opt-in (and not opt-out) because they think that the Wayland experience on PCSX2/DuckStation isn't good enough for it to be the default. See https://github.com/PCSX2/pcsx2/pull/10179. It is important to respect this when making a package of their program.
• So, if I have to do the above while not double wrapping, then I need to "revert" write-qt-apps-hook.sh: use make-binary-wrapper for significant speedups #225881's behavior (that is, make wrapQtAppsHook use makeWrapper instead of makeBinaryWrapper). I don't know how to do that.

_{^{Also, no one said anything when I did the same thing on PCSX2.}}

[AndersonTorres]

I read the small words.
And, seriously, the "no one said this to me before" is a bad argument, especially in a codebase with sudden changes like Nixpkgs.

Also instead of adding a new magic variable which does things, why can't we tell people to just export QT_QPA_PLATFORM=xcb for duckstation? That would avoid the problem entirely.

I think it would be a serious problem of usability. Yes, we Nixpkgs users are usually a well-informed people, however things like "extra preparation steps" are too much to require, since

1. Other distros rarely worry about those things
2. export variables can affect more than the running program, breaking one of the main flagships of Nixpkgs - the sandbox
3. it is easy to forget to undo the exports, or even why they were made in the first place

[pbsds]

Do you consider this thread blocking? Would a simple s/I_WANT_A_BROKEN_WAYLAND_UI/DUCKSTATION_I_WANT_A_BROKEN_WAYLAND_UI/ suffice?

[AndersonTorres]

As the maintainer of this package, everything LGTM.

The problems of this expression are external to it.

fixed

[nixos-discourse]

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

https://discourse.nixos.org/t/prs-already-reviewed/2617/1459