tools: cache V8 build on test-shared workflow · nodejs/node@c2de1fa · GitHub
Skip to content

Commit c2de1fa

Browse files
aduh95aduh95
committed
tools: cache V8 build on test-shared workflow
This commit makes the CI use the Nix/Cachix cache rather than the sccache one for building V8 and its dependencies. This should speed up the average test-shared build time and reduce the stress on the GHA binary cache – at the expense of slower builds when touching a file used to building V8. PR-URL: #61860 Refs: #61436 Reviewed-By: Yagiz Nizipli <yagiz@nizipli.com> Reviewed-By: Chemi Atlow <chemi@atlow.co.il> Reviewed-By: Chengzhong Wu <legendecas@gmail.com>
1 parent b1c1ddf commit c2de1fa

6 files changed

Lines changed: 218 additions & 51 deletions

File tree

‎.github/workflows/test-shared.yml‎

Lines changed: 5 additions & 10 deletions

‎configure.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2001,10 +2001,16 @@ def configure_v8(o, configs):
20012001
o['variables']['node_enable_v8windbg'] = b(options.enable_v8windbg)
20022002
if options.enable_d8:
20032003
o['variables']['test_isolation_mode'] = 'noop' # Needed by d8.gyp.
2004-
if options.without_bundled_v8 and options.enable_d8:
2005-
raise Exception('--enable-d8 is incompatible with --without-bundled-v8.')
2006-
if options.without_bundled_v8 and options.enable_v8windbg:
2007-
raise Exception('--enable-v8windbg is incompatible with --without-bundled-v8.')
2004+
if options.without_bundled_v8:
2005+
if options.enable_d8:
2006+
raise Exception('--enable-d8 is incompatible with --without-bundled-v8.')
2007+
if options.enable_v8windbg:
2008+
raise Exception('--enable-v8windbg is incompatible with --without-bundled-v8.')
2009+
(pkg_libs, pkg_cflags, pkg_libpath, _) = pkg_config("v8")
2010+
if pkg_libs and pkg_libpath:
2011+
output['libraries'] += [pkg_libpath] + pkg_libs.split()
2012+
if pkg_cflags:
2013+
output['include_dirs'] += [flag for flag in [flag.strip() for flag in pkg_cflags.split('-I')] if flag]
20082014
if options.static_zoslib_gyp:
20092015
o['variables']['static_zoslib_gyp'] = options.static_zoslib_gyp
20102016
if flavor != 'linux' and options.v8_enable_hugepage:

‎node.gyp‎

