emscripten: 3.0.0 -> 3.1.12 #175358
emscripten: 3.0.0 -> 3.1.12 #175358
Conversation
|
@ofborg build emscripten emscriptenPackages.xmlmirror |
|
Result of 6 packages built:
|
|
@ofborg build binaryen |
| inherit pname version; | ||
| sha256 = "sha256-Ae4ye2t0LO0v2eRDGdLXBqRXPl4O5yYZONDeFqd+XAY="; | ||
| }; | ||
|
|
There was a problem hiding this comment.
Missing pythonImportsCheck and we should execute tests
There was a problem hiding this comment.
The pypi source does not package the tests.
There was a problem hiding this comment.
If that's the case, please use fetchFromGitHub instead of fetchPypi.
There was a problem hiding this comment.
I would strongly prefer not to, the github repo also uses poetry instead of setuptools.
There was a problem hiding this comment.
Changing to github source is a standard procedure to get tests executed which we should always do if they work in the sandbox.
Building from poetry is easy. Add format = "pyproject"; and check if the project uses poetry or poetry-core and add that to nativeBuildInputs.
|
The checks for binaryen don't seem to work on darwin-x86_64. Here's the error output: |
If only checks are broken I would suggest to disable them on darwin. If the entire binary is broken then please mark it broken for darwin. |
|
Removing the checks made binaryen sucessfully compile, but left emscripten broken with a compilation error. I tried to research it but I wasn't able to find the solution for now. If we merge this pull request, we might have to mark darwin as broken for the moment. |
|
If no one can help here marking it broken is fine @NixOS/darwin-maintainers |
This fixes the emscriptenPackages.xmlmirror-unstable build
If updating Is there a rule written about how to handle this case for multiple architectures? Are there different tiers support (like tier1 = linux amd64, tier 2 = linux arm64, tier 3 = darwin) that would allow to merge a commit breaking a lower tier at the benefit of an upper tier. When you start supporting many architectures and OS, it's normal to make concessions for less used systems in favor to the more mainstream one if we don't want to multiply packages versions to keep compatibility everywhere. |
Yes, these are defined https://github.com/NixOS/rfcs/blob/master/rfcs/0046-platform-support-tiers.md |
|
Fwiw, this looks like something for upstream to at least investigate. They may be making assumptions based on Linux-specific behavior here. |
thanks! Given this document, we need to involve @NixOS/darwin-maintainers to know if the update can be merged or not. |
|
Per emscripten-core/emscripten#10072, emscripten expects tip-of-tree llvm. I see that you bumped llvm to 14 -- is there any chance that this is a yet-unresolved llvm issue? It strikes me as generally unlikely, but at the moment I can't figure out why this error would be re-cropping up under llvm 14, and @gracicot's already followed up with all the obvious leads for similar issues. |
|
@willcohen The only way I can get clang to have a |
|
If there's a pressing reason to get this merged, I suppose okay to break it on darwin for now (though I was finally getting the emscriptenPackages to build again on darwin as of a few weeks ago!). That said, if there's nothing pressing, it'd be great to get another week or two to work on this. I feel odd posing such an open-ended question upstream and suspect that we need to do some bisecting to figure out what broke this -- at least what point release and hopefully what commit -- but I will need probably at least another week to find the time to get into that in a little more detail. |
|
Would it work to merge #172741 instead? Then at least we'd be up to 3.1.10 and can work on addressing the LLVM issues needed to get to 3.1.12 after that in this PR after rebasing? |
|
@gracicot is your failing darwin build x86 or aarch64? If I turn off checks for binaryen on darwin, I can build this PR on x86: |
|
@willcohen I confirm I'm able to build it. I'm not sure what went wrong in my previous attempt. |
merged |
|
Thanks! Now that we've determined that this PR should be okay, looks like this'll just require a rebase to bump from 3.1.10 and include the other |
bobby285271
added
the
12.approvals: 1
SuperSandro2000
added
the
2.status: merge conflict
|
I've rebased this and updated it to 3.1.13 with #178620. Note that 3.1.14 breaks due to #178620 (comment). |
|
Closing in favor of the linked PR |
Description of changes
Update emscripten and its dependencies, also fix build dependencies (were complaining about missing archive indices).
Things done
sandbox = trueset innix.conf? (See Nix manual)nix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)nixos/doc/manual/md-to-db.shto update generated release notes