Skip to content
/ icu Public
forked from unicode-org/icu
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

Rich0003/font1635 update icu to 72 1 #20

Open
wants to merge 274 commits into
base: runtimecore
Choose a base branch
from

Conversation

rshepherd549
Copy link

#issue: https://devtopia.esri.com/runtime/fontana/issues/1635

Update esri/icu/runtimecore to use icu 72.1

aheninger and others added 30 commits December 16, 2021 11:56
Revise uses of UVector in Formatting related code to better handle memory
allocation failures.  This is one of an ongoing series of commits to address
similar problems with UVector usage throughout ICU.

The changes primarily involve switching uses of UVector::addElementX() to the
new adoptElement() or addElement() functions, as appropriate, and using
LocalPointers for tracking memory ownership.
1. vector.contains() uses sequential comparison, O(n).
   As the vector size is great, the performance will be impacted.
   Remove this unnecessary check, vector.contains(), in C++.

2. At Java's CjkBreakEngine, replace "vector.contains()" with "if(pos > previous)" to deal with duplicate breakpoint position.
   This way, C++ and Java implementation will be synchronous.
   Test: ant checkTest -Dtestclass='com.ibm.icu.dev.test.rbbi.RBBITest'
   (RBBTest#TestBreakAllChars() can generate duplicate position for word break. It could pass with this change)
…pattern;

includes new getter/setter API per TC discussion.
gcc 5.5 on Solaris refuses to recognise pow(int, int32_t)
Bumps xercesImpl from 2.12.0 to 2.12.2.

---
updated-dependencies:
- dependency-name: xerces:xercesImpl
  dependency-type: direct:production
...

Signed-off-by: dependabot[bot] <[email protected]>
Remove the functions UVector::addElementX() and UVector::ensureCapacityX() that
were temporarily added to aid in the cleaning up of UVector's out-of-memory
error handling.
1. \u30fc doesn't belong to Hira, Kana nor Han. Add it into CJK dictionary
2. Include fullwidth char into ALPlus
…n of

adding performance tests to ICU CI.
- test/perf/Makefile.in:
    adds strsrchperf to list of subdirs.
    changes target 'all' to compile everything in the standard way.
- test/perf/ustrperf/Makefile.in: changes target executable from stringperf to
  ustrperf (i.e. name of directory) to allow uniform handling with other
  perf tests in GHA CI rules.
- tools/ctestfw/uperf.cpp: changes output to ndjson format for processing with
  GHA Benchmark. Keep the previous output, which gets processed by the Perl
  scripts, when executed in 'verbose' mode. Backward compatibility,
  in case someone still wants to use the Perl scripts for the time being.
  May get cleaned up later.
Also remove a few non-essential output lines that would interfer with
GHA Benchmark.
processing
ICUNotifier::notifyChanged() was using the thread-unsafe double-checked lock
idiom. Replace it with use of the mutex only.
@rshepherd549
Copy link
Author

rshepherd549 commented Mar 8, 2023

https://runtime-rtc.esri.com/view/vTest/job/vtest/job/3rdparty-interface/341/downstreambuildview/

  • winuwp error for deprecated ucnv_safeClone: change to ucnv_clone
  • winuwp error for deprecated ucol_safeClone: change to ucol_clone

@rshepherd549
Copy link
Author

@rshepherd549 rshepherd549 marked this pull request as ready for review March 9, 2023 09:03
@rshepherd549
Copy link
Author

@chri7325 Could you review when you get chance

Copy link

@chri7325 chri7325 left a comment

Choose a reason for hiding this comment

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

Looks good. I would suggest amending the commits to provide a bit more detail for data creation and the deprecated functions. These can probably be mostly copy pasted from the last time @robert-craig-houston created the data but if there are any changes, it helps tremendously to document that step in the history for the next upgrade.

@@ -323,7 +323,7 @@ ucnv_safeClone(const UConverter* cnv, void *stackBuffer, int32_t *pBufferSize, U
U_CAPI UConverter* U_EXPORT2
ucnv_clone(const UConverter* cnv, UErrorCode *status)
{
return ucnv_safeClone(cnv, nullptr, nullptr, status);
return ucnv_clone(cnv, status);
Copy link

Choose a reason for hiding this comment

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

Were these changes needed to compile? I wonder if these need to be upstreamed? Then we can back this commit out and cleanly merge next upgrade.

Copy link
Author

Choose a reason for hiding this comment

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

These changes were needed for winuwp to compile. It seems to be more picky than the others.
When you say push them upstream, do you mean as a PR to the github/icu repository?

Choose a reason for hiding this comment

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

Correct. It has been a while since I upstreamed a change to ICU but I remember the maintainers were very fast and receptive. If we don't upstream this, we'll probably need to make these changes every time we upgrade so removing that would benefit our process and make upgrades more trivial.

@rshepherd549
Copy link
Author

Thanks @chri7325 . For breaking up the commits, do you mean apply them as separate PRs so it's easier to distinguish:

  1. the icu files changes
  2. runtimecore version and data file
  3. code patches

@chri7325
Copy link

For breaking up the commits, do you mean apply them as separate PRs so it's easier to distinguish:

Luckily no, that would be awful haha. I was just thinking that you can amend your current commits with the more context for the change and the commands you used to make them. You can easily amend commit messages with an interactive rebase and force push them here. They help a lot more with 3rdparty libraries since there's no other contacts we have for working on them to find out why a change was made. Here's the example I was talking about 68e8dc2

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet