setup.sh: avoid running the same hook twice #49552
Conversation
|
OH! it only applies in the unstrict deps case. Perfect! I guess all I'd ask for is more comments, explaining it's intentionally only for strictDeps=false / adding another TODO remove to match. |
|
Any update on getting this into staging? I've been running into #41340 on darwin and it would be really nice to have it finally fixed! |
|
@cdepillabout I don't think this will fix your issue. The main Haskell issue was fixed in f375825. I would make sure your nixpkgs version is correct and all of your channels are up to date. |
|
I don't think this solves #27533, right? Seems related |
|
No it won't fix that issue. But it shouldn't be too hard to fix... |
In strictDeps=false, we don’t want the same package hook to be run twice, otherwise we get duplicates in some flags. Fixes NixOS#41340
matthewbauer
force-pushed
the
setup-dedupe
branch
from
2bac916
to
9172c1e
Compare
| @@ -562,6 +562,10 @@ _addToEnv() { | |||
| (( "$depHostOffset" <= "$depTargetOffset" )) || continue | |||
| local hookRef="${hookVar}[$depTargetOffset - $depHostOffset]" | |||
| if [[ -z "${strictDeps-}" ]]; then | |||
|
|
|||
| # Keep track of which packages we have visited before. | |||
| local visitedPkgs="" | |||
There was a problem hiding this comment.
(I'm not an expert, but would it be better to use an array instead of building/scanning a string? IIRC there are some issues with some shell features so maybe it's a bad idea for those or other reasons)
There was a problem hiding this comment.
Yeah that might have been a better idea… I am kind of wanting to just fix the argmax issue as it seems to still be happening. Making this more performant would definitely be better.
| @@ -574,7 +578,11 @@ _addToEnv() { | |||
| ${pkgsHostTarget+"${pkgsHostTarget[@]}"} \ | |||
| ${pkgsTargetTarget+"${pkgsTargetTarget[@]}"} | |||
| do | |||
| if [[ "$visitedPkgs" = *"$pkg"* ]]; then | |||
There was a problem hiding this comment.
Is this susceptible to false matches when package is a substring of another?
Oh, nevermind, these are probably store paths so that can't happen, right? :)
There was a problem hiding this comment.
Yeah I forgot to leave in the spaces here
In strictDeps=false, we don’t want the same package hook to be run
twice, otherwise we get duplicates in some flags.
Fixes #41340