build: make full-icu the default for releases #29887

richardlau · 2019-10-08T17:01:49Z

Release builds specifically set the --with-intl= configure option
instead of relying on the default. Explicitly set to full-icu.

Refs: #29522

Checklist

make -j4 test (UNIX), or vcbuild test (Windows) passes
commit message follows commit guidelines

Release builds specifically set the `--with-intl=` configure option instead of relying on the default. Explicitly set to `full-icu`. Refs: nodejs#29522

richardlau · 2019-10-08T17:05:47Z

semver-major because #29522 is.

srl295

the change is good. I wonder whether being explicit is good here, or whether it should just keep the defaults?

At least, perhaps some documentation somewhere about this interlock would be good?

approving with comments.

Makefile

vcbuild.bat

nodejs-github-bot · 2019-10-08T17:09:47Z

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

Trott · 2019-10-09T16:53:12Z

CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2041/ (queued)

mhdawson

LGTM

Trott · 2019-10-09T22:10:12Z

csv-parser is failing but a fix for that (in execa) landed and was released, so I'm wondering if it's still using a cached execa. Anyone know if it's safe for me to rm -rf /home/iojs/build/workspace/citgm-smoker/nodes/ubuntu1604-64/npm_cache or not? @nodejs/build @nodejs/citgm-admins Or maybe there's a way to query the cache easily?

Trott · 2019-10-09T22:10:44Z

(By the way, CITGM results look good, despite the csv-parser stuff that has been going on on master for some time.)

Trott · 2019-10-09T22:12:17Z

(The bluebird failures are also a bit of a mystery to me for now, but nothing new in this PR. Been happening on master.)

richardlau · 2019-10-10T00:59:58Z

Anyone know if it's safe for me to rm -rf /home/iojs/build/workspace/citgm-smoker/nodes/ubuntu1604-64/npm_cache or not?

Should be safe to delete anything in a job's workspace directory as long as there's not a currently running job using it.

Trott · 2019-10-10T03:59:19Z

Should be safe to delete anything in a job's workspace directory as long as there's not a currently running job using it.

Deleted workspaces and re-running CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2044/ Previous results were fine so if that doesn't fix the issue, this is still good to land.

Trott · 2019-10-10T05:05:30Z

Should be safe to delete anything in a job's workspace directory as long as there's not a currently running job using it.

Deleted workspaces and re-running CITGM: https://ci.nodejs.org/view/Node.js-citgm/job/citgm-smoker/2044/ Previous results were fine so if that doesn't fix the issue, this is still good to land.

Ah, it's failing because it's package-lock.json keeps it loading the old version of execa when running tests in the source tree.

Trott · 2019-10-10T18:57:42Z

Ah, it's failing because it's package-lock.json keeps it loading the old version of execa when running tests in the source tree.

