JDK-8289541 : Update ICU4C to 71.1 #893

HimaBinduMeda · 2022-09-07T18:13:43Z

Updated icu to v71.1.
Verified build and sanity testing on windows,Mac and Linux.
Removed icu directory from Source/WTF, as the functionality is already provided by Source/ThirdParty/icu

Progress

Change must not contain extraneous whitespace
Commit message must refer to an issue
Change must be properly reviewed (2 reviews required, with at least 1 Reviewer, 1 Author)

Issue

JDK-8289541: Update ICU4C to 71.1

Reviewers

Kevin Rushforth (@kevinrushforth - Reviewer)
Johan Vos (@johanvos - Reviewer)

Reviewing

Using git

Checkout this PR locally:
$ git fetch https://git.openjdk.org/jfx pull/893/head:pull/893
$ git checkout pull/893

Update a local copy of the PR:
$ git checkout pull/893
$ git pull https://git.openjdk.org/jfx pull/893/head

Using Skara CLI tools

Checkout this PR locally:
$ git pr checkout 893

View PR using the GUI difftool:
$ git pr show -t 893

Using diff file

Download this PR as a diff file:
https://git.openjdk.org/jfx/pull/893.diff

bridgekeeper · 2022-09-07T18:15:31Z

👋 Welcome back hmeda! A progress list of the required criteria for merging this PR into master will be added to the body of your pull request. There are additional pull request commands available for use with this pull request.

mlbridge · 2022-09-08T15:12:27Z

Webrevs

kevinrushforth · 2022-09-08T15:29:50Z

/reviewers 2

openjdk · 2022-09-08T15:31:02Z

@kevinrushforth
The total number of required reviews for this PR (including the jcheck configuration and the last /reviewers command) is now set to 2 (with at least 1 Reviewer, 1 Author).

kevinrushforth · 2022-09-08T15:34:32Z

One quick comment. I noticed that several files have changed from Unix-style line endings to DOS-style (CRLF) line endings. These are all extensions that aren't in our list of ones to check for whitespace (so jcheck won't catch them), but they should be converted back to Unix-style line endings to minimize diffs and avoid churn. Here is the list:

Source/ThirdParty/icu/source/common/common.rc
Source/ThirdParty/icu/source/common/common.vcxproj
Source/ThirdParty/icu/source/common/common.vcxproj.filters
Source/ThirdParty/icu/source/common/common_uwp.vcxproj
Source/ThirdParty/icu/source/common/rbbicst.pl
Source/ThirdParty/icu/source/common/rbbirpt.txt
Source/ThirdParty/icu/source/common/sources.txt
Source/ThirdParty/icu/source/i18n/i18n.rc
Source/ThirdParty/icu/source/i18n/i18n.vcxproj
Source/ThirdParty/icu/source/i18n/i18n.vcxproj.filters
Source/ThirdParty/icu/source/i18n/i18n_uwp.vcxproj
Source/ThirdParty/icu/source/i18n/regexcst.pl
Source/ThirdParty/icu/source/i18n/regexcst.txt
Source/ThirdParty/icu/source/i18n/sources.txt
Source/ThirdParty/icu/source/stubdata/sources.txt
Source/ThirdParty/icu/source/stubdata/stubdata.vcxproj
Source/ThirdParty/icu/source/stubdata/stubdata.vcxproj.filters
Source/ThirdParty/icu/source/tools/toolutil/sources.txt
Source/ThirdParty/icu/source/tools/toolutil/toolutil.vcxproj

HimaBinduMeda · 2022-09-08T16:54:41Z

One quick comment. I noticed that several files have changed from Unix-style line endings to DOS-style (CRLF) line endings. These are all extensions that aren't in our list of ones to check for whitespace (so jcheck won't catch them), but they should be converted back to Unix-style line endings to minimize diffs and avoid churn. Here is the list:

Source/ThirdParty/icu/source/common/common.rc
Source/ThirdParty/icu/source/common/common.vcxproj
Source/ThirdParty/icu/source/common/common.vcxproj.filters
Source/ThirdParty/icu/source/common/common_uwp.vcxproj
Source/ThirdParty/icu/source/common/rbbicst.pl
Source/ThirdParty/icu/source/common/rbbirpt.txt
Source/ThirdParty/icu/source/common/sources.txt
Source/ThirdParty/icu/source/i18n/i18n.rc
Source/ThirdParty/icu/source/i18n/i18n.vcxproj
Source/ThirdParty/icu/source/i18n/i18n.vcxproj.filters
Source/ThirdParty/icu/source/i18n/i18n_uwp.vcxproj
Source/ThirdParty/icu/source/i18n/regexcst.pl
Source/ThirdParty/icu/source/i18n/regexcst.txt
Source/ThirdParty/icu/source/i18n/sources.txt
Source/ThirdParty/icu/source/stubdata/sources.txt
Source/ThirdParty/icu/source/stubdata/stubdata.vcxproj
Source/ThirdParty/icu/source/stubdata/stubdata.vcxproj.filters
Source/ThirdParty/icu/source/tools/toolutil/sources.txt
Source/ThirdParty/icu/source/tools/toolutil/toolutil.vcxproj

