compilers/ghc/common-hadrian: expand flags for structuredAttrs#472517
compilers/ghc/common-hadrian: expand flags for structuredAttrs#472517SFrijters wants to merge 1 commit intoNixOS:stagingfrom
Conversation
|
Commit message should probably read |
eead43f to
68c9b2e
Compare
|
I based the commit message prefix on a previous commit that touched the same file, but I've updated this one to something better. |
|
|
||
| # We need to go via the bindist for installing | ||
| hadrian $hadrianFlags "''${hadrianFlagsArray[@]}" binary-dist-dir | ||
| hadrian "''${hadrianFlags[@]}" "''${hadrianFlagsArray[@]}" binary-dist-dir |
There was a problem hiding this comment.
We need those to be space split.
| hadrian "''${hadrianFlags[@]}" "''${hadrianFlagsArray[@]}" binary-dist-dir | |
| hadrian ''${hadrianFlags[@]} "''${hadrianFlagsArray[@]}" binary-dist-dir |
There was a problem hiding this comment.
Done, Stage1 still successfully starts building for __structuredAttrs.
There was a problem hiding this comment.
One of the main selling points of structuredAttrs is the "native" support for spaces - so this change seems very much backwards to me.
Instead, we should add the quotes back and then enable __structuredAttrs = true for this file. This will give us the correct behavior consistently.
There was a problem hiding this comment.
That would be my preferred solution also, but then I think we should also revisit #470435 for consistency, where setting __structuredAttrs = true was not welcomed.
There was a problem hiding this comment.
Also, we'll run into the same discussion for generic-builder.nix in haskell-modules.
There was a problem hiding this comment.
That would be my preferred solution also, but then I think we should also revisit #470435 for consistency, where setting
__structuredAttrs = truewas not welcomed.
The difference in that PR was, that the code would behave correctly both with and without structuredAttrs. The "consistent" pattern here is: Only add _structuredAttrs = true when it actually makes a difference. It does here, because we can't easily treat both cases. Ofc it's possible, but we'd have to add the same code as in many of the setup-hooks with concatTo etc. - but that'd be overkill for here, where we actually control the derivation. Thus, __structuredAttrs = true is the preferred solution here, from my perspective.
There was a problem hiding this comment.
Also, we'll run into the same discussion for
generic-builder.nixinhaskell-modules.
We will, but since the builder is used for many more different packages, it might make sense to just implement it in a more robust way, that supports both cases, similar to the setup hooks I mentioned earlier.
68c9b2e to
2f79de9
Compare
|
To be replaced by #475546 . |
Otherwise we lose all flags apart from the first.
This results in a
__structuredAttrs = true;build trying to find xelatex while it is not available.Things done
passthru.tests.nixpkgs-reviewon this PR. See nixpkgs-review usage../result/bin/.Add a 👍 reaction to pull requests you find important.