(Submitted a PR for that and it got merged. Now need a release to get the fix into CITGM. mafintosh/csv-parser#147 (comment))

In other news, this has waited the necessary 48 hours and can land....so: Landing!

Trott · 2019-10-10T18:58:29Z

Landed in 4885e50

Release builds specifically set the `--with-intl=` configure option instead of relying on the default. Explicitly set to `full-icu`. Refs: nodejs#29522 PR-URL: nodejs#29887 Reviewed-By: James M Snell <jasnell@gmail.com> Reviewed-By: Steven R Loomis <srloomis@us.ibm.com> Reviewed-By: Beth Griggs <Bethany.Griggs@uk.ibm.com> Reviewed-By: Jiawen Geng <technicalcute@gmail.com> Reviewed-By: Rich Trott <rtrott@gmail.com> Reviewed-By: Gus Caplan <me@gus.host> Reviewed-By: Ruben Bridgewater <ruben@bridgewater.de> Reviewed-By: Michael Dawson <michael_dawson@ca.ibm.com> Reviewed-By: Michaël Zasso <targos@protonmail.com> Reviewed-By: Colin Ihrig <cjihrig@gmail.com>

Notable changes: - **assert**: - do not repeat .throws() code (Ruben Bridgewater) [#28263](#28263) - wrap validation function errors (Ruben Bridgewater) [#28263](#28263) - fix generatedMessage property (Ruben Bridgewater) [#28263](#28263) - improve class instance errors (Ruben Bridgewater) [#28263](#28263) - **benchmark**: - use test/common/tmpdir consistently (João Reis) [#28858](#28858) - **build**: - make full-icu the default for releases (Richard Lau) [#29887](#29887) - update minimum Xcode version for macOS (Michael Dawson) [#29622](#29622) - **child_process**: - runtime deprecate \_channel (cjihrig) [#27949](#27949) - simplify spawn argument parsing (cjihrig) [#27854](#27854) - **console**: - display timeEnd with suitable time unit (Xavier Stouder) [#29251](#29251) - **deps**: - patch V8 to 7.8.279.14 (Myles Borins) [#29694](#29694) - patch V8 to 7.8.279.12 (Myles Borins) [#29694](#29694) - patch V8 to 7.8.279.10 (Myles Borins) [#29694](#29694) - update V8's postmortem script (cjihrig) [#29694](#29694) - V8: cherry-pick 716875d (Myles Borins) [#29694](#29694) - update V8 to 7.8.279.9 (Myles Borins) [#29694](#29694) - V8: cherry-pick b33af60 (Michaël Zasso) [#28016](#28016) - update V8 to 7.6.303.28 (Michaël Zasso) [#28016](#28016) - **domain**: - error handler runs outside of its domain (Julien Gilli) [#26211](#26211) - **fs**: - make FSWatcher.start private (Lucas Holmquist) [#29905](#29905) - add runtime deprecate for file stream open() (Robert Nagy) [#29061](#29061) - allow int64 offset in fs.write/writeSync/fd.write (Zach Bjornson) [#26572](#26572) - use IsSafeJsInt instead of IsNumber for ftruncate (Zach Bjornson) [#26572](#26572) - allow int64 offset in fs.read/readSync/fd.read (Zach Bjornson) [#26572](#26572) - close file descriptor of promisified truncate (João Reis) [#28858](#28858) - **http**: - do not emit end after aborted (Robert Nagy) [#27984](#27984) - don't emit 'data' after 'error' (Robert Nagy) [#28711](#28711) - remove legacy parser (Anna Henningsen) [#29589](#29589) - throw if 'host' agent header is not a string value (Giorgos Ntemiris) [#29568](#29568) - replace superfluous connection property with getter/setter (Robert Nagy) [#29015](#29015) - fix test where aborted should not be emitted (Robert Nagy) [#20077](#20077) - remove default 'timeout' listener on upgrade (Luigi Pinca) [#26030](#26030) - **http, http2**: - remove default server timeout (Ali Ijaz Sheikh) [#27558](#27558) - **http2**: - remove security revert flags (Anna Henningsen) [#29141](#29141) - remove callback-based padding (Anna Henningsen) [#29144](#29144) - **lib**: - rename validateInteger to validateSafeInteger (Zach Bjornson) [#26572](#26572) - correct error.errno to always be numeric (Joyee Cheung) [#28140](#28140) - no need to strip BOM or shebang for scripts (Refael Ackermann) [#27375](#27375) - rework logic of stripping BOM+Shebang from commonjs (Gus Caplan) [#27768](#27768) - **module**: - runtime deprecate createRequireFromPath() (cjihrig) [#27951](#27951) - **readline**: - error on falsy values for callback (Sam Roberts) [#28109](#28109) - **repl**: - close file descriptor of history file (João Reis) [#28858](#28858) - **src**: - bring 425 status code name into accordance with RFC 8470 (Sergei Osipov) [#29880](#29880) - update NODE\_MODULE\_VERSION to 79 (Myles Borins) [#29694](#29694) - update NODE\_MODULE\_VERSION to 78 (Michaël Zasso) [#28918](#28918) - add error codes to errors thrown in C++ (Yaniv Friedensohn) [#27700](#27700) - use non-deprecated overload of V8::SetFlagsFromString (Michaël Zasso) [#28016](#28016) - update NODE\_MODULE\_VERSION to 77 (Michaël Zasso) [#28016](#28016) - update NODE\_MODULE\_VERSION to 74 (Refael Ackermann) [#27375](#27375) - make process.env.TZ setter clear tz cache (Ben Noordhuis) [#20026](#20026) - enable V8's WASM trap handlers (Gus Caplan) [#27246](#27246) - **stream**: - throw unhandled error for readable with autoDestroy (Robert Nagy) [#29806](#29806) - always invoke callback before emitting error (Robert Nagy) [#29293](#29293) - invoke callback before emitting error always (Robert Nagy) [#29293](#29293) - do not flush destroyed writable (Robert Nagy) [#29028](#29028) - don't emit finish on error (Robert Nagy) [#28979](#28979) - disallow stream methods on finished stream (Robert Nagy) [#28687](#28687) - do not emit after 'error' (Robert Nagy) [#28708](#28708) - fix destroy() behavior (Robert Nagy) [#29058](#29058) - simplify `.pipe()` and `.unpipe()` in Readable (Weijia Wang) [#28583](#28583) - **tools**: - patch V8 to run on older XCode versions (Ujjwal Sharma) [#29694](#29694) - update V8 gypfiles (Michaël Zasso) [#29694](#29694) - support full-icu by default (Steven R. Loomis) [#29522](#29522) - **util**: validate formatWithOptions inspectOptions (Ruben Bridgewater) [#29824](#29824) PR-URL: #29504

Notable changes: * assert: * If the validation function passed to `assert.throws()` or `assert.rejects()` returns a value other than `true`, an assertion error will be thrown instead of the original error to highlight the programming mistake (Ruben Bridgewater). #28263 * If a constructor function is passed to validate the instance of errors thrown in `assert.throws()` or `assert.reject()`, an assertion error will be thrown instead of the original error (Ruben Bridgewater). #28263 * build: * Node.js releases are now built with default full-icu support. This means that all locales supported by ICU are now included and Intl-related APIs may return different values than before (Richard Lau). #29887 * The minimum Xcode version supported for macOS was increased to 10. It is still possible to build Node.js with Xcode 8 but this may no longer be the case in a future v13.x release (Michael Dawson). #29622 * child_process: * `ChildProcess._channel` (DEP0129) is now a Runtime deprecation (cjihrig). #27949 * console: * The output `console.timeEnd()` and `console.timeLog()` will now automatically select a suitable time unit instead of always using milliseconds (Xavier Stouder). #29251 * deps: * The V8 engine was updated to version 7.8. This includes performance improvements to object destructuring, memory usage and WebAssembly startup time (Myles Borins). #29694) * domain: * The domain's error handler is now executed with the active domain set to the domain's parent to prevent inner recursion (Julien Gilli). #26211 * fs: * The undocumented method `FSWatcher.prototype.start()` was removed (Lucas Holmquist). #29905 * Calling the `open()` method on a `ReadStream` or `WriteStream` now emits a runtime deprecation warning. The methods are supposed to be internal and should not be called by user code (Robert Nagy). #29061 * `fs.read/write`, `fs.readSync/writeSync` and `fd.read/write` now accept any safe integer as their `offset` parameter. The value of `offset` is also no longer coerced, so a valid type must be passed to the functions (Zach Bjornson). #26572 * http: * Aborted requests no longer emit the `end` or `error` events after `aborted` (Robert Nagy). #27984 #20077 * Data will no longer be emitted after a socket error (Robert Nagy). #28711 * The legacy HTTP parser (previously available under the `--http-parser=legacy` flag) was removed (Anna Henningsen). #29589 * The `host` option for HTTP requests is now validated to be a string value (Giorgos Ntemiris). #29568 * The `request.connection` and `response.connection` properties are now runtime deprecated. The equivalent `request.socket` and `response.socket` should be used instead (Robert Nagy). #29015 * http, http2: * The default server timeout was removed (Ali Ijaz Sheikh). #27558 * Brought 425 status code name into accordance with RFC 8470. The name changed from "Unordered Collection" to "Too Early" (Sergei Osipov). #29880 * lib: * The `error.errno` property will now always be a number. To get the string value, use `error.code` instead (Joyee Cheung). #28140 * module: * `module.createRequireFromPath()` is deprecated. Use `module.createRequire()` instead (cjihrig). #27951 * src: * Changing the value of `process.env.TZ` will now clear the tz cache. This affects the default time zone used by methods such as `Date.prototype.toString` (Ben Noordhuis). #20026 * stream: * The timing and behavior of streams was consolidated for a number of edge cases. Please look at the individual commits below for more information. PR-URL: #29504

build: make full-icu the default for releases

fb621e0

Release builds specifically set the `--with-intl=` configure option instead of relying on the default. Explicitly set to `full-icu`. Refs: nodejs#29522

nodejs-github-bot added build Issues and PRs related to build files or the CI. windows Issues and PRs related to the Windows platform. labels Oct 8, 2019

richardlau mentioned this pull request Oct 8, 2019

Make full-icu the default #29522

Closed

4 tasks

richardlau added the semver-major PRs that contain breaking changes and should be released in the next major version. label Oct 8, 2019

richardlau added i18n-api Issues and PRs related to the i18n implementation. and removed windows Issues and PRs related to the Windows platform. labels Oct 8, 2019

jasnell approved these changes Oct 8, 2019

View reviewed changes

srl295 approved these changes Oct 8, 2019

View reviewed changes

Makefile Show resolved Hide resolved

vcbuild.bat Show resolved Hide resolved

vcbuild.bat Show resolved Hide resolved

richardlau added the author ready PRs that have at least one approval, no pending requests for changes, and a CI started. label Oct 8, 2019

BethGriggs approved these changes Oct 8, 2019

View reviewed changes

gengjiawen approved these changes Oct 9, 2019

View reviewed changes

richardlau mentioned this pull request Oct 9, 2019

2019-10-22, Version 13.0.0 (Current) #29504

Merged

Trott approved these changes Oct 9, 2019

View reviewed changes

devsnek approved these changes Oct 9, 2019

View reviewed changes

BridgeAR approved these changes Oct 9, 2019

View reviewed changes

mhdawson approved these changes Oct 9, 2019

View reviewed changes

targos approved these changes Oct 9, 2019

View reviewed changes

cjihrig approved these changes Oct 9, 2019

View reviewed changes

Trott added this to the 13.0.0 milestone Oct 10, 2019

Trott closed this Oct 10, 2019

DRoet mentioned this pull request Nov 25, 2019

13.1.0-alpine image size grew by 31% compared to 12.13.0-alpine nodejs/docker-node#1154

Closed

ehmicky mentioned this pull request Nov 25, 2019

Add full-icu internationalization support in Node builds netlify/build-image#224

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

build: make full-icu the default for releases #29887

build: make full-icu the default for releases #29887

richardlau commented Oct 8, 2019

richardlau commented Oct 8, 2019

srl295 left a comment

nodejs-github-bot commented Oct 8, 2019

Trott commented Oct 9, 2019

mhdawson left a comment

Trott commented Oct 9, 2019

Trott commented Oct 9, 2019

Trott commented Oct 9, 2019

richardlau commented Oct 10, 2019

Trott commented Oct 10, 2019 •

edited

Loading

Trott commented Oct 10, 2019

Trott commented Oct 10, 2019

Trott commented Oct 10, 2019

build: make full-icu the default for releases #29887

build: make full-icu the default for releases #29887

Conversation

richardlau commented Oct 8, 2019

Checklist

richardlau commented Oct 8, 2019

srl295 left a comment

Choose a reason for hiding this comment

nodejs-github-bot commented Oct 8, 2019

Trott commented Oct 9, 2019

mhdawson left a comment

Choose a reason for hiding this comment

Trott commented Oct 9, 2019

Trott commented Oct 9, 2019

Trott commented Oct 9, 2019

richardlau commented Oct 10, 2019

Trott commented Oct 10, 2019 • edited Loading

Trott commented Oct 10, 2019

Trott commented Oct 10, 2019

Trott commented Oct 10, 2019

Trott commented Oct 10, 2019 •

edited

Loading