Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[v14.x] deps: restore minimum ICU version to 65 #39068

Closed
wants to merge 1 commit into from

Conversation

richardlau
Copy link
Member

@richardlau richardlau commented Jun 17, 2021

This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on v14.x-staging and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

Refs: #38178 (comment)

I've tested this in a Fedora 32 container with

  • ./configure --with-intl=system-icu (system ICU 65)
  • ./configure (ICU 69 in the Node.js source tree)

make test passes with both configurations.

A wider discussion regarding ICU upgrade policy within a release line is happening in nodejs/Release#679.

cc @nodejs/releasers @AdamMajer

@github-actions github-actions bot added i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v14.x v8 engine Issues and PRs related to the V8 dependency. labels Jun 17, 2021
@nodejs-github-bot
Copy link
Collaborator

@addaleax addaleax changed the title deps: restore minimum ICU version to 65 [v14.x] deps: restore minimum ICU version to 65 Jun 17, 2021
Copy link
Member

@mhdawson mhdawson left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rubber Stamp LGTM

@richardlau richardlau force-pushed the v14.x-staging branch 2 times, most recently from 1f06fcf to 16dcd9c Compare July 5, 2021 16:02
bmwiedemann pushed a commit to bmwiedemann/openSUSE that referenced this pull request Jul 6, 2021
https://build.opensuse.org/request/show/904343
by user adamm + dimstar_suse
- update to 14.17.2:
  deps: libuv upgrade - Out of bounds read (Medium)
  (bsc#1187973, CVE-2021-22918)
- old_icu.patch: update with upstream's patch from
  nodejs/node#39068
- specfile cleanup
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede
@nodejs-github-bot
Copy link
Collaborator

nodejs-github-bot commented Jul 15, 2021

richardlau added a commit that referenced this pull request Jul 15, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@richardlau
Copy link
Member Author

Landed in 3543ed2.

@richardlau richardlau closed this Jul 15, 2021
jameshilliard pushed a commit to jameshilliard/node that referenced this pull request Jul 19, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 20, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@richardlau richardlau mentioned this pull request Jul 20, 2021
richardlau added a commit to jameshilliard/node that referenced this pull request Jul 23, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 23, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
@richardlau richardlau mentioned this pull request Jul 23, 2021
guybedford pushed a commit that referenced this pull request Jul 26, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
richardlau added a commit that referenced this pull request Jul 26, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: #39068
Backport-PR-URL: #39451
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
foxxyz pushed a commit to foxxyz/node that referenced this pull request Oct 18, 2021
This modifies 40df0dc so that the changes it applies are only used
if ICU 67 or greater is used, and restores the previous code path for
versions of ICU below 67.

The minimum ICU version was bumped to 67 in Node.js 14.6.0 by
nodejs#34356 but the referenced V8
commit[1] isn't on `v14.x-staging` and appears to have been reverted
on V8 8.4[2] so this PR also restores the minimum ICU version to 65.

[1] v8/v8@611e412
[2] v8/v8@eeccede

PR-URL: nodejs#39068
Reviewed-By: Michael Dawson <midawson@redhat.com>
Reviewed-By: Colin Ihrig <cjihrig@gmail.com>
Reviewed-By: Beth Griggs <bgriggs@redhat.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
i18n-api Issues and PRs related to the i18n implementation. icu Issues and PRs related to the ICU dependency. needs-ci PRs that need a full CI run. tools Issues and PRs related to the tools directory. v8 engine Issues and PRs related to the V8 dependency.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants