New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
guid: fix compilation of Setup.hs and disable tests #88754
guid: fix compilation of Setup.hs and disable tests #88754
Conversation
@@ -750,4 +750,9 @@ self: super: builtins.intersectAttrs super { | |||
''; | |||
}); | |||
|
|||
# Fix compilation of Setup.hs and disable tests since it's empty | |||
guid = overrideCabal (super.guid) (drv: { | |||
prePatch = "sed -i '1d' Setup.hs"; |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
What is this doing?
Also, is this an upstream problem? If so, please report it upstream.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This removes the module declaration at the first line of Setup.hs.
Upstream already notified about it but unresponsive. This package is dependency of many packages in haskell ecosystem and I'm trying to fix it to use nix in unison compiler source.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This removes the module declaration at the first line of Setup.hs
Could you add a comment about this right above the line 755? It should be obvious to anyone why sed
is being used, and what it is doing, even if they haven't personally looked at the problematic Setup.hs
file.
Also, could you link to the upstream issue? This is so that it is easy for us to know when a new (fixed) version is released, so we can remove this override.
Two other things:
-
Could you rebase this on the
haskell-updates
branch and change the base here on github tohaskell-updates
? We generally send haskell fixes to thehaskell-updates
branch (notmaster
). -
Could you remove
guid
from the list ofbroken-packages
inpkgs/development/haskell-modules/configuration-hackage2nix.yaml
? If you don't do this, thenbroken = true
will always be set in thepkgs/development/haskell-modules/hackage-packages.nix
file. -
Maybe the comment should be edited to not talk about disabling tests? It looks like you went back and enabled the tests again. I guess they didn't end up causing a problem?
Also, completely unrelated, but I'm surprised you're saying guid
is a dependency of many packages in the Haskell ecosystem. I've never heard of it, and it doesn't look like it is a dependency in any other packages in hackage-packages.nix
.
9336628
to
fe3dc1d
Compare
guid: fix compilation of Setup.hs and disable tests Add more docs Remove guid from broken-packages
7008017
to
74a6889
Compare
@cdepillabout Ouch it's a disaster, I think I mistyped my search term, thank you for spotting it. And it feels awesome to see how nixpkgs quality still the best by means of code review and quality. I can catch up how things evolved with your guidance. I think it's unnecessary to fix this package for now but for the sake of completeness I did all the tasks that you mentioned. Thank you. |
@aycanirican Ah, well thanks for going through and fixing this up even though it might not have been the package you were after! Looks good to me, thanks! |
Motivation for this change
haskellPackages.guid
doesn't compile due to the error below:Removing module declaration in
Setup.hs
fixes the problem. Also tests are failing due to the dependency on hspec-discover, I just disabled them since there actually are no tests.Things done
sandbox
innix.conf
on non-NixOS linux)nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
./result/bin/
)nix path-info -S
before and after)