deps: V8: hot-patch fix for Temporal cherry-pick cd89fe6f7acb

[srl295]

Fixes: #62676

Original commit message:

build: Fix Temporal compilation with system or no ICU

- remove reference to internal udatamem.h header file

See: https://unicode-org.atlassian.net/browse/ICU-23400
See: Node: https://github.com/nodejs/node/issues/62676
See: V8: https://chromium-review.googlesource.com/c/v8/v8/+/8281487

Bug: 8281487

Refs: srl295/v8@cd89fe6

See: https://unicode-org.atlassian.net/browse/ICU-23400
See: Node: #62676
See: V8: https://chromium-review.googlesource.com/c/v8/v8/+/8281487

[nodejs-github-bot]

Review requested:

• @nodejs/gyp
• @nodejs/security-wg
• @nodejs/v8-update

[nodejs-github-bot]

CI: https://ci.nodejs.org/job/node-test-pull-request/77384/

[legendecas]

This check can be removed (in a new commit):

node/configure.py

Lines 2264 to 2266 in 6a7f94c

	case 'system-icu':
	warn('Temporal support disabled when compiling with a shared ICU library')
	options.v8_disable_temporal_support = True

[srl295]

This check can be removed (in a new commit):

node/configure.py

Lines 2264 to 2266 in 6a7f94c

	case 'system-icu':
	warn('Temporal support disabled when compiling with a shared ICU library')
	options.v8_disable_temporal_support = True

Good catch. Thanks.

I will be setting a new v8 option there anyway, so makes sense. Hope to have a new patch tomorrow.

[legendecas]

Thank you, I verified this locally and node works with a brew provided icu4c shared library.

[aduh95]

ICU	Builds	test-temporal-with-zoneinfo
No ICU (--without-intl)	✅1	✅
Shared (--with=intl=system-icu)	✅	✅
Small ICU (--with-intl=small-icu)	✅	❌2
Full ICU (default)	✅	✅

Footnotes

1. requires some manual edits

   As described in #62676 (comment), the make_temporal_zoneinfo_cpp GN rule has not been integrated in our GYP files:

   python deps/v8/tools/include-file-as-bytes.py deps/crates/vendor/zoneinfo64-v0_3/src/data/zoneinfo64.res deps/v8/src/objects/builtins-temporal-zoneinfo64-data.cc zoneinfo64_static_data
   echo '#include "builtins-temporal-zoneinfo64-data.cc"' >> deps/v8/src/objects/js-temporal-zoneinfo64.cc
   rm -r out && nix-shell --pure --arg icu null --arg withTemporal true --arg devTools '[]' --arg benchmarkTools '[]' --run 'make build-ci -j16' && ./node -e 'assert.strictEqual(Temporal.Instant.from("1969-07-20T20:17:00Z").toString(), "1969-07-20T20:17:00Z")'
   

   ↩
2. Temporal error: Internal error: Failed to load timezone info.

   $ rm -r out && nix-shell --pure --argstr icu small --arg withTemporal true --arg devTools '[]' --arg benchmarkTools '[]' --run 'make build-ci -j16' && ./node -e 'assert.strictEqual(Temporal.Instant.from("1969-07-20T20:17:00Z").toString(), "1969-07-20T20:17:00Z")'
   
   assert.strictEqual(Temporal.Instant.from("1969-07-20T20:17:00Z").toString(), "1969-07-20T20:17:00Z")
                                                                    ^
   
   Error: Temporal error: Internal error: Failed to load timezone info.
       at Instant.toString (<anonymous>)
       at [eval]:1:66
       at runScriptInThisContext (node:internal/vm:219:10)
       at node:internal/process/execution:485:12
       at [eval]-wrapper:6:24
       at runScriptInContext (node:internal/process/execution:483:60)
       at evalFunction (node:internal/process/execution:317:30)
       at evalTypeScript (node:internal/process/execution:329:3)
       at node:internal/main/eval_string:71:3
   
   Node.js v27.0.0-pre
   

   ↩

[srl295]

@aduh95 @legendecas somehow the v8 configuration isn't getting propagated… can you look at my 2nd commit and see what i'm doing wrong?

[legendecas]

node build does not use GN (it's used by electron). I think you have added the flag declaration in the features.gypi, could you also add a definition like the following definition as well?

node/tools/v8_gypfiles/features.gypi

Lines 426 to 428 in dd5dfb5

	['v8_enable_temporal_support==1', {
	'defines': ['V8_TEMPORAL_SUPPORT',],
	}],

like:

      ['v8_enable_temporal_systemicu==1', {
        'defines': ['V8_ENABLE_TEMPORAL_SYSTEMICU',],
      }],

[srl295]

node build does not use GN (it's used by electron). I think you have added the flag declaration in the features.gypi, could you also add a definition like the following definition as well?

node/tools/v8_gypfiles/features.gypi

Lines 426 to 428 in dd5dfb5

	['v8_enable_temporal_support==1', {
	'defines': ['V8_TEMPORAL_SUPPORT',],
	}],

like:

      ['v8_enable_temporal_systemicu==1', {
        'defines': ['V8_ENABLE_TEMPORAL_SYSTEMICU',],
      }],

Thanks, i will try that.

I assumed that the settings within v8 would automatically do that. In a sense we don't need v8_enable_temporal_systemicu but just set V8_ENABLE_TEMPORAL_SYSTEMICU but it's probably good to allow this fix to be reachable by other consumers of v8 (and maybe v8 itself eventually)

[srl295]

OK, this worked for me also!

[legendecas]

LGTM, please feel free to mark the PR as ready for review if you are ready.

[legendecas]

Though, we should wait https://chromium-review.googlesource.com/c/v8/v8/+/8281487 to land first.

[srl295]

Though, we should wait https://chromium-review.googlesource.com/c/v8/v8/+/8281487 to land first.

i'll mark it ready for review.