-
-
Notifications
You must be signed in to change notification settings - Fork 14k
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
emscripten: 3.1.47 -> 3.1.50 #269904
emscripten: 3.1.47 -> 3.1.50 #269904
Conversation
Result of 6 packages failed to build:
10 packages built:
|
libxml2 is failing for me, but it looks like a mac-specific issue related to the
|
737fe83
to
9c35504
Compare
Bumped to 3.1.49. 3.1.50 will require bumping LLVM too, so I'll leave that till LLVM 17 stabilizes a little. |
Sounds good to me, thank you for taking care of that. |
Starting with emscripten-3.1.46, this flag to LLVM is needed. This is a backport of https://github.com/llvm/llvm-project/commit/93adcb770b99351b18553089c164fe3ef2119699.patch, with additional review at https://reviews.llvm.org/D158892 and emscripten-core/emscripten#20097.
9c35504
to
c4a686b
Compare
Smaller than I thought. Bumped to 3.1.50 -- required updating the patch file to deal with upstream changes, and carrying the |
Result of 9 packages failed to build:
12 packages built:
|
@RaitoBezarius ofborg looks green except for darwin aarch64, which worked out on my own nixpkgs-review. Let me know if this looks ready! |
This pull request has been mentioned on NixOS Discourse. There might be relevant details there: https://discourse.nixos.org/t/prs-already-reviewed/2617/1312 |
Thank you! |
This broke the javascript backend of GHC:
|
This appears to be due to a crash in emscripten.
|
Is it a crash or cache mishandling? I remember another PR we merged for removing cache stuff. |
That is the question… Emscripten has a writeable cache in this case. |
Many apologies for all this. The mention of atomic operations reminds me of #223282, which makes my hunch that this is about the bump to LLVM 17, which the crash also says is unsupported. Emscripten runs on tip of tree LLVM. If this is agreeable, in a few hours I can add a derivation back for 3.1.47 so GHC can keep depending on that. I’m not sure how else to fix the other caching issue in emscripten. It seems like the various libs just don’t build in all the variants any more, and I assumed the caching was just to speed things up for downstream derivations. |
@willcohen LLVM HEAD not being available doesn't seem to be the problem to me, since 3.1.50 explicitly states it uses 17.0.4? If the LLVM update is the issue, emscripten 3.1.49 should work, right? |
Good point. My hunch about LLVM was a combo of
and 2) #223282 (comment), which was in the end just a need to change the include directory path names to use only major version numbers. I'll adjust #278294 to go all the way to 3.1.49. |
@sternenseemann Adjusted with 79c0025 |
@willcohen I just want to say that I am amazed by your professionalism and your prompt answer in this issue, I am humbled by your skills. Thank you for being part of this community. |
Thanks, @RaitoBezarius! I'm much more appreciative of all the collective work you all do to keep this crazy thing working. |
Right @sternenseemann -- this is where we enter a funny territory. Emscripten very explicitly doesn't support any specific release of LLVM, but specifically only whatever is HEAD at the time a version number is cut (emscripten-core/emscripten#11362). I often come up with build failures at whatever time I try to bump the emscripten derivation, since it seems like this rapid cadence means that it's rare for packaging distributions to keep up. If we're early enough in the LLVM cycle to request backports, sometimes whatever new tip of tree stuff emscripten needs can get put into a point release, but more likely (as with #229718), emscripten sometimes needs some patches to stable LLVM (and therefore that runs through stable here), until the next major release gets cut. For this PR, "support" of LLVM 17.0.4 just meant that when I tried to build it, it worked and passed the tests. Should ofborg have caught this before, or is there another set of tests I should have run through before to make sure GHC was good? |
Description of changes
Bump from 3.1.47 to 3.1.50.
Things done
nix.conf
? (See Nix manual)sandbox = relaxed
sandbox = true
nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD"
. Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/
)Priorities
Add a 👍 reaction to pull requests you find important.