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

Fixed GHCJS #34037

Merged
merged 1 commit into from Jan 21, 2018
Merged

Fixed GHCJS #34037

merged 1 commit into from Jan 21, 2018

Conversation

ElvishJerricco
Copy link
Contributor

Motivation for this change
  • The bindists are not currently suitable for bootstrapping GHCJS
  • GHCJS needs to be built with the GHC version it represents
  • The haddock library in the package set depends on GHC 8.2's Cabal version, so needs to be downgraded.
  • ghcjsHEAD has a library dependency on Cabal (< 2.0), but needs cabal-install (>= 2.0) as a build tool. I think the best way to satisfy this is to just pass pkgs.cabal-install as the tool.

We should fix the bindists issue, but that's orthogonal to this PR.

Things done
  • Tested using sandboxing (nix.useSandbox on NixOS, or option build-use-sandbox in nix.conf on non-NixOS)
  • 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 nox --run "nox-review wip"
  • Tested execution of all binary files (usually in ./result/bin/)
  • Fits CONTRIBUTING.md.

# Newer versions require Cabal-2.0, and overrideScope is
# inappropriate for libraries.
haddock-api = self.haddock-api_2_17_4;
haddock = self.callHackage "haddock" "2.17.4" {};
Copy link
Contributor Author

Choose a reason for hiding this comment

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

What's the right way to do this? How can I get self.haddock_2_17_4 into hackage-packages.nix? /cc @peti

Copy link
Member

Choose a reason for hiding this comment

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

Add the package into the extra-packages section of pkgs/development/haskell-modules/configuration-hackage2nix.yaml. Then the package will show up the next time haskell-updates is merged.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Got it. Do I leave this as is in the meantime, or switch it to something that would fail e.g. leaving it un-overriden or referencing the version that won't exist until the next haskell-updates?

Copy link
Member

@peti peti left a comment

Choose a reason for hiding this comment

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

Looks good to me! There are 2 minor glitches; I added comments.

@@ -943,4 +943,6 @@ self: super: {
# Add support for https://github.com/haskell-hvr/multi-ghc-travis.
multi-ghc-travis = self.callPackage ../tools/haskell/multi-ghc-travis { ShellCheck = self.ShellCheck_0_4_6; };

shelly = dontCheck super.shelly;
Copy link
Member

Choose a reason for hiding this comment

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

Has the issue that causes the tests to fail been reported to upstream? If it has, then please add a link to the upstream ticket before that line so that other people can figure out what is going on.

# Newer versions require Cabal-2.0, and overrideScope is
# inappropriate for libraries.
haddock-api = self.haddock-api_2_17_4;
haddock = self.callHackage "haddock" "2.17.4" {};
Copy link
Member

Choose a reason for hiding this comment

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

Add the package into the extra-packages section of pkgs/development/haskell-modules/configuration-hackage2nix.yaml. Then the package will show up the next time haskell-updates is merged.

@peti
Copy link
Member

peti commented Jan 19, 2018

@ElvishJerricco, we already have haddock_2_17_5. Won't that work?

@ElvishJerricco
Copy link
Contributor Author

@peti Oh I hadn't noticed that somehow. I'll give it a shot

@ElvishJerricco
Copy link
Contributor Author

@peti I have made the requested changes.

@fosskers
Copy link

I'm imminently going to get into GHCjs (never touched it before). Should I wait for this PR to go through?

@ElvishJerricco
Copy link
Contributor Author

@fosskers GHCJS is currently broken on unstable (which this PR will fix), but it works fine on the 17.09 channel.

@fosskers
Copy link

Cool, I'll be patient then.

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

4 participants