Lines changed: 18 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -641,7 +641,6 @@
641641
'xcode_settings': {
642642
'OTHER_LDFLAGS': [
643643
'-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)<(node_core_target_name)<(STATIC_LIB_SUFFIX)',
644-
'-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)v8_base_without_compiler<(STATIC_LIB_SUFFIX)',
645644
],
646645
},
647646
'msvs_settings': {
@@ -653,14 +652,28 @@
653652
},
654653
},
655654
'conditions': [
656-
['OS != "aix" and OS != "os400" and OS != "mac" and OS != "ios"', {
655+
['node_use_bundled_v8=="true"', {
656+
'xcode_settings': {
657+
'OTHER_LDFLAGS': [
658+
'-Wl,-force_load,<(PRODUCT_DIR)/<(STATIC_LIB_PREFIX)v8_base_without_compiler<(STATIC_LIB_SUFFIX)',
659+
],
660+
},
661+
}],
662+
['node_use_bundled_v8=="true" and OS != "aix" and OS != "os400" and OS != "mac" and OS != "ios"', {
657663
'ldflags': [
658664
'-Wl,--whole-archive',
659665
'<(obj_dir)/<(STATIC_LIB_PREFIX)<(node_core_target_name)<(STATIC_LIB_SUFFIX)',
660666
'<(obj_dir)/tools/v8_gypfiles/<(STATIC_LIB_PREFIX)v8_base_without_compiler<(STATIC_LIB_SUFFIX)',
661667
'-Wl,--no-whole-archive',
662668
],
663669
}],
670+
['node_use_bundled_v8!="true" and OS != "aix" and OS != "os400" and OS != "mac" and OS != "ios"', {
671+
'ldflags': [
672+
'-Wl,--whole-archive',
673+
'<(obj_dir)/<(STATIC_LIB_PREFIX)<(node_core_target_name)<(STATIC_LIB_SUFFIX)',
674+
'-Wl,--no-whole-archive',
675+
],
676+
}],
664677
[ 'OS=="win"', {
665678
'sources': [ 'src/res/node.rc' ],
666679
}],
@@ -1530,6 +1543,9 @@
15301543
'src/builtin_info.cc',
15311544
],
15321545
'conditions': [
1546+
[ 'OS=="mac"', {
1547+
'libraries': [ '-framework CoreFoundation -framework Security' ],
1548+
}],
15331549
[ 'node_shared_simdutf=="false"', {
15341550
'dependencies': [ 'tools/v8_gypfiles/v8.gyp:simdutf#host' ],
15351551
}],

‎node.gypi‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -317,8 +317,8 @@
317317
'NODE_PLATFORM="sunos"',
318318
],
319319
}],
320-
[ '(OS=="freebsd" or OS=="linux" or OS=="openharmony") and node_shared=="false"'
321-
' and force_load=="true"', {
320+
[ 'node_use_bundled_v8=="true" and (OS=="freebsd" or OS=="linux" or OS=="openharmony") '
321+
'and node_shared=="false" and force_load=="true"', {
322322
'ldflags': [
323323
'-Wl,-z,noexecstack',
324324
'-Wl,--whole-archive <(v8_base)',

‎shell.nix‎

Lines changed: 56 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@
88
extraConfigFlags ? [
99
"--debug-node"
1010
],
11+
useSeparateDerivationForV8 ? false, # to help CI better managed its binary cache, not recommended outside of CI usage.
1112

1213
# Build options
1314
icu ? pkgs.icu,
@@ -17,16 +18,21 @@
1718
withSQLite ? true,
1819
withSSL ? true,
1920
withTemporal ? false,
20-
sharedLibDeps ? import ./tools/nix/sharedLibDeps.nix {
21-
inherit
22-
pkgs
23-
withLief
24-
withQuic
25-
withSQLite
26-
withSSL
27-
withTemporal
28-
;
29-
},
21+
sharedLibDeps ?
22+
let
23+
d = import ./tools/nix/sharedLibDeps.nix {
24+
inherit
25+
pkgs
26+
withLief
27+
withQuic
28+
withSQLite
29+
withSSL
30+
withTemporal
31+
;
32+
};
33+
in
34+
# To avoid conflicts with V8's bundled simdutf lib, it's easier to remove it when using a precompiled V8.
35+
if (useSeparateDerivationForV8 != false) then builtins.removeAttrs d [ "simdutf" ] else d,
3036

3137
# dev tools (not needed to build Node.js, useful to maintain it)
3238
ncu-path ? null, # Provide this if you want to use a local version of NCU
@@ -40,11 +46,49 @@ let
4046
useSharedOpenSSL = builtins.hasAttr "openssl" sharedLibDeps;
4147

4248
needsRustCompiler = withTemporal && !builtins.hasAttr "temporal_capi" sharedLibDeps;
49+
50+
buildInputs = builtins.attrValues sharedLibDeps ++ pkgs.lib.optional useSharedICU icu;
51+
configureFlags = [
52+
(
53+
if icu == null then
54+
"--without-intl"
55+
else
56+
"--with-intl=${if useSharedICU then "system" else icu}-icu"
57+
)
58+
]
59+
++ extraConfigFlags
60+
++ pkgs.lib.optional (!withAmaro) "--without-amaro"
61+
++ pkgs.lib.optional (!withLief) "--without-lief"
62+
++ pkgs.lib.optional withQuic "--experimental-quic"
63+
++ pkgs.lib.optional (!withSQLite) "--without-sqlite"
64+
++ pkgs.lib.optional (!withSSL) "--without-ssl"
65+
++ pkgs.lib.optional withTemporal "--v8-enable-temporal-support"
66+
++ pkgs.lib.optional (ninja != null) "--ninja"
67+
++ pkgs.lib.optional loadJSBuiltinsDynamically "--node-builtin-modules-path=${builtins.toString ../..}"
68+
++ pkgs.lib.concatMap (name: [
69+
"--shared-${name}"
70+
"--shared-${name}-libpath=${pkgs.lib.getLib sharedLibDeps.${name}}/lib"
71+
"--shared-${name}-include=${pkgs.lib.getInclude sharedLibDeps.${name}}/include"
72+
]) (builtins.attrNames sharedLibDeps);
4373
in
4474
pkgs.mkShell {
4575
inherit (pkgs.nodejs_latest) nativeBuildInputs;
4676

47-
buildInputs = builtins.attrValues sharedLibDeps ++ pkgs.lib.optional useSharedICU icu;
77+
buildInputs =
78+
buildInputs
79+
++ pkgs.lib.optional (useSeparateDerivationForV8 != false) (
80+
if useSeparateDerivationForV8 == true then
81+
import ./tools/nix/v8.nix {
82+
inherit
83+
pkgs
84+
configureFlags
85+
buildInputs
86+
needsRustCompiler
87+
;
88+
}
89+
else
90+
useSeparateDerivationForV8
91+
);
4892

4993
packages =
5094
pkgs.lib.optional (ccache != null) ccache
@@ -74,28 +118,7 @@ pkgs.mkShell {
74118
]
75119
);
76120
CONFIG_FLAGS = builtins.toString (
77-
[
78-
(
79-
if icu == null then
80-
"--without-intl"
81-
else
82-
"--with-intl=${if useSharedICU then "system" else icu}-icu"
83-
)
84-
]
85-
++ extraConfigFlags
86-
++ pkgs.lib.optional (!withAmaro) "--without-amaro"
87-
++ pkgs.lib.optional (!withLief) "--without-lief"
88-
++ pkgs.lib.optional withQuic "--experimental-quic"
89-
++ pkgs.lib.optional (!withSQLite) "--without-sqlite"
90-
++ pkgs.lib.optional (!withSSL) "--without-ssl"
91-
++ pkgs.lib.optional withTemporal "--v8-enable-temporal-support"
92-
++ pkgs.lib.optional (ninja != null) "--ninja"
93-
++ pkgs.lib.optional loadJSBuiltinsDynamically "--node-builtin-modules-path=${builtins.toString ./.}"
94-
++ pkgs.lib.concatMap (name: [
95-
"--shared-${name}"
96-
"--shared-${name}-libpath=${pkgs.lib.getLib sharedLibDeps.${name}}/lib"
97-
"--shared-${name}-include=${pkgs.lib.getInclude sharedLibDeps.${name}}/include"
98-
]) (builtins.attrNames sharedLibDeps)
121+
configureFlags ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8"
99122
);
100123
NOSQLITE = pkgs.lib.optionalString (!withSQLite) "1";
101124
}

‎tools/nix/v8.nix‎

Lines changed: 127 additions & 0 deletions

0 commit comments

Comments
 (0)