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

ghcjs: init at 8.10.7 #137066

Merged
merged 5 commits into from
Sep 15, 2021

Conversation

dfordivam
Copy link
Contributor

@dfordivam dfordivam commented Sep 8, 2021

Motivation for this change

Adds the ghcjs back to nixpkgs
fixes #133271

And contains a few other commits to make this work.

On darwin the llvm_13 (required by emscripten) fails, and therefore this change could not be tested.

In file included from /tmp/nix-build-lld-13.0.0-rc2.drv-0/source/lld/MachO/Driver.cpp:22:
/tmp/nix-build-lld-13.0.0-rc2.drv-0/source/lld/MachO/UnwindInfoSection.h:15:10: fatal error: 'mach-o/compact_unwind_encoding.h' file not found
#include "mach-o/compact_unwind_encoding.h"
         ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
1 error generated.
make[2]: *** [MachO/CMakeFiles/lldMachO2.dir/build.make:160: MachO/CMakeFiles/lldMachO2.dir/Driver.cpp.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:897: MachO/CMakeFiles/lldMachO2.dir/all] Error 2
make: *** [Makefile:149: all] Error 2
Things done
  • Built on platform(s)
    • x86_64-linux
    • aarch64-linux
    • x86_64-darwin
    • aarch64-darwin
  • For non-Linux: Is sandbox = true set in nix.conf? (See Nix manual)
  • Tested via one or more NixOS test(s) if existing and applicable for the change (look inside nixos/tests)
  • Tested compilation of all packages 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/)
  • 21.11 Release Notes (or backporting 21.05 Release notes)
    • (Package updates) Added a release notes entry if the change is major or breaking
    • (Module updates) Added a release notes entry if the change is significant
    • (Module addition) Added a release notes entry if adding a new NixOS module
  • Fits CONTRIBUTING.md.

@cdepillabout
Copy link
Member

I don't know very much about GHCJS, so I'm hesitant to merge this without at least a look-over from someone more knowledgeable about GHCJS.

Off the top of my head, I think @Ericson2314 and @angerman are both using GHCJS at their respective companies, so maybe they'd be able to give a good review.


Also, @dfordivam we have a Hydra jobset for Haskell in pkgs/top-level/release-haskell.nix. You might not want to do it in this PR (since it is already long), but at some point you may want to send a PR adding GHCJS to this jobset. If you do this, you'll get pinged on our haskell-updates PRs if GHCJS ever stops compiling. Here's an example of people being pinged for packages that have stopped compiling: #136589 (comment)

@cdepillabout
Copy link
Member

@dfordivam Ah, one last thing. Can you set the base branch to the haskell-updates branch instead of main for this PR? We generally try to merge all Haskell-related changes into haskell-updates.

@dfordivam dfordivam changed the base branch from master to haskell-updates September 9, 2021 05:09
@dfordivam dfordivam force-pushed the dn-add-ghcjs-810-1 branch 2 times, most recently from 915428b to 7a0b830 Compare September 10, 2021 02:15
Copy link
Member

@Ericson2314 Ericson2314 left a comment

Choose a reason for hiding this comment

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

Other than the little nit I left, this is good

@Ericson2314
Copy link
Member

Oh, and maybe ghcjs-ng -> ghcjs since the old GHCJS is now many years long gone?

@Ericson2314
Copy link
Member

I checked, and that shouldn't cause collisions on 21.05 or 20.09 either, where we might want to backport, so yes let's do that rename.

The src points to the obsidiansystems repo as it has the ghcjs ported from
8.10.5 to 8.10.7, and a bunch of other fixes (NixOS#812, NixOS#811, NixOS#809)
@Ericson2314 Ericson2314 merged commit 1f06456 into NixOS:haskell-updates Sep 15, 2021
@dfordivam dfordivam deleted the dn-add-ghcjs-810-1 branch September 16, 2021 01:24
@sternenseemann
Copy link
Member

ghcjs is quite big and 2.9GB over the output limit on Hydra at the moment, so we can't provide binary cache for it.

Any ideas how we can cut this down? Building the libraries separately and individually could be a solution, but not sure if it is even possible.

> du --max-depth 2 -h /nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/
57M	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/bin
20M	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/lib/ghcjs-8.10.7
3.8G	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/lib/js-ghcjs-ghcjs-8.10.7-ghc8_10_7
3.8G	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/lib
112K	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/share/doc
112K	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/share
3.9G	/nix/store/jnlca2nvk61mxr6x0pgqchfnrfnzwsjd-ghcjs-8.10.7/

@Ericson2314
Copy link
Member

Building the libraries separately is always something I'm keen to do in general. I'll try to see if we can investigate.

@sternenseemann
Copy link
Member

@dfordivam did report that the output was significantly smaller on their machine, so I'm a bit confused what exactly is going wrong on Hydra and my machine.

@dfordivam
Copy link
Contributor Author

@sternenseemann I just managed to build ghcjs on Darwin, and the output turns out to be 3.9G

3.9G	/nix/store/d9frl6sh7iydrgrqz9hgxv1fvkhjgcff-ghcjs-8.10.7

Given this I looked back at the linux machine and figured out that my number were wrong due to zfs compression. This is the real value..

$ du -h --apparent-size  /nix/store/vqw115sfnwl3y514shqc0malc1aaqgfj-ghcjs-8.10.7/
3.9G    /nix/store/vqw115sfnwl3y514shqc0malc1aaqgfj-ghcjs-8.10.7/

@angerman
Copy link
Contributor

So this is in line with the 4G llvm-aarch64 build we had? I hope that the ghcjs-in-ghc will be smaller, as it will eventually drop some packages from the rather large dependency closure of ghcjs.

@sternenseemann
Copy link
Member

@angerman I don't think ghcjs' (Haskell) dependency closure is the problem (if you mean that). The problem is rather the /lib/js-ghcjs-ghcjs-8.10.7-ghc8_10_7 (3.8 GB) directory which is probably necessary as it contains the bundled packages for the compilation target.

@Ericson2314
Copy link
Member

Ericson2314 commented Sep 23, 2021

@angerman yeah we should very soon ™ be able to build GHC itself, RTS, base by themselves for regular GHC, and I hope that will be the case for GHCJS too.

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.

Re-add ghcjs
5 participants