Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 7 additions & 1 deletion AGENTS.md
Original file line number Diff line number Diff line change
Expand Up @@ -73,7 +73,13 @@ Bump `upstream-base.txt`, run `prepare.sh`, resolve conflicts in `out/`,
regenerate. Expect a handful of small conflicts. **A clean `git am` is not
proof of correctness** — an upstream restructure can leave a platform guard
(`#if TARGET_OS_OSX`) enclosing the wrong span, which only the compile
catches. → [docs/UPGRADING.md](./docs/UPGRADING.md)
catches. Two things bite every time: `prepare.sh`'s `--depth 1` clone has no
pre-image blobs, so a patch on an upstream-modified file dies with `sha1
information is lacking or useless` until you `git -C out fetch --depth 1
origin tag <old base>`; and `prepare.sh` can't resume, so after a conflict
you apply the rest of the series *and* redo the `mobile-src` overlay by hand.
New upstream tests need `.status` triage before the PR, not after — the full
device suite gates the merge. → [docs/UPGRADING.md](./docs/UPGRADING.md)

## Gotchas

Expand Down
9 changes: 8 additions & 1 deletion docs/TESTING.md
Original file line number Diff line number Diff line change
Expand Up @@ -410,9 +410,16 @@ A test that runs with `--permission` but without `--allow-fs-write` cannot be
scored when it calls `process.exit()`: the harness writes its verdict to a file
from a `process.on('exit')` hook, and the permission model — correctly — denies
that write, so no verdict lands and the run reports FAIL whatever the test did.
This is not fixable from inside the sandbox doing the denying. The 22 affected
This is not fixable from inside the sandbox doing the denying. The affected
cases are skipped, with that reason recorded next to them in `parallel.status`.

