Skip to content

Commit 4769b86

Browse files
aduh95aduh95
committed
tools: make v8.nix more stable
If we only pass to the V8 builders the dependencies it actually needs, we can avoid rebuilding the same version of V8 just because an unrelated Node.js dependency has been updated.
1 parent 3cf5b62 commit 4769b86

File tree

5 files changed

+196
-83
lines changed

5 files changed

+196
-83
lines changed

‎shell.nix‎

Lines changed: 65 additions & 47 deletions
Original file line numberDiff line numberDiff line change
@@ -18,21 +18,18 @@
1818
withSQLite ? true,
1919
withSSL ? true,
2020
withTemporal ? false,
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,
21+
sharedLibDeps ? (
22+
import ./tools/nix/sharedLibDeps.nix {
23+
inherit
24+
pkgs
25+
withLief
26+
withQuic
27+
withSQLite
28+
withSSL
29+
withTemporal
30+
;
31+
}
32+
),
3633

3734
# dev tools (not needed to build Node.js, useful to maintain it)
3835
ncu-path ? null, # Provide this if you want to use a local version of NCU
@@ -45,9 +42,20 @@ let
4542
useSharedAda = builtins.hasAttr "ada" sharedLibDeps;
4643
useSharedOpenSSL = builtins.hasAttr "openssl" sharedLibDeps;
4744

48-
needsRustCompiler = withTemporal && !builtins.hasAttr "temporal_capi" sharedLibDeps;
45+
useSharedTemporal = builtins.hasAttr "temporal_capi" sharedLibDeps;
46+
needsRustCompiler = withTemporal && !useSharedTemporal;
4947