Changed Line Endings to Unix-style

kevinrushforth · 2022-09-08T17:00:56Z

modules/javafx.web/src/main/native/Source/WTF/CMakeLists.txt

@@ -3,7 +3,6 @@ include(WebKitCommon)
 set_property(DIRECTORY . PROPERTY FOLDER "WTF")

 add_subdirectory(wtf)
-


It looks like when you reverted some debugging changes, you removed an extra blank line. Since this file is otherwise now unchanged from master, can you revert the whole file (e.g., by using git checkout master -- FILENAME)?

Reverted the file to be same as master.

kevinrushforth

It looks like there are 4 missing blank lines in the newly added attribution. Otherwise looks good.

kevinrushforth · 2022-09-21T13:17:34Z

modules/javafx.web/src/main/legal/icu_web.md

@@ -508,3 +443,40 @@ publicity pertaining to distribution of the software without specific,
 written prior permission.  M.I.T. makes no representations about the
 suitability of this software for any purpose.  It is provided "as is"
 without express or implied warranty.
+----------------------------------------------------------------------


Add blank line above and below this line.

modules/javafx.web/src/main/legal/icu_web.md

kevinrushforth

Approved.

The icu_web.md file looks good now. All other testing and review was previously done.

kevinrushforth · 2022-09-21T16:36:48Z

@johanvos or @tiainen can you do a test build?

johanvos · 2022-09-21T19:33:55Z

@johanvos or @tiainen can you do a test build?

building + testing

johanvos

Looking good on win/linux/mac

openjdk · 2022-09-22T07:53:34Z

@HimaBinduMeda This change now passes all automated pre-integration checks.

ℹ️ This project also has non-automated pre-integration requirements. Please see the file CONTRIBUTING.md for details.

After integration, the commit message for the final commit will be:

8289541: Update ICU4C to 71.1

Reviewed-by: kcr, jvos

You can use pull request commands such as /summary, /contributor and /issue to adjust it as needed.

At the time when this comment was updated there had been 7 new commits pushed to the master branch:

7c6a54d: 8293839: Documentation memory consistency effects of runLater
a27840e: 8293368: GitHub Workflows security hardening
5e4552d: 8089280: horizontal scrollbar should never become visible in TableView with constrained resize policy
cef583e: 8293214: Add support for Linux/LoongArch64
27f1905: 8279640: ListView with null SelectionModel/FocusModel throws NPE
205b721: 8293615: Create release notes for JavaFX 19
28f8fa9: 8293375: add_definitions USE_SYSTEM_MALLOC when USE_SYSTEM_MALLOC is ON

Please see this link for an up-to-date comparison between the source branch of this pull request and the master branch.
As there are no conflicts, your changes will automatically be rebased on top of these commits when integrating. If you prefer to avoid this automatic rebasing, please check the documentation for the /integrate command for further details.

As you do not have Committer status in this project an existing Committer must agree to sponsor your change. Possible candidates are the reviewers of this PR (@kevinrushforth, @johanvos) but any other Committer may sponsor as well.

➡️ To flag this PR as ready for integration with the above commit message, type /integrate in a new comment. (Afterwards, your sponsor types /sponsor in a new comment to perform the integration).

HimaBinduMeda · 2022-09-22T09:50:11Z

/integrate

openjdk · 2022-09-22T09:52:50Z

@HimaBinduMeda
Your change (at version 88301d5) is now ready to be sponsored by a Committer.

kevinrushforth · 2022-09-22T11:20:34Z

/sponsor

openjdk · 2022-09-22T11:21:56Z

Going to push as commit 82db6cc.
Since your change was applied there have been 7 commits pushed to the master branch:

7c6a54d: 8293839: Documentation memory consistency effects of runLater
a27840e: 8293368: GitHub Workflows security hardening
5e4552d: 8089280: horizontal scrollbar should never become visible in TableView with constrained resize policy
cef583e: 8293214: Add support for Linux/LoongArch64
27f1905: 8279640: ListView with null SelectionModel/FocusModel throws NPE
205b721: 8293615: Create release notes for JavaFX 19
28f8fa9: 8293375: add_definitions USE_SYSTEM_MALLOC when USE_SYSTEM_MALLOC is ON

