Skip to content

Commit d5279e5

Browse files
aduh95aduh95
authored
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 a73cda9 commit d5279e5

6 files changed

Lines changed: 221 additions & 51 deletions

File tree

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

Lines changed: 5 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -182,25 +182,20 @@ jobs:
182182
core.exportVariable('ACTIONS_CACHE_SERVICE_V2', 'on');
183183
core.exportVariable('ACTIONS_RESULTS_URL', process.env.ACTIONS_RESULTS_URL || '');
184184
core.exportVariable('ACTIONS_RUNTIME_TOKEN', process.env.ACTIONS_RUNTIME_TOKEN || '');
185-
186-
- name: Load shell.nix
187-
if: github.event_name != 'workflow_dispatch'
188-
run: |
189-
mv "$TAR_DIR"/*.nix .
190-
mkdir tools
191-
mv "$TAR_DIR"/tools/nix tools/.
185+
core.exportVariable('NIX_SCCACHE', '(import <nixpkgs> {}).sccache');
192186
193187
- name: Build Node.js and run tests
194188
run: |
195189
nix-shell \
196-
-I nixpkgs=./tools/nix/pkgs.nix \
190+
-I "nixpkgs=$TAR_DIR/tools/nix/pkgs.nix" \
197191
--pure --keep TAR_DIR --keep FLAKY_TESTS \
198192
--keep SCCACHE_GHA_ENABLED --keep ACTIONS_CACHE_SERVICE_V2 --keep ACTIONS_RESULTS_URL --keep ACTIONS_RUNTIME_TOKEN \
199193
--arg loadJSBuiltinsDynamically false \
200-
--arg ccache '(import <nixpkgs> {}).sccache' \
194+
--arg useSeparateDerivationForV8 true \
195+
--arg ccache "${NIX_SCCACHE:-null}" \
201196
--arg devTools '[]' \
202197
--arg benchmarkTools '[]' \
203198
${{ endsWith(matrix.system, '-darwin') && '--arg withAmaro false --arg withLief false --arg withSQLite false --arg extraConfigFlags ''["--without-inspector" "--without-node-options"]'' \' || '\' }}
204199
--run '
205200
make -C "$TAR_DIR" run-ci -j4 V=1 TEST_CI_ARGS="-p actions --measure-flakiness 9 --skip-tests=$CI_SKIP_TESTS"
206-
'
201+
' "$TAR_DIR/shell.nix"

‎configure.py‎

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -2064,10 +2064,16 @@ def configure_v8(o, configs):
20642064
o['variables']['node_enable_v8windbg'] = b(options.enable_v8windbg)
20652065
if options.enable_d8:
20662066
o['variables']['test_isolation_mode'] = 'noop' # Needed by d8.gyp.
2067-
if options.without_bundled_v8 and options.enable_d8:
2068-
raise Exception('--enable-d8 is incompatible with --without-bundled-v8.')
2069-
if options.without_bundled_v8 and options.enable_v8windbg:
2070-
raise Exception('--enable-v8windbg is incompatible with --without-bundled-v8.')
2067+
if options.without_bundled_v8:
2068+
if options.enable_d8:
2069+
raise Exception('--enable-d8 is incompatible with --without-bundled-v8.')
2070+
if options.enable_v8windbg:
2071+
raise Exception('--enable-v8windbg is incompatible with --without-bundled-v8.')
2072+
(pkg_libs, pkg_cflags, pkg_libpath, _) = pkg_config("v8")
2073+
if pkg_libs and pkg_libpath:
2074+
output['libraries'] += [pkg_libpath] + pkg_libs.split()
2075+
if pkg_cflags:
2076+
output['include_dirs'] += [flag for flag in [flag.strip() for flag in pkg_cflags.split('-I')] if flag]
20712077
if options.static_zoslib_gyp:
20722078
o['variables']['static_zoslib_gyp'] = options.static_zoslib_gyp
20732079
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: 130 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,130 @@
1+
# Derivation for Node.js CI (not officially supported for regular applications)
2+
{
3+
pkgs ? import ./pkgs.nix { },
4+
buildInputs ? [ ],
5+
configureFlags ? [ ],
6+
needsRustCompiler ? false,
7+
}:
8+
9+
let
10+
nodejs = pkgs.nodejs-slim_latest;
11+
12+
version =
13+
let
14+
v8Version = builtins.match (
15+
".*#define V8_MAJOR_VERSION ([0-9]+).*"
16+
+ "#define V8_MINOR_VERSION ([0-9]+).*"
17+
+ "#define V8_BUILD_NUMBER ([0-9]+).*"
18+
+ "#define V8_PATCH_LEVEL ([0-9]+).*"
19+
) (builtins.readFile ../../deps/v8/include/v8-version.h);
20+
v8_embedder_string = builtins.match ".*'v8_embedder_string': '-(node.[0-9]+)'.*" (
21+
builtins.readFile ../../common.gypi
22+
);
23+
in
24+
if v8Version == null || v8_embedder_string == null then
25+
throw "V8 version not found"
26+
else
27+
"${builtins.elemAt v8Version 0}.${builtins.elemAt v8Version 1}.${builtins.elemAt v8Version 2}.${builtins.elemAt v8Version 3}-${builtins.elemAt v8_embedder_string 0}";
28+
in
29+
pkgs.stdenv.mkDerivation (finalAttrs: {
30+
pname = "v8";
31+
inherit version;
32+
src =
33+
let
34+
inherit (pkgs.lib) fileset;
35+
in
36+
fileset.toSource {
37+
root = ../../.;
38+
fileset = fileset.unions [
39+
../../common.gypi
40+
../../configure
41+
../../configure.py
42+
../../deps/inspector_protocol/inspector_protocol.gyp
43+
../../deps/ncrypto/ncrypto.gyp
44+
../../deps/v8
45+
../../node.gyp
46+
../../node.gypi
47+
../../src/inspector/node_inspector.gypi
48+
../../src/node_version.h
49+
../../tools/configure.d
50+
../../tools/getmoduleversion.py
51+
../../tools/getnapibuildversion.py
52+
../../tools/gyp
53+
../../tools/gyp_node.py
54+
../../tools/icu/icu_versions.json
55+
../../tools/icu/icu-system.gyp
56+
../../tools/utils.py
57+
../../tools/v8_gypfiles
58+
];
59+
};
60+
61+
# We need to patch tools/gyp/ to work from within Nix sandbox
62+
prePatch = ''
63+
patches=()
64+
for patch in ${pkgs.lib.concatStringsSep " " nodejs.patches}; do
65+
filtered=$(mktemp)
66+
filterdiff -p1 -i 'tools/gyp/*' "$patch" > "$filtered"
67+
if [ -s "$filtered" ]; then
68+
patches+=("$filtered")
69+
fi
70+
done
71+
'';
72+
73+
inherit (nodejs) configureScript;
74+
inherit configureFlags buildInputs;
75+
76+
nativeBuildInputs =
77+
nodejs.nativeBuildInputs
78+
++ [
79+
pkgs.patchutils
80+
pkgs.validatePkgConfig
81+
]
82+
++ pkgs.lib.optionals needsRustCompiler [
83+
pkgs.cargo
84+
pkgs.rustc
85+
];
86+
87+
buildPhase = ''
88+
ninja -v -C out/Release v8_snapshot v8_libplatform
89+
'';
90+
installPhase = ''
91+
${
92+
if pkgs.stdenv.buildPlatform.isDarwin then
93+
# Darwin is excluded from creating thin archive in tools/gyp/pylib/gyp/generator/ninja.py:2488
94+
"install -Dm644 out/Release/lib* -t $out/lib"
95+
else
96+
# On other platforms, we need to create non-thin archive.
97+
''
98+
mkdir -p $out/lib
99+
for a in out/Release/obj/tools/v8_gypfiles/lib*; do
100+
base=$(basename "$a")
101+
dir=$(dirname "$a")
102+
103+
(
104+
cd "$dir"
105+
"$AR" rc "$out/lib/$base" $("$AR" t "$base")
106+
)
107+
108+
"$RANLIB" "$out/lib/$base"
109+
done
110+
''
111+
}
112+
libs=$(for f in $out/lib/lib*.a; do
113+
b=$(basename "$f" .a)
114+
printf " -l%s" "''${b#lib}"
115+
done)
116+
117+
# copy v8 headers
118+
cp -r deps/v8/include $out/
119+
120+
# create a pkgconfig file for v8
121+
mkdir -p $out/lib/pkgconfig
122+
cat -> $out/lib/pkgconfig/v8.pc << EOF
123+
Name: v8
124+
Description: V8 JavaScript Engine build for Node.js CI
125+
Version: ${version}
126+
Libs: -L$out/lib $libs
127+
Cflags: -I$out/include
128+
EOF
129+
'';
130+
})

0 commit comments

Comments
 (0)