50-
buildInputs = builtins.attrValues sharedLibDeps ++ pkgs.lib.optional useSharedICU icu;
48+
nativeBuildInputs =
49+
pkgs.nodejs-slim_latest.nativeBuildInputs
50+
++ pkgs.lib.optionals needsRustCompiler [
51+
pkgs.cargo
52+
pkgs.rustc
53+
];
54+
buildInputs =
55+
pkgs.lib.optional useSharedICU icu
56+
++ pkgs.lib.optional (withTemporal && useSharedTemporal) sharedLibDeps.temporal_capi;
57+
58+
# Put here only the configure flags that affect the V8 build
5159
configureFlags = [
5260
(
5361
if icu == null then
@@ -57,47 +65,31 @@ let
5765
)
5866
]
5967
++ 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"
6568
++ 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);
69+
++ pkgs.lib.optional (withTemporal && useSharedTemporal) "--shared-temporal_capi";
7370
in
7471
pkgs.mkShell {
75-
inherit (pkgs.nodejs-slim_latest) nativeBuildInputs;
72+
inherit nativeBuildInputs;
7673

7774
buildInputs =
78-
buildInputs
75+
builtins.attrValues sharedLibDeps
76+
++ buildInputs
7977
++ pkgs.lib.optional (useSeparateDerivationForV8 != false) (
8078
if useSeparateDerivationForV8 == true then
81-
import ./tools/nix/v8.nix {
82-
inherit
83-
pkgs
84-
configureFlags
85-
buildInputs
86-
needsRustCompiler
87-
;
79+
let
80+
sharedLibsToMock = pkgs.callPackage ./tools/nix/non-v8-deps-mock.nix { };
81+
in
82+
pkgs.callPackage ./tools/nix/v8.nix {
83+
inherit nativeBuildInputs;
84+
85+
configureFlags = configureFlags ++ sharedLibsToMock.configureFlags ++ [ "--ninja" ];
86+
buildInputs = buildInputs ++ [ sharedLibsToMock ];
8887
}
8988
else
9089
useSeparateDerivationForV8
9190
);
9291

93-
packages =
94-
pkgs.lib.optional (ccache != null) ccache
95-
++ devTools
96-
++ benchmarkTools
97-
++ pkgs.lib.optionals needsRustCompiler [
98-
pkgs.cargo
99-
pkgs.rustc
100-
];
92+
packages = devTools ++ benchmarkTools ++ pkgs.lib.optional (ccache != null) ccache;
10193

10294
shellHook = pkgs.lib.optionalString (ccache != null) ''
10395
export CC="${pkgs.lib.getExe ccache} $CC"
@@ -107,7 +99,33 @@ pkgs.mkShell {
10799
BUILD_WITH = if (ninja != null) then "ninja" else "make";
108100
NINJA = pkgs.lib.optionalString (ninja != null) "${pkgs.lib.getExe ninja}";
109101
CONFIG_FLAGS = builtins.toString (
110-
configureFlags ++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8"
102+
configureFlags
103+
++ pkgs.lib.optional (ninja != null) "--ninja"
104+
++ pkgs.lib.optional (!withAmaro) "--without-amaro"
105+
++ pkgs.lib.optional (!withLief) "--without-lief"
106+
++ pkgs.lib.optional withQuic "--experimental-quic"
107+
++ pkgs.lib.optional (!withSQLite) "--without-sqlite"
108+
++ pkgs.lib.optional (!withSSL) "--without-ssl"
109+
++ pkgs.lib.optional loadJSBuiltinsDynamically "--node-builtin-modules-path=${builtins.toString ./.}"
110+
++ pkgs.lib.optional (useSeparateDerivationForV8 != false) "--without-bundled-v8"
111+
++
112+
pkgs.lib.concatMap
113+
(name: [
114+
"--shared-${name}"
115+
"--shared-${name}-libpath=${pkgs.lib.getLib sharedLibDeps.${name}}/lib"
116+
"--shared-${name}-include=${pkgs.lib.getInclude sharedLibDeps.${name}}/include"
117+
])
118+
(
119+
builtins.attrNames (
120+
if (useSeparateDerivationForV8 != false) then
121+
builtins.removeAttrs sharedLibDeps [
122+
"simdutf"
123+
"temporal_capi"
124+
]
125+
else
126+
sharedLibDeps
127+
)
128+
)
111129
);
112130
NOSQLITE = pkgs.lib.optionalString (!withSQLite) "1";
113131
}

‎tools/dep_updaters/update-gyp-next.sh‎

Lines changed: 10 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -39,16 +39,24 @@ GYP_NEXT_TARBALL="$NEW_VERSION.tar.gz"
3939

4040
cd "$WORKSPACE"
4141

42-
curl -sL -o "$GYP_NEXT_TARBALL" "https://github.com/nodejs/gyp-next/archive/refs/tags/v$NEW_VERSION.tar.gz"
42+
URL="https://github.com/nodejs/gyp-next/archive/refs/tags/v$NEW_VERSION.tar.gz"
43+
curl -sL -o "$GYP_NEXT_TARBALL" "$URL"
4344

44-
log_and_verify_sha256sum "gyp-next" "$GYP_NEXT_TARBALL"
45+
HASH=$(log_and_verify_sha256sum "gyp-next" "$GYP_NEXT_TARBALL")
4546

4647
gzip -dc "$GYP_NEXT_TARBALL" | tar xf -
4748

4849
rm "$GYP_NEXT_TARBALL"
4950

5051
mv "gyp-next-$NEW_VERSION" gyp
5152

53+
cat -> "$WORKSPACE/gyp/src.nix" <<EOF
54+
builtins.fetchurl {
55+
url = "$URL";
56+
sha256 = "${HASH##*= }";
57+
}
58+
EOF
59+
5260
rm -rf "$WORKSPACE/gyp/.github"
5361

5462
rm -rf "$BASE_DIR/tools/gyp"

‎tools/gyp/src.nix‎

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,4 @@
1+
builtins.fetchurl {
2+
url = "https://github.com/nodejs/gyp-next/archive/refs/tags/v0.21.1.tar.gz";
3+
sha256 = "d75d7a8365e823292d94d80ea2178aa37897272af275a71ce32493d931178caf";
4+
}

‎tools/nix/non-v8-deps-mock.nix‎

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,76 @@
1+
{
2+
symlinkJoin,
3+
writeTextFile,
4+
validatePkgConfig,
5+
testers,
6+
lib,
7+
}:
8+
9+
let
10+
sharedLibsToMock = {
11+
zlib = [ "zlib" ];
12+
http-parser = [ "libllhttp" ];
13+
libuv = [ "libuv" ];
14+
ada = [ "ada" ];
15+
simdjson = [ "simdjson" ];
16+
brotli = [
17+
"libbrotlidec"
18+
"libbrotlienc"
19+
];
20+
cares = [ "libcares" ];
21+
gtest = [ "gtest" ];
22+
hdr-histogram = [ "hdr_histogram" ];
23+
merve = [ "merve" ];
24+
nbytes = [ "nbytes" ];
25+
nghttp2 = [ "libnghttp2" ];
26+
nghttp3 = [ "libnghttp3" ];
27+
ngtcp2 = [ "libngtcp2" ];
28+
uvwasi = [ "uvwasi" ];
29+
zstd = [ "libzstd" ];
30+
};
31+
in
32+
symlinkJoin (finalAttrs: {
33+
name = "non-v8-deps-mock";
34+
version = "0.0.0-mock";
35+
36+
nativeBuildInputs = [ validatePkgConfig ];
37+
38+
paths = lib.concatMap (
39+
sharedLibName:
40+
(builtins.map (
41+
pkgName:
42+
writeTextFile {
43+
name = "mock-${pkgName}.pc";
44+
destination = "/lib/pkgconfig/${pkgName}.pc";
45+
text = ''
46+
Name: ${pkgName}
47+
Description: Mock package for ${sharedLibName}
48+
Version: ${finalAttrs.version}
49+
Libs:
50+
Cflags:
51+
'';
52+
}
53+
) sharedLibsToMock.${sharedLibName})
54+
) (builtins.attrNames sharedLibsToMock);
55+
passthru = {
56+
configureFlags = [
57+
"--without-lief"
58+
"--without-sqlite"
59+
"--without-ssl"
60+
]
61+
++ (lib.concatMap (sharedLibName: [
62+
"--shared-${sharedLibName}"
63+
"--shared-${sharedLibName}-libname="
64+
]) (builtins.attrNames sharedLibsToMock));
65+
66+
tests.pkg-config = testers.hasPkgConfigModules {
67+
package = finalAttrs.finalPackage;
68+
};
69+
};
70+
71+
meta = {
72+
description = "Mock of Node.js dependencies that are not needed for building V8";
73+
license = lib.licenses.mit;
74+
pkgConfigModules = lib.concatMap (x: x) (builtins.attrValues sharedLibsToMock);
75+
};
76+
})

‎tools/nix/v8.nix‎

Lines changed: 41 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,15 +1,24 @@
11
# Derivation for Node.js CI (not officially supported for regular applications)
22
{
3-
pkgs ? import ./pkgs.nix { },
3+
stdenv,
4+
lib,
5+
patchutils,
6+
validatePkgConfig,
7+
nodejs-slim_latest,
8+
49
buildInputs ? [ ],
510
configureFlags ? [ ],
6-
needsRustCompiler ? false,
11+
12+
configureScript ? nodejs-slim_latest.configureScript,
13+
nativeBuildInputs ? nodejs-slim_latest.nativeBuildInputs,
14+
patches ? nodejs-slim_latest.patches,
715
}:
816

917
let
10-
nodejs = pkgs.nodejs-slim_latest;
1118
v8Dir = ../../deps/v8;
12-
19+
in
20+
stdenv.mkDerivation (finalAttrs: {
21+
pname = "v8";
1322
version =
1423
let
1524
v8Version = builtins.match (
@@ -26,71 +35,69 @@ let
2635
throw "V8 version not found"
2736
else
2837
"${builtins.elemAt v8Version 0}.${builtins.elemAt v8Version 1}.${builtins.elemAt v8Version 2}.${builtins.elemAt v8Version 3}-${builtins.elemAt v8_embedder_string 0}";
29-
in
30-
pkgs.stdenv.mkDerivation (finalAttrs: {
31-
pname = "v8";
32-
inherit version;
3338
src =
3439
let
35-
inherit (pkgs.lib) fileset;
40+
inherit (lib) fileset;
3641
in
3742
fileset.toSource {
3843
root = ../../.;
3944
fileset = fileset.unions [
45+
v8Dir
4046
../../common.gypi
41-
../../configure
4247
../../configure.py
43-
../../deps/inspector_protocol/inspector_protocol.gyp
44-
../../deps/ncrypto/ncrypto.gyp
45-
v8Dir
4648
../../node.gyp
4749
../../node.gypi
48-
../../src/inspector/node_inspector.gypi
4950
../../src/node_version.h
50-
../../tools/configure.d
51+
../../tools/configure.d/nodedownload.py
5152
../../tools/getmoduleversion.py
5253
../../tools/getnapibuildversion.py
53-
../../tools/gyp
5454
../../tools/gyp_node.py
5555
../../tools/icu/icu_versions.json
5656
../../tools/icu/icu-system.gyp
5757
../../tools/utils.py
58-
../../tools/v8_gypfiles
58+
../../tools/v8_gypfiles/abseil.gyp
59+
../../tools/v8_gypfiles/features.gypi
60+
../../tools/v8_gypfiles/ForEachFormat.py
61+
../../tools/v8_gypfiles/ForEachReplace.py
62+
../../tools/v8_gypfiles/GN-scraper.py
63+
../../tools/v8_gypfiles/inspector.gypi
64+
../../tools/v8_gypfiles/toolchain.gypi
65+
../../tools/v8_gypfiles/v8.gyp
5966
];
6067
};
6168

62-
# We need to patch tools/gyp/ to work from within Nix sandbox
69+
# We need to download and patch GYP to work from within Nix sandbox
70+
# and so the local pycache does not pollute the hash.
6371
prePatch = ''
6472
patches=()
65-
for patch in ${pkgs.lib.concatStringsSep " " nodejs.patches}; do
73+
for patch in ${lib.concatStringsSep " " patches}; do
6674
filtered=$(mktemp)
67-
filterdiff -p1 -i 'tools/gyp/*' "$patch" > "$filtered"
75+
filterdiff -p1 -i 'tools/gyp/pylib/*' "$patch" > "$filtered"
6876
if [ -s "$filtered" ]; then
6977
patches+=("$filtered")
7078
fi
7179
done
80+
tar -C tools -xzf ${import ../../tools/gyp/src.nix} --wildcards 'gyp-*/pylib'
81+
mv tools/gyp-* tools/gyp
82+
'';
83+
# We need to remove the node_inspector.gypi ref so GYP does not search for it.
84+
postPatch = ''
85+
substituteInPlace node.gyp --replace-fail "'includes' : [ 'src/inspector/node_inspector.gypi' ]" "'includes' : []"
7286
'';
7387

74-
inherit (nodejs) configureScript;
75-
inherit configureFlags buildInputs;
88+
inherit configureScript configureFlags buildInputs;
7689

77-
nativeBuildInputs =
78-
nodejs.nativeBuildInputs
79-
++ [
80-
pkgs.patchutils
81-
pkgs.validatePkgConfig
82-
]
83-
++ pkgs.lib.optionals needsRustCompiler [
84-
pkgs.cargo
85-
pkgs.rustc
86-
];
90+
nativeBuildInputs = nativeBuildInputs ++ [
91+
patchutils
92+
validatePkgConfig
93+
];
8794

8895
buildPhase = ''
8996
ninja -v -C out/Release v8_snapshot v8_libplatform
9097
'';
9198
installPhase = ''
9299
${
93-
if pkgs.stdenv.hostPlatform.isDarwin then
100+
if stdenv.hostPlatform.isDarwin then
94101
# Darwin is excluded from creating thin archive in tools/gyp/pylib/gyp/generator/ninja.py:2488
95102
"install -Dm644 out/Release/lib* -t $out/lib"
96103
else
@@ -115,7 +122,7 @@ pkgs.stdenv.mkDerivation (finalAttrs: {
115122
cat -> $out/lib/pkgconfig/v8.pc << EOF
116123
Name: v8
117124
Description: V8 JavaScript Engine build for Node.js CI
118-
Version: ${version}
125+
Version: ${finalAttrs.version}
119126
Libs: -L$out/lib $(for f in $out/lib/lib*.a; do
120127
b=$(basename "$f" .a)
121128
printf " -l%s" "''${b#lib}"

0 commit comments

Comments
 (0)