there can be only one UnicodeProperty.java #66

markusicu · 2021-05-29T17:45:25Z

Background: ucd-dev email thread “Unicode Tools vs. UnicodeProperty.java”

We have two versions of org/unicode/cldr/util/props/UnicodeProperty.java, one in CLDR and one in the Unicode Tools.
Until 1.5 years ago they were identical, but they have been diverging and are now no longer even interface-compatible. The CLDR version is the newer one.

For now, I am adding a note to the setup instructions to move cldr-code above unicodetools in the build path, but of course that's evil and brittle.

We had agreed to move UnicodeProperty from CLDR into the Unicode Tools, but other CLDR code depends on this class, and I am not sure if I have time to get through that.
Maybe I just rename the unused Unicode Tools version at first.

There is also a third version of UnicodeProperty.java, in the UnicodeJsps code, but thankfully it is in a different package. Still, this is confusing and error-prone; we should either merge the classes or give them distinct names.

srl295 · 2021-10-01T20:37:17Z

See https://unicode-org.atlassian.net/browse/CLDR-9787 for the CLDR side of duplicate classes between ICU/CLDR (similar problem), and https://unicode-org.atlassian.net/browse/ICU-21757 for what may be a needed part of the solution
~~Once move main tools to maven #64 is done, we should change UnicodeJsps to be dependent on unicodetools. This will allow the classes to be present only once and shared between those two.~~ (filed this as make UnicodeJsps depend on unicodetools #131 )

macchiati · 2021-10-01T21:51:54Z

I tried at one point to pull UnicodeProperty out from CLDR, but it is not simple. There are classes that depend on classes and so on, soit was too hard to figure out what the tangled set of dependencies were, and where I could cut the cord to allow me to extract a set of classes and tie off the loose ends. Mark

…

On Sat, May 29, 2021 at 10:45 AM Markus Scherer ***@***.***> wrote: Background: ucd-dev email thread “Unicode Tools vs. UnicodeProperty.java <https://groups.google.com/g/ucd-dev/c/H60xj1kTfCU>” We have two versions of org/unicode/cldr/util/props/UnicodeProperty.java, one in CLDR and one in the Unicode Tools. Until 1.5 years ago they were identical, but they have been diverging and are now no longer even interface-compatible. The CLDR version is the newer one. For now, I am adding a note to the setup instructions to move cldr-code above unicodetools in the build path, but of course that's evil and brittle. We had agreed to move UnicodeProperty from CLDR into the Unicode Tools, but other CLDR code depends on this class, and I am not sure if I have time to get through that. Maybe I just rename the unused Unicode Tools version at first. There is also a third version of UnicodeProperty.java, in the UnicodeJsps code, but thankfully it is in a different package. Still, this is confusing and error-prone; we should either merge the classes or give them distinct names. - https://github.com/unicode-org/cldr/blob/master/tools/cldr-code/src/main/java/org/unicode/cldr/util/props/UnicodeProperty.java - https://github.com/unicode-org/unicodetools/blob/main/unicodetools/org/unicode/cldr/util/props/UnicodeProperty.java - https://github.com/unicode-org/unicodetools/blob/main/UnicodeJsps/src/main/java/org/unicode/jsp/UnicodeProperty.java — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub <#66>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMDLXLVTZAO7LDBIWP3TQER4DANCNFSM45YP3R3A> .

srl295 · 2021-10-04T20:17:28Z

Josh's analysis: https://docs.google.com/spreadsheets/d/1sAIFut5QL30EXwMK_ZM4yKzWJge2gatLA0yfl_zxtSY/edit?usp=sharing

josh-hadley · 2021-10-08T23:12:41Z

As @srl295 posted, I have performed a basic analysis of duplicated .java files as a starting point. I attempted a very basic diff to try to triage a bit but I think that was not terribly useful with the multi-file overlaps. Sadly, looks like only one duplicated file is identical.

So I think the next step will be to look more closely into the overlaps and determine commonalities/differences inside and figure out what can be re-worked, removed, etc.

srl295 · 2021-11-09T20:45:40Z

Now that #148 (#131) is merged, the JSPs can easily call into unicodetools librarycode.

srl295 · 2022-03-23T18:54:49Z

this bit me trying to do #186 (IDNA)— because of the order of loading, ICU4J's UnicodeSet ends up calling into unicodetools' UnicodeProperty and crashes.

macchiati · 2022-03-23T19:47:23Z

Hmmm. We could rename unicodetools' version

…

