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

Node.js always builds as if cross-compiling #321847

Closed
wh0 opened this issue Jun 22, 2024 · 4 comments
Closed

Node.js always builds as if cross-compiling #321847

wh0 opened this issue Jun 22, 2024 · 4 comments
Labels

Comments

@wh0
Copy link
Contributor

wh0 commented Jun 22, 2024

Describe the bug

I tried building Node.js using ccacheStdenv, and it was faster than with normal stdenv. Here's my theory of why that is.

Node.js always builds as if cross-compiling, producing various 'host' and 'target' files even when building for the same architecture and OS. Everything still seems to work, but there's quite some wasted time in the build process compiling several things twice exactly the same.

https://github.com/nodejs/gyp-next/blob/main/docs/UserDocumentation.md#cross-compiling

https://github.com/nodejs/node/blob/v22.2.0/tools/gyp/pylib/gyp/common.py#L695-L696

Docs say it cross compiles if "one of the toolchain-related variables (like CC_host or CC_target) is set."

CC_host = "cc";
CXX_host = "c++";

Nixpkgs does set these.

Steps To Reproduce

Steps to reproduce the behavior:

  1. Build nodejs_22 package, for example
  2. Observe logs

https://cache.nixos.org/log/ix3b9lqmh4iq26kwx3jnvb2kn076961g-nodejs-22.2.0.drv

On this build from Hydra, 5414 lines are about .host stuff and 5862 lines are about .target (30 lines with both).

Here's a sample of two lines from the log:

(lines are long, expand this to view them)

c++ -o /build/node-v22.2.0/out/Release/obj.host/v8_initializers/deps/v8/src/builtins/builtins-regexp-gen.o ../deps/v8/src/builtins/builtins-regexp-gen.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_CERT_STORE' '-DICU_NO_USER_DATA_OVERRIDE' '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-D__STDC_FORMAT_MACROS' '-DV8_TARGET_ARCH_X64' '-DV8_HAVE_TARGET_OS' '-DV8_TARGET_OS_LINUX' '-DV8_EMBEDDER_STRING="-node.12"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DV8_ENABLE_PRIVATE_MAPPING_FORK_OPTIMIZATION' '-DV8_SHORT_BUILTIN_CALLS' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '-DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '-DV8_SHARED_RO_HEAP' '-DNDEBUG' '-DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '-DV8_USE_ZLIB' '-DV8_ENABLE_SPARKPLUG' '-DV8_ENABLE_MAGLEV' '-DV8_ENABLE_TURBOFAN' '-DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ENABLE_CONTINUATION_PRESERVED_EMBEDDER_DATA' '-DV8_ALLOCATION_FOLDING' '-DV8_ALLOCATION_SITE_TRACKING' '-DV8_ADVANCED_BIGINT_ALGORITHMS' -I/nix/store/qj9byzfvh7dd61kk0aglj7cwqj1xqg6l-zlib-1.3.1-dev/include -I/nix/store/fn8a5spls1h6iim00fqbcm8802k48l3p-libuv-1.48.0-dev/include -I/nix/store/191vca5vdxdlr32k2hpzd66mic98930f-openssl-3.0.13-dev/include -I/nix/store/zfi1y943q15js1ysgl4vk6lp2bzrngn2-icu4c-73.2-dev/include -I../deps/v8 -I../deps/v8/include -I/build/node-v22.2.0/out/Release/obj/gen -I/build/node-v22.2.0/out/Release/obj/gen/generate-bytecode-output-root -I../deps/v8/third_party/abseil-cpp -pthread -Wno-unused-parameter -Wno-strict-overflow -Wno-return-type -Wno-int-in-bool-context -Wno-deprecated -Wno-stringop-overflow -Wno-stringop-overread -Wno-restrict -Wno-array-bounds -Wno-nonnull -Wno-dangling-pointer -flax-vector-conversions -fno-strict-aliasing -m64 -m64 -O3 -fno-omit-frame-pointer -fdata-sections -ffunction-sections -O3 -fno-rtti -fno-exceptions -std=gnu++20 -Wno-invalid-offsetof -MMD -MF /build/node-v22.2.0/out/Release/.deps//build/node-v22.2.0/out/Release/obj.host/v8_initializers/deps/v8/src/builtins/builtins-regexp-gen.o.d.raw -c