Your commit was automatically rebased without conflicts.

openjdk · 2022-09-22T11:22:21Z

@kevinrushforth @HimaBinduMeda Pushed as commit 82db6cc.

💡 You may see a message that your pull request was closed with unmerged commits. This can be safely ignored.

Hima Bindu Meda added 3 commits September 7, 2022 11:44

Update icu to v71-1

0b0054f

Update icu version and checksum

0216d7a

Remove white spaces

49e9831

Hima Bindu Meda added 4 commits September 8, 2022 14:27

whitespace corrections

a3cc2b7

Remove tabs

7d40a3e

update file permission

d3d7979

Update file permission for rbbicst.pl

cc800ab

openjdk bot added the rfr Ready for review label Sep 8, 2022

kevinrushforth requested review from kevinrushforth, johanvos and tiainen September 8, 2022 15:30

Convert Line Endings to UNIX and update mainifest.txt

8135a60

kevinrushforth reviewed Sep 8, 2022

View reviewed changes

Hima Bindu Meda added 2 commits September 8, 2022 22:36

Revert CMakelists.txt to be same as master

84ef0a3

Space Corrections

c9678a2

kevinrushforth self-requested a review September 9, 2022 17:38

Update icu_web.md

22c8e53

kevinrushforth reviewed Sep 21, 2022

View reviewed changes

Space corrections

88301d5

kevinrushforth approved these changes Sep 21, 2022

View reviewed changes

johanvos approved these changes Sep 22, 2022

View reviewed changes

openjdk bot added the ready Ready to be integrated label Sep 22, 2022

openjdk bot added the sponsor Ready to sponsor label Sep 22, 2022

openjdk bot added the integrated Pull request has been integrated label Sep 22, 2022

openjdk bot closed this Sep 22, 2022

openjdk bot removed ready Ready to be integrated rfr Ready for review sponsor Ready to sponsor labels Sep 22, 2022

JDK-8289541 : Update ICU4C to 71.1 #893

JDK-8289541 : Update ICU4C to 71.1 #893

HimaBinduMeda commented Sep 7, 2022 •

edited by openjdk bot

bridgekeeper bot commented Sep 7, 2022

mlbridge bot commented Sep 8, 2022 •

edited

kevinrushforth commented Sep 8, 2022

openjdk bot commented Sep 8, 2022

kevinrushforth commented Sep 8, 2022

HimaBinduMeda commented Sep 8, 2022

kevinrushforth Sep 8, 2022

HimaBinduMeda Sep 8, 2022

kevinrushforth left a comment

kevinrushforth Sep 21, 2022

HimaBinduMeda Sep 21, 2022

kevinrushforth left a comment

kevinrushforth commented Sep 21, 2022

johanvos commented Sep 21, 2022

johanvos left a comment

openjdk bot commented Sep 22, 2022 •

edited

HimaBinduMeda commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

kevinrushforth commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

		@@ -3,7 +3,6 @@ include(WebKitCommon)
		set_property(DIRECTORY . PROPERTY FOLDER "WTF")

		add_subdirectory(wtf)

JDK-8289541 : Update ICU4C to 71.1 #893

JDK-8289541 : Update ICU4C to 71.1 #893

Conversation

HimaBinduMeda commented Sep 7, 2022 • edited by openjdk bot

Progress

Issue

Reviewers

Reviewing

bridgekeeper bot commented Sep 7, 2022

mlbridge bot commented Sep 8, 2022 • edited

Webrevs

kevinrushforth commented Sep 8, 2022

openjdk bot commented Sep 8, 2022

kevinrushforth commented Sep 8, 2022

HimaBinduMeda commented Sep 8, 2022

kevinrushforth Sep 8, 2022

Choose a reason for hiding this comment

HimaBinduMeda Sep 8, 2022

Choose a reason for hiding this comment

kevinrushforth left a comment

Choose a reason for hiding this comment

kevinrushforth Sep 21, 2022

Choose a reason for hiding this comment

HimaBinduMeda Sep 21, 2022

Choose a reason for hiding this comment

kevinrushforth left a comment

Choose a reason for hiding this comment

kevinrushforth commented Sep 21, 2022

johanvos commented Sep 21, 2022

johanvos left a comment

Choose a reason for hiding this comment

openjdk bot commented Sep 22, 2022 • edited

HimaBinduMeda commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

kevinrushforth commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

openjdk bot commented Sep 22, 2022

HimaBinduMeda commented Sep 7, 2022 •

edited by openjdk bot

mlbridge bot commented Sep 8, 2022 •

edited

openjdk bot commented Sep 22, 2022 •

edited