haskell-generic-builder: allow passing flags to the test suite(s)#126364
haskell-generic-builder: allow passing flags to the test suite(s)#126364maralorn merged 1 commit intoNixOS:haskell-updatesfrom
Conversation
Every flag the generic builder receives via `testFlags` is passed via `--test-option` [1] to `Setup.hs` which in turn passes them to the underlying test suite binary. These wrapped options are added to `checkFlagsArray` in `checkPhase`. This needs to be done in bash since without structuredAttrs in nixpkgs so far, Nix arrays aren't properly translated into bash arrays, so we'd have all sorts of quoting issues when spaces are involved. Re-using `checkFlags` and `checkFlagsArray` from standard stdenv setup.sh also results in an additional feature: Using `overrideAttrs` `checkFlags` and `checkFlagsArray` can additionally be overridden, which allows passing extra flags to `Setup.hs` whithout being wrapped with `--test-option`. [1]: See also https://cabal.readthedocs.io/en/3.4/setup-commands.html?highlight=test-option#cmdoption-runhaskell-Setup.hs-test-test-option According to the cabal-install man page this also allows passing special variables which are substituted for other values depending on context.
$ nix-build -A haskellPackages.libarchive -A haskellPackages.alpaca-netcode
/nix/store/14wim3ssk7bg757wy1x9w68y0j6hwf3p-libarchive-3.0.2.2
/nix/store/815491m7rc8f58kwnzd9lw6msrx55vqj-alpaca-netcode-0.1.0.0 |
|
I’ll have to trust your bash magic here. But I don‘t see anything wrong with this. |
I'm fairly confident it's fine. It is mostly based on what We may need to wait with merging until your 2 weeks though as we are kind of starved for builds, it seems. |
| , testDepends ? [], testHaskellDepends ? [], testSystemDepends ? [], testFrameworkDepends ? [] | ||
| , benchmarkDepends ? [], benchmarkHaskellDepends ? [], benchmarkSystemDepends ? [], benchmarkFrameworkDepends ? [] | ||
| , testTarget ? "" | ||
| , testTarget ? "", testFlags ? [] |
There was a problem hiding this comment.
Since the arguments aren't documented here, I guess you could add a helper function in haskell.lib and add documentation and an example for this argument.
Although I don't think that is strictly necessary for this PR to be merged, more of a nice-to-have.
I also don't know bash well enough to work out if the checkFlagsArray expression is correct, but it looks like it at least works for your two examples, so it looks good to me!
There was a problem hiding this comment.
I'm a bit sceptical that it is actually a good thing having all these functions for specific arguments. In some cases they are justified when there is an element of “doing the right thing” to it (like appendConfigureFlags, …) and when the overrides are more complicated yet reusable (generateOptparseApplicativeCompion, justStaticExecutables, ...).
In this case overrideCabal is perfectly fine for setting this and my intention is to document this in the manual, I have already documented all existing arguments to haskellPackages.mkDerivation (which was apparently needed as I had to check the cabal2nix source for some details…) and would add testFlags there as well when this is merged.
There was a problem hiding this comment.
I was thinking that ideally this documentation could go here in pkgs/development/haskell-modules/generic-builder.nix, immediately above the place where it is defined.
Although it would be good if we could generate the manual directly from this file.
There was a problem hiding this comment.
This as well, but I think the actual manual is actually more important at this time, I'm hoping nixdoc will get some love in the future to support this use case as well (it kinda does currently, but also kinda doesn't).
testFlags was introduced in NixOS/nixpkgs#126364 and has been available since NixOS 21.11. Future work is removing these PostProcessing rules altogether, see #504.
testFlags was introduced in NixOS/nixpkgs#126364 and has been available since NixOS 21.11. Future work is removing these PostProcessing rules altogether, see #504.
Every flag the generic builder receives via
testFlagsis passed via--test-option[1] toSetup.hswhich in turn passes them to theunderlying test suite binary. These wrapped options are added to
checkFlagsArrayincheckPhase. This needs to be done in bash sincewithout structuredAttrs in nixpkgs so far, Nix arrays aren't properly
translated into bash arrays, so we'd have all sorts of quoting issues
when spaces are involved.
Re-using
checkFlagsandcheckFlagsArrayfrom standard stdenvsetup.sh also results in an additional feature: Using
overrideAttrscheckFlagsandcheckFlagsArraycan additionally be overridden,which allows passing extra flags to
Setup.hswhithout being wrappedwith
--test-option.[1]: See also https://cabal.readthedocs.io/en/3.4/setup-commands.html?highlight=test-option#cmdoption-runhaskell-Setup.hs-test-test-option
According to the cabal-install man page this also allows passing
special variables which are substituted for other values
depending on context.
Motivation for this change
Add a clean way (without abusing
testTarget) to pass extra flags to test suites and/or./Setup test.Things done
sandboxinnix.confon non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"./result/bin/)