Skip to content
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

matterhorn: fix build #92618

Merged
merged 1 commit into from Aug 12, 2020
Merged

Conversation

Kiwi
Copy link
Member

@Kiwi Kiwi commented Jul 7, 2020

really it's fixing Unique which fixes matterhorn

Motivation for this change
Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option sandbox in nix.conf on non-NixOS linux)
  • Built on platform(s)
    • NixOS
    • macOS
    • other Linux distributions
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all pkgs that depend on this change using nix-shell -p nixpkgs-review --run "nixpkgs-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Determined the impact on package closure size (by running nix path-info -S before and after)
  • Ensured that relevant documentation is up to date
  • Fits CONTRIBUTING.md.

Comment on lines 933 to 935
# matterhorn needs this
Unique = doJailbreak super.Unique;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why is this doJailbreak needed? Is this something you could get fixed upstream? (Even if it is just a hackage revision...)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

CallStack (from HasCallStack):
  die', called at libraries/Cabal/Cabal/Distribution/Simple/Configure.hs:1022:20 in Cabal-3.0.1.0:Distribution.Simple.Configure
  configureFinalizedPackage, called at libraries/Cabal/Cabal/Distribution/Simple/Configure.hs:475:12 in Cabal-3.0.1.0:Distribution.Simple.Configure
  configure, called at libraries/Cabal/Cabal/Distribution/Simple.hs:625:20 in Cabal-3.0.1.0:Distribution.Simple
  confHook, called at libraries/Cabal/Cabal/Distribution/Simple/UserHooks.hs:65:5 in Cabal-3.0.1.0:Distribution.Simple.UserHooks
  configureAction, called at libraries/Cabal/Cabal/Distribution/Simple.hs:180:19 in Cabal-3.0.1.0:Distribution.Simple
  defaultMainHelper, called at libraries/Cabal/Cabal/Distribution/Simple.hs:116:27 in Cabal-3.0.1.0:Distribution.Simple
  defaultMain, called at Setup.hs:2:8 in main:Main
Setup: Encountered missing or private dependencies:
extra >=1.6.2 && <1.7

builder for '/nix/store/gn395r9sq2ik1v3vfyv96s9qhvf4mcyj-Unique-0.4.7.7.drv' failed with exit code 1

all i know is every time stupid stackage gets updated everything breaks

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i think the real question is why do i keep having to add jailbreaks back after people remove them when i'll have to do it again in a few weeks ;_;

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kiwi I'm sorry that you end up having to keep fixing this.

The nixpkgs haskell packages generally follow Stackage, which has major updates about every 3 to 6 months. For packages that are in stackage, nixpkgs generally has good support, and you don't end up having to fix things too often.

For packages not in Stackage, it needs a lot more work from the nixpkgs maintainers to make sure everything compiles together. You are much more likely to see packages failing that are not in stackage.

Also, users will sometimes go through and remove things like doJailbreak when they are no longer needed. My guess is that at some point Unique was compiling correctly without doJailbreak, so someone removed. Then possibly with a future update of Unique or stackage, version bounds were incorrect again, so it needs to be fixed again.

The easiest solution here is to get Unique (and possibly matterhorn) into Stackage.

If you don't want to do that, then possibly create issues upstream and urge the maintainers to make new releases that fix versioning problems. Many packages have versions that are too strict, and really require frequent updates from their maintainers. For maintainers that don't want to do this, they should use versions that are not as strict.

If upstream is not responsive, then we generally use doJailbreak here in nixpkgs, but it is the nixpkgs users responsibility to be vigilant when the package ends up breaking again.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thank you for the explanation. sorry if i came off a bit curt. can we merge this in the meantime and i'll try to talk to people about getting it fixed upstream? sometime

Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's done. Please proceed.
@Kiwi Thanks for help again

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

awesome! which is good since it seems i screwed up the commits on this one anyway when i tried to update it last... @kapralVV sure thing! glad to help anytime if you need anything let me know

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

success!

04:05:37 [kiwi@mvp-nixos nixpkgs]$ nix-build --no-out-link -A haskellPackages.Unique --arg config '{ allowBroken = true; }'
/nix/store/i7gsk67a9a4942vjzizkdrs9mjj70ckk-Unique-0.4.7.8

so it builds now on

commit bb8c74439a0e9aca68fee4434b3c5565e2a2a46a (HEAD -> haskell-updates, origin/haskell-updates)
Author: Peter Simons <simons@cryp.to>
Date:   Tue Aug 11 02:30:28 2020 +0200

but is still marked broken. should I change this pr to only remove that it's broken? and fix the rebase...

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@Kiwi Yes, if you change this PR to just remove that Unique is broken, I'll go ahead and merge this in.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

i'm starting to get the hang of this rebase thing that only took me one try and a few minutes. i think it's fixed now

@evils
Copy link
Member

evils commented Jul 8, 2020

builds and runs for me, thanks
and it looks like the latest matterhorn version fixed custom emoji / reactions :D

Comment on lines 933 to 935
# matterhorn needs this
Unique = doJailbreak super.Unique;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you please add that URL to the Nixpkgs source code in a comment before your override? That's more helpful than "matterhorn needs this". :-)

@peti peti force-pushed the haskell-updates branch 3 times, most recently from 8a7eb3e to ed16234 Compare August 7, 2020 18:35
also removes broken for the dependency Unique
@cdepillabout
Copy link
Member

Looks like everything builds now, thanks for working through this with us!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants