tools: fix riscv64 build failed
fix https://github.com/nodejs/node/issues/52678
Review requested:
- [ ] @nodejs/gyp
- [ ] @nodejs/v8-update
I had a try with your branch with GCC10 from Ubuntu 20.04/riscv64 and run into an issue with a number of symbols when building the test code - is that expected and do you get the same failure? I don't think I've seen these before.
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::RmDir(const char*)':
../deps/googletest/include/gtest/internal/gtest-port.h:2053:44: error: 'rmdir' was not declared in this scope; did you mean 'simdutf::internal::rmdir'?
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::DoIsATTY(int)':
../deps/googletest/include/gtest/internal/gtest-port.h:2084:38: error: 'isatty' was not declared in this scope; did you mean 'simdutf::internal::isatty'?
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::ChDir(const char*)':
../deps/googletest/include/gtest/internal/gtest-port.h:2113:44: error: 'chdir' was not declared in this scope; did you mean 'simdutf::internal::chdir'?
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::Read(int, void*, unsigned int)':
../deps/googletest/include/gtest/internal/gtest-port.h:2135:27: error: 'read' was not declared in this scope; did you mean 'simdutf::internal::read'?
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::Write(int, const void*, unsigned int)':
../deps/googletest/include/gtest/internal/gtest-port.h:2138:27: error: 'write' was not declared in this scope; did you mean 'simdutf::internal::write'?
../deps/googletest/include/gtest/internal/gtest-port.h: In function 'int testing::internal::posix::Close(int)':
../deps/googletest/include/gtest/internal/gtest-port.h:2140:35: error: 'close' was not declared in this scope; did you mean 'simdutf::internal::close'?
EDIT: I'm going to be away for the next 2½ weeks - so until June - so if I don't respond to any further queries on this, that will be why
Why does that check for host_arch=="arm64" or target_arch=="arm64"?
Why does that check for host_arch=="arm64" or target_arch=="arm64"?
Thanks. I had fix this.
CI: https://ci.nodejs.org/job/node-test-pull-request/60910/
CI: https://ci.nodejs.org/job/node-test-pull-request/60910/
LGTM - three failures - all passed on first re-run. First two are marked flakey:
- https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl31_x64/45075/testReport/(root)/benchmark/test_benchmark_crypto_/
- https://ci.nodejs.org/job/node-test-commit-plinux/nodes=rhel8-ppc64le/54657/testReport/(root)/parallel/test_sqlite_/
- https://ci.nodejs.org/job/node-test-commit-arm/nodes=ubuntu2004-arm64/53936/testReport/(root)/parallel/test_error_serdes_/
@luyahan Before I approve (it fixes the build failure for me on a recent gcc) is there a reason why this PR appears to add extra files into the x64 build as well? I would expect this fox to only be in riscv64 specific sections.
@luyahan Hello,
can you add the repair code for the loong64 architecture? loong64 also needs this patch.
Thanks you.
['v8_target_arch=="loong64"', {
'sources': [
'<!@pymod_do_main(GN-scraper "<(V8_ROOT)/BUILD.gn" "\\"v8_base_without_compiler.*?v8_enable_wasm_gdb_remote_debugging.*?v8_current_cpu == \\"loong64\\".*?sources \\+= ")',
],
'conditions': [
['v8_enable_webassembly==1', {
'conditions': [
['(_toolset=="host" and host_arch=="arm64" or _toolset=="target" and target_arch=="arm64") or (_toolset=="host" and host_arch=="loong64" or _toolset=="target" and target_arch=="loong64") or (_toolset=="host" and host_arch=="x64" or _toolset=="target" and target_arch=="x64")', {
'sources': [
'<(V8_ROOT)/src/trap-handler/handler-inside-posix.cc',
'<(V8_ROOT)/src/trap-handler/handler-outside-posix.cc',
],
}],
['(_toolset=="host" and host_arch=="x64" or _toolset=="target" and target_arch=="x64") and (OS=="linux")', {
'sources': [
'<(V8_ROOT)/src/trap-handler/handler-outside-simulator.cc',
],
}],
],
}],
],
}],
Is the riscv build for 22 still broken waiting for this to be reviewed?
@luyahan Before I approve (it fixes the build failure for me on a recent gcc) is there a reason why this PR appears to add extra files into the x64 build as well? I would expect this fox to only be in riscv64 specific sections.
@luyahan and @sxa Can you get this sorted out?
It would be great if Node.js supported Linux/RISC-V (64-bit) again.
I had to pull this patch into Fedora/RISCV NodeJS 22 to get it building again. http://fedora.riscv.rocks/koji/buildinfo?buildID=343396
Could someone review this patch to unblock NodeJS 22?
CI: https://ci.nodejs.org/job/node-test-pull-request/60910/
LGTM - three failures - all passed on first re-run. First two are marked flakey:
- https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl31_x64/45075/testReport/(root)/benchmark/test_benchmark_crypto_/
- https://ci.nodejs.org/job/node-test-commit-plinux/nodes=rhel8-ppc64le/54657/testReport/(root)/parallel/test_sqlite_/
- https://ci.nodejs.org/job/node-test-commit-arm/nodes=ubuntu2004-arm64/53936/testReport/(root)/parallel/test_error_serdes_/
@luyahan Before I approve (it fixes the build failure for me on a recent gcc) is there a reason why this PR appears to add extra files into the x64 build as well? I would expect this fox to only be in riscv64 specific sections.
There have condition ['v8_target_arch=="riscv64"',, so it don't add file into x64 build.
((_toolset=="host" and host_arch=="x64" or _toolset=="target" and target_arch=="x64")
The condition will be effective after the pr https://github.com/nodejs/node/pull/55848 When enable v8_simulator, v8_target_arch != target_arch.
CI: https://ci.nodejs.org/job/node-test-pull-request/60910/
LGTM - three failures - all passed on first re-run. First two are marked flakey:
- https://ci.nodejs.org/job/node-test-commit-linux-containered/nodes=ubuntu2204_sharedlibs_openssl31_x64/45075/testReport/(root)/benchmark/test_benchmark_crypto_/
- https://ci.nodejs.org/job/node-test-commit-plinux/nodes=rhel8-ppc64le/54657/testReport/(root)/parallel/test_sqlite_/
- https://ci.nodejs.org/job/node-test-commit-arm/nodes=ubuntu2004-arm64/53936/testReport/(root)/parallel/test_error_serdes_/
@luyahan Before I approve (it fixes the build failure for me on a recent gcc) is there a reason why this PR appears to add extra files into the x64 build as well? I would expect this fox to only be in riscv64 specific sections.
Hi @sxa Could you have time to review this pr again?
CI: https://ci.nodejs.org/job/node-test-pull-request/63570/
CI: https://ci.nodejs.org/job/node-test-pull-request/63613/
@luyahan Can you adjust the commit message as per the "Merging is blocked" notification above please?
@luyahan Can you adjust the commit message as per the "Merging is blocked" notification above please?
You can ignore that -- the tooling for merging will take care of it.
Landed in bdaa898ceaf61840d030ee83b2e9adf40973544d