g++ -o /build/node-v22.2.0/out/Release/obj.target/v8_initializers/deps/v8/src/builtins/builtins-regexp-gen.o ../deps/v8/src/builtins/builtins-regexp-gen.cc '-D_GLIBCXX_USE_CXX11_ABI=1' '-DNODE_OPENSSL_CONF_NAME=nodejs_conf' '-DNODE_OPENSSL_CERT_STORE' '-DICU_NO_USER_DATA_OVERRIDE' '-DV8_GYP_BUILD' '-DV8_TYPED_ARRAY_MAX_SIZE_IN_HEAP=64' '-D__STDC_FORMAT_MACROS' '-DV8_TARGET_ARCH_X64' '-DV8_HAVE_TARGET_OS' '-DV8_TARGET_OS_LINUX' '-DV8_EMBEDDER_STRING="-node.12"' '-DENABLE_DISASSEMBLER' '-DV8_PROMISE_INTERNAL_FIELD_COUNT=1' '-DV8_ENABLE_PRIVATE_MAPPING_FORK_OPTIMIZATION' '-DV8_SHORT_BUILTIN_CALLS' '-DOBJECT_PRINT' '-DV8_INTL_SUPPORT' '-DV8_ATOMIC_OBJECT_FIELD_WRITES' '-DV8_ENABLE_LAZY_SOURCE_POSITIONS' '-DV8_USE_SIPHASH' '-DV8_SHARED_RO_HEAP' '-DNDEBUG' '-DV8_WIN64_UNWINDING_INFO' '-DV8_ENABLE_REGEXP_INTERPRETER_THREADED_DISPATCH' '-DV8_USE_ZLIB' '-DV8_ENABLE_SPARKPLUG' '-DV8_ENABLE_MAGLEV' '-DV8_ENABLE_TURBOFAN' '-DV8_ENABLE_WEBASSEMBLY' '-DV8_ENABLE_JAVASCRIPT_PROMISE_HOOKS' '-DV8_ENABLE_CONTINUATION_PRESERVED_EMBEDDER_DATA' '-DV8_ALLOCATION_FOLDING' '-DV8_ALLOCATION_SITE_TRACKING' '-DV8_ADVANCED_BIGINT_ALGORITHMS' -I/nix/store/qj9byzfvh7dd61kk0aglj7cwqj1xqg6l-zlib-1.3.1-dev/include -I/nix/store/fn8a5spls1h6iim00fqbcm8802k48l3p-libuv-1.48.0-dev/include -I/nix/store/191vca5vdxdlr32k2hpzd66mic98930f-openssl-3.0.13-dev/include -I/nix/store/zfi1y943q15js1ysgl4vk6lp2bzrngn2-icu4c-73.2-dev/include -I../deps/v8 -I../deps/v8/include -I/build/node-v22.2.0/out/Release/obj/gen -I/build/node-v22.2.0/out/Release/obj/gen/generate-bytecode-output-root -I../deps/v8/third_party/abseil-cpp -pthread -Wno-unused-parameter -Wno-strict-overflow -Wno-return-type -Wno-int-in-bool-context -Wno-deprecated -Wno-stringop-overflow -Wno-stringop-overread -Wno-restrict -Wno-array-bounds -Wno-nonnull -Wno-dangling-pointer -flax-vector-conversions -fno-strict-aliasing -m64 -m64 -O3 -fno-omit-frame-pointer -fdata-sections -ffunction-sections -O3 -fno-rtti -fno-exceptions -std=gnu++20 -Wno-invalid-offsetof -MMD -MF /build/node-v22.2.0/out/Release/.deps//build/node-v22.2.0/out/Release/obj.target/v8_initializers/deps/v8/src/builtins/builtins-regexp-gen.o.d.raw -c

They compile the exact same files with the exact same flags, with g++ and c++ referring to the exact same compiler, and I would guess those .o.d.raw files are the same too.

Expected behavior

For comparison, here's Node.js's own CI build, with very few lines about .host. (I think you need to be logged in on GitHub to open this.)

https://github.com/nodejs/node/actions/runs/9091211063/job/24985365999

The sample lines from above about builtins-regexp-gen.cc only occurs once.

Screenshots

If applicable, add screenshots to help explain your problem.

Additional context

Add any other context about the problem here.

Notify maintainers

@NickCao if you remember about these vars

maintainers @cillianderoiste @cko @aduh95

Metadata

Please run nix-shell -p nix-info --run "nix-info -m" and paste the result.

$ /tmp/nix/store/gqks8cdk8dmq1bffw7ka3pyzv4zmj2i3-nix-info/bin/nix-info -m
/tmp/nix/store/lgjm7zgsbd3xzwdkvlriid9jqm7y13rz-bash-5.2p26/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_US.UTF-8)
/tmp/nix/store/gqks8cdk8dmq1bffw7ka3pyzv4zmj2i3-nix-info/bin/nix-info: line 74: nix-instantiate: command not found
 - system: `0`
 - host os: `Linux 4.4.0-1128-aws, Ubuntu, 16.04.7 LTS (Xenial Xerus), nobuild`
 - multi-user?: `no`
 - sandbox: `yes`
/tmp/nix/store/gqks8cdk8dmq1bffw7ka3pyzv4zmj2i3-nix-info/bin/nix-info: line 172: nix-env: command not found
 - version: `0`
find: '/nix/var/nix/profiles/per-user': No such file or directory

I don't have stuff set up for nix-info to run successfully, but it's Nix 2.18.2, nixos-24.05


Add a 👍 reaction to issues you find important.

@wh0 wh0 added the 0.kind: bug Something is broken label Jun 22, 2024
@wh0
Copy link
Contributor Author

wh0 commented Jul 3, 2024

reposting the notifications. I had filled that in after submitting this and I think that doesn't actually notify the mentioned people when editing. but sorry for the extra notification if it already had


Notify maintainers

@NickCao if you remember about these vars

maintainers @cillianderoiste @cko @aduh95

@aduh95
Copy link
Contributor

aduh95 commented Jul 5, 2024

I think this has been fixed by #322799. Closing for now, let me know if I missed something.

@aduh95 aduh95 closed this as completed Jul 5, 2024
@tie
Copy link
Member

tie commented Jul 5, 2024

I don’t think this was fixed by #322799 since it still sets {CC,CXX}_host but from configureScript. I assume #220204 technically should make it so that Node.js package doesn’t need a compiler for the build platform (by using an emulator), but there are a few other PRs with Node.js package that I’d like to get reviewed and merged before focusing on this issue.

@tie
Copy link
Member

tie commented Aug 13, 2024

Fixed in #327653.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

3 participants