wrapQtAppsHook: support structuredAttrs

[MattSturgeon]

Use stdenv's concatTo helper to normalize qtWrapperArgs, instead of assuming a single string representation. This handles both structuredAttrs modes: when __structuredAttrs = true, qtWrapperArgs is already an array.

Tests are added to cover both structuredAttrs modes and verify that wrapper behaviour remains identical across both configurations, including argument propagation and runtime behaviour.

Fixes #526275, which is blocking #461881.

AI assistance disclosure

• I used ChatGPT to review the patch and help iterate on test structure and coverage.
• The final implementation and decisions were made and validated manually.

Things done

• Built on platform:
  • x86_64-linux
  • aarch64-linux
  • x86_64-darwin
  • aarch64-darwin
• Tested, as applicable:
  • NixOS tests in nixos/tests.
  • Package tests at passthru.tests.
  • Tests in lib/tests or pkgs/test for functions and "core" functionality.
    • nix-build -A tests.qt5 -A tests.qt6
• Ran nixpkgs-review on this PR. See nixpkgs-review usage.
• Tested basic functionality of all binary files, usually in ./result/bin/.
• Nixpkgs Release Notes
  • Package update: when the change is major or breaking.
• NixOS Release Notes
  • Module addition: when adding a new NixOS module.
  • Module update: when the change is significant.
• Fits CONTRIBUTING.md, pkgs/README.md, maintainers/README.md and other READMEs.
• Follows the automation/AI policy.

[MattSturgeon]

I'd like to include a test, does anyone know where this hook's tests live?

Drafting for now as the fix may be incomplete: #461881 (comment)

EDIT: I couldn't find any existing qt hook tests, so I added passthru tests and exposed them to the top-level in pkgs/test/default.nix.

[K900]

Why do the strings dance instead of just sourcing the hook and printing out qtWrapperArgs?

[MattSturgeon]

Why do the strings dance instead of just sourcing the hook and printing out qtWrapperArgs?

Context

When I was iterating on the fix locally, I needed a "full" integration test with makeCWrapper, as my initial approach "fixed" the shell variables in the hook but resulted in args not being passed to makeCWrapper.

I wrote the test to align with my manual debugging method (inspecting the actual wrapper). And the test caught that what I thought was an internal qtWrapperArgs -> qtWrapperArgsArray copy was actually a breaking change.

Sourcing the hook and printing out qtWrapperArgs would catch that too, you're right. And in hindsight, now that I know qtWrapperArgsArray would be a breaking change it'd be much simpler to just read qtWrapperArgs, yes.

It would be slightly less coverage, in that it only asserts the expected array is produced; it wouldn't test that the array is used to produce the expected wrapper.

Even if we kept a strings-based test, checking the expected array is constructed would also be a valuable test in its own right.

[MattSturgeon]

I've re-thought the test coverage and removed the strings-based executable inspection. Thanks for the feedback @K900!

The test coverage is now structured as:

1. Verify that the expected qtWrapperArgs array is produced by wrapQtAppsHook
2. Verify runtime behaviour: the wrapped program observes the expected effects
   • (working directory, environment variables such as QT_PLUGIN_PATH, and injected wrapper arguments)
3. Build both cases with __structuredAttrs = true and false

This preserves coverage of both the hook's argument handling and its effect on the final wrapped executable, without relying on inspecting the generated binary wrapper.

The patches to wrapQtAppsHook itself are unchanged; only the test strategy has been simplified.

[MattSturgeon]

I pushed a further simplification to the tests: https://github.com/NixOS/nixpkgs/compare/f2cb7c5d4a144bcf9026d5c1ce232a7cedd1b5fa..0c0980f13122c07211ca0c6ad4992a162f62bcab

It turns out qtWrapperArgs is processed when the hook is sourced, before the wrapQtAppsHook function itself is invoked by any stdenv phase.

Therefore, only the runtime integration test needs to call wrapQtAppsHook. The qtWrapperArgs unit test has been simplified to only exercise the hook's setup logic, which is where qtWrapperArgs is setup & processed.

[khaneliman]

LGTM tests pass on darwin now

[K900]

FWIW you can avoid the C compile dance by just explicitly doing wrapQtApp on a bash script or whatever.

[MattSturgeon]

I started off with a bash script, but it didn't seem to be getting wrapped. I assumed the hook was only wrapping binary executables?

Edit: looks like that's intentional:

nixpkgs/pkgs/development/libraries/qt-6/hooks/wrap-qt-apps-hook.sh

Lines 92 to 93 in 4346a1c

	# Skip the file if it is not a binary (ELF or Mach-O)
	isELF "$file" || isMachO "$file" || continue

EDIT

FWIW you can avoid the C compile dance by just explicitly doing wrapQtApp on a bash script or whatever.

That's true, although my goal with the runtime test was to exercise the normal nativeBuildInputs = [ wrapQtAppsHook ] workflow end-to-end, rather than wrapQtApp in isolation.

I think the C compiled fixture is a reasonable trade-off here, as the hook's main entrypoint filters for ELF binaries.