Skip to content

[v12.x] Fix build with ICU >= 68. - #39440

Closed
jameshilliard wants to merge 1 commit into
nodejs:v12.x-stagingfrom
jameshilliard:fix-icu-68
Closed

[v12.x] Fix build with ICU >= 68.#39440
jameshilliard wants to merge 1 commit into
nodejs:v12.x-stagingfrom
jameshilliard:fix-icu-68

Conversation

@jameshilliard

@jameshilliardjameshilliard commented Jul 19, 2021

Copy link
Copy Markdown

@github-actionsgithub-actionsBot added needs-ci PRs that need a full CI run. v12.x v8 engine Issues and PRs related to the V8 dependency. labels Jul 19, 2021
@richardlaurichardlau changed the title Fix build with ICU >= 68.[v12.x] Fix build with ICU >= 68.Jul 19, 2021
@aduh95

aduh95 commented Jul 19, 2021

Copy link
Copy Markdown
Contributor

Is it the same as v8/v8@035c305? If so, can you apply the upstream commit instead (git node v8 backport 035c305ce7761f51328b45f1bd83e26aef267c9d). See https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md#backporting-to-abandoned-branches for detailed instructions.

@richardlau

richardlau commented Jul 19, 2021

Copy link
Copy Markdown
Member

Is it the same as v8/v8@035c305? If so, can you apply the upstream commit instead (git node v8 backport 035c305ce7761f51328b45f1bd83e26aef267c9d). See https://github.com/nodejs/node/blob/master/doc/guides/maintaining-V8.md#backporting-to-abandoned-branches for detailed instructions.

For history, we had to patch up v8/v8@035c305 for 14.x so that LTS Node.js could continue to build against the existing ICU versions #39068.

@jameshilliard

jameshilliard commented Jul 19, 2021

Copy link
Copy Markdown
Author

Is it the same as v8/v8@035c305?

I think so, but I tried to maintain backwards compat with older ICU versions.

@jameshilliard

jameshilliard commented Jul 19, 2021

Copy link
Copy Markdown
Author

For history, we had to patch up v8/v8@035c305 for 14.x so that LTS Node.js could continue to build against the existing ICU versions #39068.

Made a version based on that in #39451 in case that's preferable.

@jameshilliard@richardlau
Fix build with ICU >= 68.
34a9207
Adapted from:
https://chromium-review.googlesource.com/c/v8/v8/+/2477751
Signed-off-by: James Hilliard <james.hilliard1@gmail.com>
@nodejs-github-bot

nodejs-github-bot commented Jul 23, 2021

Copy link
Copy Markdown
Collaborator

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

@richardlau

richardlau commented Jul 23, 2021

Copy link
Copy Markdown
Member

I've landed #39451 instead. Thanks.

@jameshilliard
jameshilliard deleted the fix-icu-68 branch July 23, 2021 15:20
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

needs-ciPRs that need a full CI run.v8 engineIssues and PRs related to the V8 dependency.

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@jameshilliard@aduh95@richardlau@nodejs-github-bot