The flags header is not the whole story: a test that starts with
`--allow-fs-write` and then calls `process.permission.drop('fs.write')` (or
`drop('fs')`) has the same problem from that point on, and the exit hook runs
after it. Read the `drop()` calls, not just the `// Flags:` line. Every
upstream release adds a few of these — → [UPGRADING.md](./UPGRADING.md#new-upstream-tests)
for the triage to run when the base moves.

### The NAPI addon gate

After the curated subset, each device workflow builds the **crc-native** N-API
Expand Down
139 changes: 129 additions & 10 deletions docs/UPGRADING.md
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@ occasionally, one that only the compile catches (see the warning below).
## Re-base the series

```sh
$EDITOR upstream-base.txt # bump the tag, e.g. v24.19.0
$EDITOR upstream-base.txt # bump the tag, e.g. v24.20.0
scripts/prepare.sh # clone new base + apply series
```

Expand Down Expand Up @@ -43,21 +43,142 @@ everything it needs to.
Also:

- check `.github/workflows/` in `out/` for **new upstream workflows** the
removal patch doesn't cover yet — delete-and-own them in the upstream-CI
removal patch (`ci-remove-upstream-only-workflows-and-config.patch`); a
removal patch doesn't cover yet — `git rm` them in `out/` *and* add each
path to `patches/files.map` against the upstream-CI removal patch
(`ci-remove-upstream-only-workflows-and-config.patch`), or
`regenerate-patches.py` rejects the deletion as owned by no patch. A
`verify-patches` job asserts the materialized tree carries zero workflow
files, so a missed one fails loudly.
- add `.status` entries for the tests upstream just added — see [new upstream
tests](#new-upstream-tests) below.
- watch for upstream **splitting a build target**. gyp resolves the new
dependency on Android, but the iOS framework links a hand-maintained list of
archives — `outputs_common` in `mobile-src/tools/ios_framework_prepare.sh`
plus four entries in the `NodeMobile.xcodeproj` pbxproj — so a new
`static_library` shows up as undefined symbols at `Ld`, long after every
compile has passed. v24.19.0 split `libnode_base` out of `libnode`;
v24.20.0 split `libncrypto_engine` out of `libncrypto`. If an iOS leg
fails with `Undefined symbols` naming a class you did not touch, check
`deps/*/*.gyp` for a target that did not exist at the old base. Note the
pbxproj is `.gitignore`d inside the tree, so staging it needs `git add -f`.

### `sha1 information is lacking or useless`

A patch that stops with

Then regenerate and commit:
```
error: sha1 information is lacking or useless (README.md).
error: could not build fake ancestor
```

has not conflicted. `--3way` needs the *pre-image* blob the patch header
records, and `prepare.sh` clones `--depth 1`, so `out/` holds only the new
tag's blobs — for any file upstream touched since the old base, git has
nothing to merge against. Fetch the base you are moving off to supply them:

```sh
git -C out fetch --depth 1 origin tag v24.19.0
```

Retry the patch and it either applies or fails as an ordinary content
conflict you can resolve. Do this before editing anything by hand: with the
old blobs present, `git am --3way` absorbs most upstream drift on its own.

### Finishing the series by hand

`prepare.sh` refuses to run when `out/` exists, so it cannot be re-run to
resume. Once you have resolved the conflict, land it and apply the rest of
`patches/series` yourself with the same invocation the script uses:

```sh
git -C out am --continue
git -C out am --3way --keep-cr --whitespace=nowarn patches/<each remaining>.patch
```

Then redo step 3, the `mobile-src/` overlay, which never ran. It is not
optional — without that commit `out/` is upstream plus patches, and
`regenerate-patches.py` reads every fork-only file as deleted:

```sh
( cd mobile-src && git ls-files -z --cached --others --exclude-standard . ) > /tmp/manifest
( cd mobile-src && tar cf - --null -T /tmp/manifest ) | ( cd out && tar xf - )
git -C out add -f --pathspec-from-file=/tmp/manifest --pathspec-file-nul
git -C out commit -m "mobile: fork-only files (mobile-src overlay)"
```

### Regenerate and commit

```sh
git -C out add -A && git -C out commit -m "resolve v24.19.0 conflicts" # any shape
git -C out add -A && git -C out commit -m "resolve v24.20.0 conflicts" # any shape
scripts/regenerate-patches.py out # re-emits patches/ + syncs mobile-src/
# update expected-tree.txt to the hash the script prints
git add -A && git commit -m "upgrade: rebase patch series onto v24.19.0"
git add -A && git commit -m "upgrade: rebase patch series onto v24.20.0"
```

Reconstruction from a clean clone is what CI checks, so check it here too:
`rm -rf /tmp/verify && scripts/prepare.sh /tmp/verify` must print `OK` on the
hash you just committed.

## New upstream tests

Edit `test/*/*.status` in `out/`, before the regenerate step above.

The full suite is a deny-list, so every test a release adds runs on device
the moment the base moves — and the suite gates the merge. Work out what
needs skipping *before* pushing rather than reading it off a red run:

```sh
git -C out diff --name-status v24.19.0 v24.20.0 -- test/parallel test/sequential | grep '^A'
```

Open the PR. CI re-runs the reconstruction against a fresh upstream clone,
A minor release can add hundreds. Most need nothing. Triage by cause:

- **Spawns a child node process** — cannot pass on Android, where
`process.execPath` is `app_process64`, so the child SIGABRTs (its stderr
comes back as `Error changing dalvik-cache ownership`). Skip. Grepping for
`child_process` is **not** enough, and this is where triage actually goes
wrong: a test can spawn through `common.spawnPromisified` destructured off
`require('../common')`, which never names `child_process`, or through
`node:test`'s `run({ isolation: 'process' })`, which spawns inside the
runner. Grep for `process.execPath`, `spawnPromisified` and `isolation`
too, and read the test titles — `'…forwarded from child processes'` is the
giveaway that greps miss.
- **Runs under `--permission` with no `--allow-fs-write`** at exit — read the
`// Flags:` header, and watch for a test that starts with the permission
and calls `process.permission.drop('fs.write')` (or `'fs'`) partway
through. The verdict write is denied either way. Skip.
- **Self-skipping** — a test that ends at `common.skip()` scores PASS, so it
needs no entry. This is why v24.20.0's ~250 new QUIC tests are absent from
`parallel.status`: the mobile build doesn't pass `--experimental-quic`, so
`hasQuic` is false and each one skips itself.
- **Already covered by a glob** — check before adding. `test-cli-*`,
`test-eslint-*` and `test-child-process-*` are skipped wholesale on both
platforms; `test-debugger-probe-*` is a glob on Android but an itemized
list on iOS, so a new member of that family needs an iOS entry only.
- **Everything else** — leave it to run. The suite is the measurement.

A child-process spawner fails the Android suite outright, and on the iOS
*simulator* it is **flaky rather than passing**: `posix_spawn` is permitted
there, so it often works and sometimes does not — the same test failed the
Android suite, passed one iOS run, then failed the next. Skip these on both
platforms. A green iOS shard is not evidence the test is sound; it cannot
work on a real device either way, which is why `parallel.status` already
carries a block saying this class is "not coverage worth keeping".

`// Flags:` headers *are* honoured on device (the proxy forwards the whole
argv `test.py` hands it), so a new flag-gated feature needs no special
handling.

Put entries in the cause-named section that matches, on both platforms unless
the cause is platform-specific, in the block's existing sort order. Leave the
blocks headed *"measured on a full device sweep"* alone: those record what a
run actually observed, and an inferred entry filed there makes the heading a
lie. Say in the PR that the new entries are inferred and the suite is what
confirms them.

## Open the PR

CI re-runs the reconstruction against a fresh upstream clone,
validates each patch individually, and — once merged — builds the full
matrix. The version bump and release are a separate step (below), so an
upgrade can land and be exercised before anyone decides to ship it.
Expand All @@ -66,9 +187,7 @@ Because the PR moves `upstream-base.txt`, it also runs the **full device
suite** on both platforms and cannot merge until that is green (the curated
gate is an allow-list, so it cannot see tests upstream just added — they
would otherwise surface on the nightly or at release). Expect the PR to take
substantially longer than a normal one, and expect new upstream tests to need
`.status` entries: anything that spawns a child node process cannot pass on
either platform. → [TESTING.md](./TESTING.md)
substantially longer than a normal one. → [TESTING.md](./TESTING.md)

Merging the bump also moves this fork's `upstream-base` branch to the new
tag, via the `upstream-base` job. That branch is load-bearing rather than
Expand Down
2 changes: 1 addition & 1 deletion expected-tree.txt
Original file line number Diff line number Diff line change
Expand Up @@ -14,4 +14,4 @@
# run prepare.sh (or regenerate-patches.py, which prints the hash) and
# update the value below in the same commit. If you forget, CI fails and
# prints the hash it got.
8dad26f6016a4ef21917fceb8d033696b0c9f2b7
fbc10b7d21282ad5b4654f335263bcd40026eb48
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
A36728F02E53580A004DF2FB /* libllhttp.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 33300EAC240437620019E302 /* libllhttp.a */; };
A36728F12E53580A004DF2FB /* libnbytes.a in Frameworks */ = {isa = PBXBuildFile; fileRef = A36728C02E530EF7004DF2FB /* libnbytes.a */; };
A36728F22E53580A004DF2FB /* libncrypto.a in Frameworks */ = {isa = PBXBuildFile; fileRef = A36728C12E530EF7004DF2FB /* libncrypto.a */; };
B1CE0A012E53580A004DF2FB /* libncrypto_engine.a in Frameworks */ = {isa = PBXBuildFile; fileRef = B1CE0A022E530EF7004DF2FB /* libncrypto_engine.a */; };
A36728F32E53580A004DF2FB /* libnghttp2.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 334822141F6AFBD200CF403E /* libnghttp2.a */; };
A36728F42E53580A004DF2FB /* libnode.a in Frameworks */ = {isa = PBXBuildFile; fileRef = 3376C91D1EC3922F0007AD59 /* libnode.a */; };
A36728F42E53580A004E02FB /* libnode_base.a in Frameworks */ = {isa = PBXBuildFile; fileRef = A36728F42E53580A004E12FB /* libnode_base.a */; };
Expand Down Expand Up @@ -77,6 +78,7 @@
A36728BF2E530EF7004DF2FB /* libabseil.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libabseil.a; path = bin/libabseil.a; sourceTree = "<group>"; };
A36728C02E530EF7004DF2FB /* libnbytes.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libnbytes.a; path = bin/libnbytes.a; sourceTree = "<group>"; };
A36728C12E530EF7004DF2FB /* libncrypto.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libncrypto.a; path = bin/libncrypto.a; sourceTree = "<group>"; };
B1CE0A022E530EF7004DF2FB /* libncrypto_engine.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libncrypto_engine.a; path = bin/libncrypto_engine.a; sourceTree = "<group>"; };
A36728C32E530EF7004DF2FB /* libzlib_data_chunk_simd.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libzlib_data_chunk_simd.a; path = bin/libzlib_data_chunk_simd.a; sourceTree = "<group>"; };
A36728C62E5355E5004DF2FB /* Security.framework */ = {isa = PBXFileReference; lastKnownFileType = wrapper.framework; name = Security.framework; path = Platforms/MacOSX.platform/Developer/SDKs/MacOSX15.5.sdk/System/Library/Frameworks/Security.framework; sourceTree = DEVELOPER_DIR; };
A3F2E7582E716AA30001C23B /* libsqlite.a */ = {isa = PBXFileReference; lastKnownFileType = archive.ar; name = libsqlite.a; path = bin/libsqlite.a; sourceTree = "<group>"; };
Expand Down Expand Up @@ -127,6 +129,7 @@
A36728FB2E53580A004DF2FB /* libv8_compiler.a in Frameworks */,
D9A7619D1F7C63A8006AFE86 /* CoreFoundation.framework in Frameworks */,
A36728F22E53580A004DF2FB /* libncrypto.a in Frameworks */,
B1CE0A012E53580A004DF2FB /* libncrypto_engine.a in Frameworks */,
A36729032E53580A004DF2FB /* libzlib.a in Frameworks */,
);
runOnlyForDeploymentPostprocessing = 0;
Expand Down Expand Up @@ -161,6 +164,7 @@
A36728BD2E530EF7004DF2FB /* libhighway.a */,
A36728C02E530EF7004DF2FB /* libnbytes.a */,
A36728C12E530EF7004DF2FB /* libncrypto.a */,
B1CE0A022E530EF7004DF2FB /* libncrypto_engine.a */,
A36728BA2E530EF7004DF2FB /* libsimdjson.a */,
DEADBEEF000000000000D001 /* libicui18n.a */,
DEADBEEF000000000000D003 /* libicuucx.a */,
Expand Down
3 changes: 3 additions & 0 deletions mobile-src/tools/ios_framework_prepare.sh
Original file line number Diff line number Diff line change
Expand Up @@ -66,6 +66,9 @@ declare -a outputs_common=(
"libmerve.a"
"libnbytes.a"
"libncrypto.a"
# v24.20.0 split EnginePointer out of ncrypto into its own target; libnode_base
# references it, so the framework link needs the archive too.
"libncrypto_engine.a"
"libnghttp2.a"
"libnode.a"
# node's own objects; upstream v24.19.0 split them out of libnode into the
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -137,7 +137,7 @@ index 5cea0393f4..dfec53e67e 100644
- os.system("./configure --dest-cpu=" + DEST_CPU + " --dest-os=android --openssl-no-asm --cross-compiling")
+ os.system("./configure --dest-cpu=" + DEST_CPU + " --dest-os=android --openssl-no-asm --with-intl=" + intl + extra_flags + " --cross-compiling --shared")
diff --git a/configure.py b/configure.py
index a9910d038f..09a7e45d57 100755
index 7fd64a9620..40642c1af4 100755
--- a/configure.py
+++ b/configure.py
@@ -1146,6 +1146,11 @@ parser.add_argument('-C',
Expand All @@ -152,7 +152,7 @@ index a9910d038f..09a7e45d57 100755
parser.add_argument('--clang-cl',
action='store',
dest='clang_cl',
@@ -1626,6 +1631,9 @@ def configure_node_cctest_sources(o):
@@ -1655,6 +1660,9 @@ def configure_node_cctest_sources(o):
SearchFiles('test/cctest', 'h')

def configure_node(o):
Expand Down
18 changes: 9 additions & 9 deletions patches/build-mobile-platform-settings-in-common.gypi.patch
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,10 @@ Co-authored-by: João Matos <joao@tritao.eu>
1 file changed, 118 insertions(+), 7 deletions(-)

diff --git a/common.gypi b/common.gypi
index a2cd172f10..1d39345051 100644
index 83e0691d4a..dc33231a0d 100644
--- a/common.gypi
+++ b/common.gypi
@@ -111,7 +111,7 @@
@@ -112,7 +112,7 @@
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/obj.target/tools/v8_gypfiles/libv8_snapshot.a',
}],
Expand All @@ -35,7 +35,7 @@ index a2cd172f10..1d39345051 100644
'obj_dir%': '<(PRODUCT_DIR)/obj.target',
'v8_base': '<(PRODUCT_DIR)/libv8_snapshot.a',
}],
@@ -222,6 +222,23 @@
@@ -223,6 +223,23 @@
}],
],
}],
Expand All @@ -59,7 +59,7 @@ index a2cd172f10..1d39345051 100644
['OS=="solaris"', {
# pull in V8's postmortem metadata
'ldflags': [ '-Wl,-z,allextract' ]
@@ -230,7 +247,7 @@
@@ -231,7 +248,7 @@
# increase performance, number from experimentation
'cflags': [ '-qINLINE=::150:100000' ]
}],
Expand All @@ -68,7 +68,7 @@ index a2cd172f10..1d39345051 100644
# -fno-omit-frame-pointer is necessary for the --perf_basic_prof
# flag to work correctly. perf(1) gets confused about JS stack
# frames otherwise, even with --call-graph dwarf.
@@ -449,7 +466,7 @@
@@ -450,7 +467,7 @@
[ 'target_arch=="arm64"', {
'msvs_configuration_platform': 'arm64',
}],
Expand All @@ -77,7 +77,7 @@ index a2cd172f10..1d39345051 100644
'cflags+': [
'-fno-omit-frame-pointer',
'-fsanitize=address',
@@ -477,7 +494,7 @@
@@ -478,7 +495,7 @@
}],
],
}],
Expand All @@ -86,7 +86,7 @@ index a2cd172f10..1d39345051 100644
'cflags+': [
'-fno-omit-frame-pointer',
'-fsanitize=undefined',
@@ -486,7 +503,7 @@
@@ -487,7 +504,7 @@
'cflags!': [ '-fno-omit-frame-pointer' ],
'ldflags': [ '-fsanitize=undefined' ],
}],
Expand All @@ -95,7 +95,7 @@ index a2cd172f10..1d39345051 100644
'xcode_settings': {
'OTHER_CFLAGS+': [
'-fno-omit-frame-pointer',
@@ -594,11 +611,13 @@
@@ -595,11 +612,13 @@
}],
['_toolset=="host"', {
'conditions': [
Expand All @@ -111,7 +111,7 @@ index a2cd172f10..1d39345051 100644
'cflags': [ '-m64' ],
'ldflags': [ '-m64' ],
}],
@@ -758,6 +777,98 @@
@@ -759,6 +778,98 @@
}],
],
}],
Expand Down
8 changes: 4 additions & 4 deletions patches/build-mobile-targets-in-node.gyp-node.gypi.patch
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ Co-authored-by: Julia Samól <j.samol@papers.ch>
2 files changed, 30 insertions(+), 8 deletions(-)