On Wed, Mar 23, 2022 at 11:55 AM Steven R. Loomis ***@***.***> wrote: this bit me trying to do #186 <#186> (IDNA)— because of the order of loading, ICU4J's UnicodeSet ends up calling into unicodetools' UnicodeProperty and crashes. — Reply to this email directly, view it on GitHub <#66 (comment)>, or unsubscribe <https://github.com/notifications/unsubscribe-auth/ACJLEMBQ54LIJJAYF35ETIDVBNSIJANCNFSM45YP3R3A> . You are receiving this because you commented.Message ID: ***@***.***>

srl295 · 2022-03-23T19:50:43Z

@macchiati I'm going to try moving unicodetools' version into org.unicode.props …

srl295 · 2022-03-23T20:01:05Z

Back burner on this due to CLDR release, but just as an example of the layering problems:

unicodetools/unicodetools/src/main/java/org/unicode/tools/TestSegments.java

Lines 151 to 155 in 8d6bd2e

    
           private static void doCompare(UnicodeProperty.Factory factory, Segmenter rl, String line) { 
        
               RandomStringGenerator rsg; 
        
               RuleBasedBreakIterator icuBreak; 
        
               if (line.equals("compareGrapheme")) { 
        
                   rsg = new RandomStringGenerator(factory, "GraphemeClusterBreak");

    private static void doCompare(UnicodeProperty.Factory factory, Segmenter rl, String line) {
        RandomStringGenerator rsg;
        RuleBasedBreakIterator icuBreak;
        if (line.equals("compareGrapheme")) {
            rsg = new RandomStringGenerator(factory, "GraphemeClusterBreak");

So here we are calling into CLDR (RandomStringGenerator) but passing it unicodetools' version of UnicodeProperty.Factory. The only way this “works” is basically that UnicodeProperty.class from CLDR gets replaced at runtime with unicodetools' … not good

markusicu · 2022-03-23T20:29:24Z

It looks like CLDR RandomStringGenerator has other problems, too. It sounds generic, but its init() function is specific to working with the GCB property. And when you call the (Factory, String, bool, bool) constructor, you get some data from the input factory and other data from the ICUPropertyFactory.

If this is used from CLDR code, then I would clone it into Unicode Tools (with a different package and modified class name). If it's not used from CLDR code, then move it into Unicode Tools.

It might also be possible to pull all of the factory calls out into the call site and construct this generator only with UnicodeSet/UnicodeMap/String/boolean values.

- also move some code out of UnicodeJsps into org.unicode.jsp Relates to #66

…ackage - also move some code out of UnicodeJsps into org.unicode.jsp Relates to #66

- deprecate org.unicode.jsp.UnicodeProperty Relates to #66

srl295 · 2022-03-24T14:54:27Z

Observations (not complaints):

it's very clear that our development tools / environment / process has had quite an impact on how things are structured. Copying source files was easy, dependencies and reuse between projects hard/impossible, even between the unicodetools and the JSPs
I think CLDR should be split up into modules. Maybe it should depend on unicodetools and not the other way around!

markusicu · 2022-03-24T21:06:58Z

I verified that the RandomStringGenerator is not used in CLDR. Please move it out of there and into the Unicode Tools.
https://unicode-org.atlassian.net/browse/CLDR-15482

srl295 · 2022-03-24T21:07:47Z

@markusicu my #214 does copy it into unicodetools paving the way for deletion on cldr.

srl295 · 2022-03-25T18:00:03Z

It's "now" illegal (as of JDK 9?) to have more than one module export the same package. So, CLDR and ICU aren't supposed to both export to com.ibm.icu.text.

For: #66

markusicu · 2022-04-07T02:53:54Z

Something I learned (and others might have realized some time ago):

The Unicode Tools use the CLDR UnicodePropertySymbolTable which calls into UnicodeProperty. That must be a CLDR UnicodeProperty, which is what the symbol table code calls from within the CLDR jar. The Tools provide the symbol table with a ToolUnicodeProperty (which lives in the Tools) which extends UnicodeProperty. Bad things happen when CLDR code uses the Unicode Tools version of UnicodeProperty.

Unicode Tools code needs the Unicode Tools version of UnicodeProperty.

David changed CLDR UnicodeProperty to base its Matcher classes on a Java Predicate:
https://unicode-org.atlassian.net/browse/CLDR-13750
unicode-org/cldr#460

The Unicode Tools version is still basing the Matchers on an older ObjectMatcher interface.

markusicu · 2022-04-07T16:52:02Z

FYI
In pull request #214 @srl295 copied most of the classes from https://github.com/unicode-org/cldr/tree/main/tools/cldr-code/src/main/java/org/unicode/cldr/util/props into the unicodetools.
It looks like only abstract class UnicodeLabel (the base for all of the UnicodeProperty classes) still exists only in CLDR.

markusicu · 2022-04-08T03:47:27Z

meld ~/cldr/uni/src/tools/cldr-code/src/main/java/org/unicode/cldr/util/props/UnicodeProperty.java unicodetools/src/main/java/org/unicode/props/UnicodeProperty.java

markusicu · 2024-01-20T01:04:24Z

@eggrobin just deleted the UnicodeJSPs version in PR #649.

srl295 mentioned this issue Oct 1, 2021

Mavenize UnicodeTools (#64) #91

Merged

srl295 mentioned this issue Oct 1, 2021

make UnicodeJsps depend on unicodetools #131

Closed

srl295 added a commit that referenced this issue Mar 24, 2022

move UnicodeProperty and its ilk to not conflict with CLDR package

ef7e474

- also move some code out of UnicodeJsps into org.unicode.jsp Relates to #66

srl295 added a commit that referenced this issue Mar 24, 2022

^ fixup: move UnicodeProperty and its ilk to not conflict with CLDR p…

2659373

…ackage - also move some code out of UnicodeJsps into org.unicode.jsp Relates to #66

srl295 added a commit that referenced this issue Mar 24, 2022

next round of fixup in jsps

34d221b

- deprecate org.unicode.jsp.UnicodeProperty Relates to #66

srl295 added a commit that referenced this issue Mar 24, 2022

next round of fixup in jsps

a6e84ea

- deprecate org.unicode.jsp.UnicodeProperty Relates to #66

srl295 mentioned this issue Mar 24, 2022

move UnicodeProperty so it doesn't shadow CLDRs #214

Merged

srl295 added a commit that referenced this issue Apr 5, 2022

delete UnicodeProperty.java

fcae6d8

For: #66

srl295 mentioned this issue Apr 5, 2022

delete UnicodeProperty.java #216

Closed

markusicu mentioned this issue Apr 8, 2022

switch from ICU ObjectMatcher to Java Predicate #219

Merged

markusicu mentioned this issue Apr 27, 2022

port UnicodeProperty improvements from CLDR #228

Open

This was referenced Dec 14, 2022

move code here from CLDR that is not needed there #369

Open

clean up duplicate classes within & across Unicode Tools, ICU, CLDR #372

Open

eggrobin mentioned this issue Jan 20, 2024

There can only be one MySymbolTable #667

Merged

This was referenced Mar 22, 2025

Segmentation test cases are being generated based on data for the previous version of Unicode #1072

Closed

There can only be one XSymbolTable #1073

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

there can be only one UnicodeProperty.java #66

there can be only one UnicodeProperty.java #66

markusicu commented May 29, 2021 •

edited by eggrobin

Loading

srl295 commented Oct 1, 2021 •

edited

Loading

macchiati commented Oct 1, 2021 via email

srl295 commented Oct 4, 2021

josh-hadley commented Oct 8, 2021

srl295 commented Nov 9, 2021 •

edited

Loading

srl295 commented Mar 23, 2022

macchiati commented Mar 23, 2022 via email

srl295 commented Mar 23, 2022

srl295 commented Mar 23, 2022

markusicu commented Mar 23, 2022

srl295 commented Mar 24, 2022

markusicu commented Mar 24, 2022

srl295 commented Mar 24, 2022

srl295 commented Mar 25, 2022

markusicu commented Apr 7, 2022

markusicu commented Apr 7, 2022

markusicu commented Apr 8, 2022

markusicu commented Jan 20, 2024

there can be only one UnicodeProperty.java #66

there can be only one UnicodeProperty.java #66

Comments

markusicu commented May 29, 2021 • edited by eggrobin Loading

srl295 commented Oct 1, 2021 • edited Loading

macchiati commented Oct 1, 2021 via email

srl295 commented Oct 4, 2021

josh-hadley commented Oct 8, 2021

srl295 commented Nov 9, 2021 • edited Loading

srl295 commented Mar 23, 2022

macchiati commented Mar 23, 2022 via email

srl295 commented Mar 23, 2022

srl295 commented Mar 23, 2022

markusicu commented Mar 23, 2022

srl295 commented Mar 24, 2022

markusicu commented Mar 24, 2022

srl295 commented Mar 24, 2022

srl295 commented Mar 25, 2022

markusicu commented Apr 7, 2022

markusicu commented Apr 7, 2022

markusicu commented Apr 8, 2022

markusicu commented Jan 20, 2024

markusicu commented May 29, 2021 •

edited by eggrobin

Loading

srl295 commented Oct 1, 2021 •

edited

Loading

srl295 commented Nov 9, 2021 •

edited

Loading