ICU-22917 Generating / updating the units data is a very clunky and manual process

[mihnita]

Before these changes the measurement unit code updates were generated from a unit test
to standard output, and had to be enabled / disabled one by one.

Although not 100% automated, after this change the tests are enabled by a flag,
and the result go to some temporary files that can be opened in editor, diff-ed, etc.

Something like an 80% improvement with a 20% effort.

Doing more becomes tricky.
In some cases the old code should be changed, or not.
In others new methods get added to files, if they don't already exist.
Or replace the old method if different.
And that would require parsing Java and C++ files to determine what is there, and how much to delete.

Checklist

• Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-22917
• Required: The PR title must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
• Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
• Required: Each commit message must be prefixed with a JIRA Issue number. Example: "ICU-1234 Fix xyz"
• Issue accepted (done by Technical Committee after discussion)
• Tests included, if applicable
• API docs and/or User Guide docs changed or added, if applicable

ALLOW_MANY_COMMITS=true

[mihnita]

TBD / Asking for opinions:

• There are two versions that need to be modified in MeasureUnitGeneratorTest
  It is unclear to me what they represent. Would need help from someone familiar with the code.
  Can I take them automatically from VersionInfo.ICU_VERSION.getMajor ?
  Or from command line?
• The generated files go in main/common_tests/ and have names stating with genunits_*
  They are easy to discover, as git status shows.
  But there is a risk that they get in the way, or get submitted.
  I can put them in the target folder, where they get cleaned automatically, and also become "invisible" to git. With pros and cons.
• Is it worth the trouble to try and modify the target files directly?
  Would require parsing the existing files to find what to replace.

[mihnita]

Hi Dragan,

I would especially appreciate your input, as you had to do this several times recently.
So you know the tricks, and also felt the pain.

Even better if you can spend 10 minutes or so to try it.

Separately, see my questions in the previous comment.
Do you know what is the meaning of the versions passed to the generators?
I find it suspicious that generateBackwardCompatibilityTest and generateCXXBackwardCompatibilityTest, which do the same thing for Java / C++, get different version numbers right now.

Thank you,
Mihai

[DraganBesevic]

Hi Dragan,

I would especially appreciate your input, as you had to do this several times recently. So you know the tricks, and also felt the pain.

Even better if you can spend 10 minutes or so to try it.

Separately, see my questions in the previous comment. Do you know what is the meaning of the versions passed to the generators? I find it suspicious that generateBackwardCompatibilityTest and generateCXXBackwardCompatibilityTest, which do the same thing for Java / C++, get different version numbers right now.

Thank you, Mihai

First, thank you very much for doing this. Any improvement in this process of updating units is very welcomed! I wish I could have better answers, but my experience with this code is mostly based on the latest digging and trying to figure out what it actually does and how it works.

Having said that, I am pretty sure that those two versions should be the same. In the previous workflow, there was a lot of manual updating of those versions values before running the unit test to get the proper output, so it is very possible that I didn't update it with 76 because I had that last minute battle to get the latest units in.

How it worked out was, one had to manually uncomment the line targeted for generating a particular code, update the version, then comment it back. Rinse and repeat for several lines.

[markusicu]

For this PR, when it's all done, please do not squash it down to one commit. IMO the first commit should be separate. Other commits should probably be squashed though.

[mihnita]

For this PR, when it's all done, please do not squash it down to one commit. IMO the first commit should be separate. Other commits should probably be squashed though.

ACK. I thought so too.

[mihnita]

Implemented (almost all) review feedback.
Explained why not when I didn't.

[mihnita]

About version: I took git-clone(d) the repo, then did a git checkout on each release-* tag starting with release-60, all the way to release-76-1. Including RC and betas and what not.
Did a grep for all the calls on the generators (the call, inside testZZZ)

And in most of them the version in the Java file matches the ICU version (in the git tag).
In some the version in the code is one step back. I guess that the file was not commited.

In all cases (60-76) all generators were called with the same version, except ICU 76.
So it was probably a case of "not submitted".

So I changed the code to get the ICU version automatically.
No need to update it by hand anymore.

[mihnita]

Any opinions about where to put the generated files?

Right now they go in main/common_tests/

So if you do a git status you will see:

Untracked files:
  (use "git add <file>..." to include in what will be committed)
	main/common_tests/genunits_MeasureUnit.java
	main/common_tests/genunits_MeasureUnitCompatibilityTest.java
	main/common_tests/genunits_MeasureUnitGeneratorTest.java
	main/common_tests/genunits_measfmttest.cpp
	main/common_tests/genunits_measunit.cpp
	main/common_tests/genunits_measunit.h

They are more discoverable, but there is a risk that they are added to the repo if nobody notices them in review.

I can put them in main/common_tests/target.
They become "invisible" to git stat, but also no risk to submit them by mistake.

[markusicu]

I can put them in main/common_tests/target.
They become "invisible" to git stat, but also no risk to submit them by mistake.

ok

[mihnita]

I can put them in main/common_tests/target.
They become "invisible" to git stat, but also no risk to submit them by mistake.

ok

DONE

[mihnita]

Everything from Markus is implemented.
Thank you very much.

[mihnita]

@markusicu: OK if I squash this in 2 commits (renames / split files, and all the rest)

Thanks,
M.

[markusicu]

OK if I squash this in 2 commits (renames / split files, and all the rest)

sgtm

[jira-pull-request-webhook]

Hooray! The files in the branch are the same across the force-push. 😃

~ Your Friendly Jira-GitHub PR Checker Bot

[mihnita]

OK if I squash this in 2 commits (renames / split files, and all the rest)

sgtm

Squashed to 2 commits.

[markusicu]

(partial rs)lgtm