diff --git a/node.gyp b/node.gyp
index bd66c9b498..06589d8cc3 100644
index c58f4812e2..d8b6eb51f5 100644
--- a/node.gyp
+++ b/node.gyp
@@ -68,6 +68,11 @@
Expand All @@ -36,7 +36,7 @@ index bd66c9b498..06589d8cc3 100644
'<@(node_builtin_shareable_builtins)',
],
'node_sources': [
@@ -922,7 +927,8 @@
@@ -923,7 +928,8 @@
'<@(node_quic_sources)',
],
}],
Expand All @@ -46,7 +46,7 @@ index bd66c9b498..06589d8cc3 100644
'target_arch=="x64" and '
'node_target_type=="executable"', {
'defines': [ 'NODE_ENABLE_LARGE_CODE_PAGES=1' ],
@@ -1399,8 +1405,9 @@
@@ -1405,8 +1411,9 @@
['OS=="solaris"', {
'ldflags': [ '-I<(SHARED_INTERMEDIATE_DIR)' ]
}],
Expand All @@ -58,7 +58,7 @@ index bd66c9b498..06589d8cc3 100644
'type': 'none',
}],
[ 'node_shared=="true"', {
@@ -1566,7 +1573,8 @@
@@ -1627,7 +1634,8 @@
[ 'node_shared_libuv=="false"', {
'dependencies': [ 'deps/uv/uv.gyp:libuv#host' ],
}],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -46,7 +46,7 @@ index e614c39a6b..711428f898 100644
'configurations': {
'Debug': {
diff --git a/tools/v8_gypfiles/v8.gyp b/tools/v8_gypfiles/v8.gyp
index 95194aca28..217a787878 100644
index f90a29d102..b814fee544 100644
--- a/tools/v8_gypfiles/v8.gyp
+++ b/tools/v8_gypfiles/v8.gyp
@@ -868,6 +868,15 @@
Expand Down
Loading
Loading