emscripten: 3.1.64 -> 3.1.73 #343743
emscripten: 3.1.64 -> 3.1.73 #343743
Conversation
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
887fcb0 to
378f60c
Compare
ofborg
bot
added
11.by: package-maintainer
|
|
Ooh actually I ran it again and no problems this time. Seems to have been a one-off thing.
|
uncenter
added
the
12.approvals: 1
|
i get same build failure on x86_64-linux as ofborg |
|
I'm also very interested in this, since the emscripten toolchain currently in nixpkgs has a version mismatch between emscripten and binaryen that i'm always running into (see here: emscripten-core/emscripten#22249 (comment)). To be able to work I've locally bumped just binaryen and just disabled the the tests, which allowed me to keep working, but I'm looking forward to this being properly fixed, so I did some digging. I found the cause of this problem, which is that here binaryen added some test files as a git submodule, and since we clone the source without submodules, some parts of the testsuite are missing. I've changed the configure phase to symlink the testsuite into test/spec/testsuite and got everything working, not sure if there is a more elegant way, it feels a bit clunky. Another way would be to git clone recursively, but that would also pull things like googletest. With the changed binaryen derivation below I can build binaryen and emscripten without issue on x86_64-linux. I did not yet try to actually use the resulting toolchain. { lib, stdenv, cmake, python3, fetchFromGitHub, emscripten,
gtest, lit, nodejs, filecheck
}:
let
testsuite = fetchFromGitHub {
owner = "WebAssembly";
repo = "testsuite";
rev = "e05365077e13a1d86ffe77acfb1a835b7aa78422";
hash = "sha256-yvZ5AZTPUA6nsD3xpFC0VLthiu2CxVto66RTXBXXeJM=";
};
in
stdenv.mkDerivation rec {
pname = "binaryen";
version = "119";
src = fetchFromGitHub {
owner = "WebAssembly";
repo = "binaryen";
rev = "version_${version}";
hash = "sha256-JYXtN3CW4qm/nnjGRvv3GxQ0x9O9wHtNYQLqHIYTTOA=";
};
nativeBuildInputs = [ cmake python3 ];
preConfigure = ''
if [ $doCheck -eq 1 ]; then
sed -i '/googletest/d' third_party/CMakeLists.txt
rmdir test/spec/testsuite
ln -s ${testsuite} test/spec/testsuite
else
cmakeFlagsArray=($cmakeFlagsArray -DBUILD_TESTS=0)
fi
'';
nativeCheckInputs = [ gtest lit nodejs filecheck ];
checkPhase = ''
LD_LIBRARY_PATH=$LD_LIBRARY_PATH:$PWD/lib python3 ../check.py $tests
'';
tests = [
"version" "wasm-opt" "wasm-dis"
"crash" "dylink" "ctor-eval"
"wasm-metadce" "wasm-reduce" "spec"
"lld" "wasm2js" "validator"
"example" "unit"
# "binaryenjs" "binaryenjs_wasm" # not building this
"lit" "gtest"
];
doCheck = stdenv.isLinux;
meta = with lib; {
homepage = "https://github.com/WebAssembly/binaryen";
description = "Compiler infrastructure and toolchain library for WebAssembly, in C++";
platforms = platforms.all;
maintainers = with maintainers; [ asppsa willcohen ];
license = licenses.asl20;
};
passthru.tests = {
inherit emscripten;
};
} |
Same here! Happy to test whatever on darwin to get this through :) |
|
@wirew0rm would you like to maybe add those changes into a new PR instead of this? Looks like OP hasn't been actively recently, really hoping we can get this through. |
|
I am VERY sorry that I missed that you found the fixes. I can incorporate this weekend if that works! |
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
378f60c to
5fd8cc5
Compare
|
Rebased and testing locally on darwin now. If it builds I'll see if we can bump past .67 without bigger LLVM changes, which can wait for a later PR. |
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
5fd8cc5 to
0c484e0
Compare
|
Looks good locally. If we're green across the board on ofborg we might be good to go! |
That is configurable via |
willcohen
force-pushed
the
emscripten-3.1.67
branch
3 times, most recently
from
8d45a95 to
409ecf6
Compare
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
409ecf6 to
a51f60e
Compare
|
Running nixpkgs-review on this PR on x86_64-linux NixOS |
|
If the nixpkgs-review is clean on Linux and ofborg comes back green I’ll merge, thanks! |
|
|
Log of binaryen build failure. |
wegank
removed
the
12.approvals: 1
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
a51f60e to
a7d93ee
Compare
willcohen
force-pushed
the
emscripten-3.1.67
branch
from
a7d93ee to
e255a90
Compare
|
Re-factored again to apply the method from @wirew0rm. If ofborg is green with x86-linux, this may be solved. @fabianhjr if you're able to run nixpkgs-review to confirm, that would be helpful as well! |
|
Running nixpkgs-review 🙌 |
|
|
Thank you! I’ll wait for ofborg to pass and merge from there. |
|
|
Thanks @uncenter. Since that’s the last hanging ofborg build I think this is good to go. |
|
Just as a quick note here, 3.1.74 will require https://github.com/llvm/llvm-project/commit/c3536b263f253a69fb336fb0617ee33a01a5c5dd.patch to either be backported to LLVM 19, or will need to wait to depend the LLVM 20 release. Now that emscripten's working again, I don't think this smaller version bump is totally critical. Additionally, the release after this will be emscripten 4.0, so I'm inclined to focus my effort on working on that in the new year. |
Description of changes
Supersedes #340130, embedding that commit as a pre-requisite.
To avoid breakage noted there, also bumps emscripten to latest release.
Things done
nix.conf? (See Nix manual)sandbox = relaxedsandbox = truenix-shell -p nixpkgs-review --run "nixpkgs-review rev HEAD". Note: all changes have to be committed, also see nixpkgs-review usage./result/bin/)Add a 👍 reaction to pull requests you find important.