Bug 70995 - Update Code to Latest ICU 52.1
Summary: Update Code to Latest ICU 52.1
Status: RESOLVED FIXED
Alias: None
Product: LibreOffice
Classification: Unclassified
Component: LibreOffice (show other bugs)
Version:
(earliest affected)
unspecified
Hardware: All All
: medium enhancement
Assignee: Eike Rathke
URL:
Whiteboard: target:4.2.0
Keywords: difficultyInteresting, easyHack, skillScript
Depends on:
Blocks: ICU
  Show dependency treegraph
 
Reported: 2013-10-29 12:07 UTC by Robert M Campbell
Modified: 2022-05-15 00:27 UTC (History)
3 users (show)

See Also:
Crash report or crash signature:


Attachments
Patch 1 of 13 to update to ICU 52.1 (4.82 KB, patch)
2013-11-10 07:51 UTC, Robert M Campbell
Details
Patch 2 of 13 to update to ICU 52.1 (2.57 KB, patch)
2013-11-10 10:37 UTC, Robert M Campbell
Details
Patch 3 of 13 to update to ICU 52.1 (1.24 KB, patch)
2013-11-10 10:43 UTC, Robert M Campbell
Details
Patch 4 of 13 to update to ICU 52.1 (4.08 KB, patch)
2013-11-10 11:22 UTC, Robert M Campbell
Details
Patch 5 of 13 to update to ICU 52.1 (1.78 KB, patch)
2013-11-10 11:27 UTC, Robert M Campbell
Details
Patch 6 of 13 to update to ICU 52.1 (703 bytes, patch)
2013-11-10 11:33 UTC, Robert M Campbell
Details
Patch 7 of 13 to update to ICU 52.1 (416 bytes, patch)
2013-11-10 11:40 UTC, Robert M Campbell
Details
Patch 8 of 13 to update to ICU 52.1 (610 bytes, patch)
2013-11-10 11:43 UTC, Robert M Campbell
Details
Patch 9 of 13 to update to ICU 52.1 (7.74 KB, patch)
2013-11-10 11:54 UTC, Robert M Campbell
Details
Patch 10 of 13 to update to ICU 52.1 (1.36 KB, patch)
2013-11-10 12:01 UTC, Robert M Campbell
Details
Patch 11 of 13 to update to ICU 52.1 (344 bytes, patch)
2013-11-10 12:04 UTC, Robert M Campbell
Details
Patch 12 of 13 to update to ICU 52.1 (1.18 KB, patch)
2013-11-10 12:07 UTC, Robert M Campbell
Details
Patch 13 of 13 to update to ICU 52.1 (600 bytes, patch)
2013-11-10 12:10 UTC, Robert M Campbell
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Robert M Campbell 2013-10-29 12:07:28 UTC
Go to any Lao website, such as laosabbathschool.com. Lao text keeps going on and on until there is a space (Lao does not use spaces as word delimeters, but similar to punctuation.)

Lao text should split words like the ICU 52.1 system now can do... (This is very similar to Thai / Khmer word wrapping / boundary analysis.)

Can you all update LibreOffice to include the latest ICU 52.1? I am running Ubuntu, so I would think it would notice my locally installed ICU, but it doesn't. Also, there are loads of other systems that could benefit from this code as well.
Comment 1 Eike Rathke 2013-10-29 13:17:32 UTC
The only platform we bundle an internal ICU is Windows, all other platforms usually use whatever is the current ICU on their system. Note that installing another ICU on your system does not make LibreOffice use that other ICU because the ICU versioning mechanism which it was compiled against prevents that.

However, upgrading the internal ICU to 52.1 (or whatever will be current when this request is picked up) is a valid request anyway.
Comment 2 Robert M Campbell 2013-10-29 14:18:03 UTC
I am thinking having it in Windows would be useful - the only solution that exists in the language right now is specifically for MS Office, which means almost eveything in the country is pirated. On top of that, the solution requires a third-party add-on, which is well done, but not open source. It would be awesome if Laos could get away from pirated MS Office and move to open source. :)

As for Linux and Mc side of things, if it could look for the latest (52.1), that would be good. I currently have systems with LibreOffice that would benefit (and I know quite a few friends who would like to use it as well). I have ICU installed but LibreOffice is using something else, and not the latest version (not sure how to override that - but workaround hints could be hlpeful).
Comment 3 Eike Rathke 2013-10-29 15:42:10 UTC
You can't override the ICU version unless you build LibreOffice yourself against a newer ICU. ICU C++ symbols include the version used as namespace prefix.
Comment 4 Robert M Campbell 2013-10-30 04:59:51 UTC
Ok, makes perfect sense now! (Had been told by several others - install latest ICU on system and it'll just work, but obviously it's not that easy...)

Is it possible for me to compile against the newer version just by modifying LibreOffice, or should it just be as simple as dl-and-compile then LibreOffice will use the latest?
Comment 5 Robert M Campbell 2013-10-30 13:07:55 UTC
Just built it for Ubuntu 13.10. And it works like a charm with Lao!

Is it possible for the next pre-release binaries (Linux / Windows / Mac) to be built with ICU 52.1? Or, I guess it's probably the same as asking if the internally used ICU can be upgraded to ICU 52.1...

I know of several who would test it here in Laos...

Or, is that something I can take on personally? (I don't mind helping contribute to LibreOffice and the like on a regular basis) What do I need to do in order to help in that process?
Comment 6 Eike Rathke 2013-10-30 20:25:00 UTC
Hey great it worked! And if you'd like to help out and invest some time
to substitute the internal ICU so we'd be good at least on Windows I'd
really appreciate. I just uploaded
9e96ed4c1d99c0d14ac03c140f9f346c-icu4c-52_1-src.tgz to our lookaside
cache of external sources so it can be used by builds once things are
setup. So what to do:

In current master the ICU build related things are located in the
directory external/icu/

There 
* in the UnpackedTarball_icu.mk file remove the
  gb_UnpackedTarball_set_pre_action labeled "# *ONLY* for ICU 51(.1)!"
  that unpacks ICU_51_LAYOUT_FIX_TARBALL
* for each patch in gb_UnpackedTarball_add_patches
  * check if the patch still needs to be applied or if for the ICU
    ticket mentioned, if any, the code was fixed
  * check if a needed patch applies correctly
    * if not, adapt the patch if applicable
      * if unsure whether a patch should still be applied or how the
        whole thing works just cry for help ;-)

In the build root:
* edit download.lst and
  * replace
    export ICU_TARBALL := 6eef33b229d0239d654983028c9c7053-icu4c-51_1-src.tgz
    with
    export ICU_TARBALL := 9e96ed4c1d99c0d14ac03c140f9f346c-icu4c-52_1-src.tgz
  * remove the line
    export ICU_51_LAYOUT_FIX_TARBALL := 7650341b04f05ff2595bf064f3e41f41-icu-51-layout-fix-10107.tgz
* edit configure.ac and search for ICU_MAJOR=51 and change to
  ICU_MAJOR=52

Then (be sure you did not have --with-system-icu configured ...)

make clean && ./autogen.sh && make && make check

and cross fingers ;-)  Run the suite and check things you are interested
in. If everything's fine create a commit and submit it to our gerrit for
review, see https://wiki.documentfoundation.org/Development/gerrit for
details about working on a branch, creating commits and submitting to
gerrit.

I hope that helps and works, and thanks again.
Comment 7 Eike Rathke 2013-10-30 20:33:07 UTC
One thing to check after ./autogen.sh just to be sure that indeed the new internal ICU is used: the file config_host.mk should contain

export ICU_MAJOR=52
export ICU_MINOR=1
export SYSTEM_ICU=NO
Comment 8 Robert M Campbell 2013-10-31 06:53:38 UTC
I am reading a bit on crosscompiling on Linux for Windows XP - that would take a LOT less time than building it on Windows XP (which is a VM) and requiring the download of loads of Windows code I won't normally need on that VM.

I've been reading http://www.derivativezero.com/blog/2012/07/tech-update-libreoffice-cross-compile-msi-installer-generation/. Seems doable, but not quite sure what the best seetings are...

The settings that site lists are:
CC=ccache i686-w64-mingw32-gcc
CXX=ccache i686-w64-mingw32-g++
CC_FOR_BUILD=ccache gcc
CXX_FOR_BUILD=ccache g++
–with-distro=LibreOfficeMinGW

Does that build for Windows XP 32bit or a 64bit? Never done anything like this before... (It does, however, look really cool!)

Noting I am working on a amd 64 bit version of Ubuntu 13.10 Saucy, and wanting to build for Windows XP 32bit, what should the CC, CXX, etc be set to?

I'll report the results of my build if I can get it to crosscompile on Linux, or if my Internet speed allows me to do it on the Windows XP VM.
Comment 9 Robert M Campbell 2013-10-31 08:53:34 UTC
Access forbidden on http://dev-www.libreoffice.org/src/9e96ed4c1d99c0d14ac03c140f9f346c-icu4c-52_1-src.tgz

Will do an Ubuntu build first, then figure out how to cross compile for Windows XP or build on Windows XP.

To cross compile for Windows XP, what should build / host / target be set to?
Comment 10 Eike Rathke 2013-10-31 10:34:08 UTC
Oops, sorry, missed a chmod on the file, should work now.

I suggest to do all work on Linux and only if it works there do a Windows build. I never cross-compiled for Windows so can't help with that, but hopefully the README.cross file contains all necessary information. If you want to persuade that and run into trouble you may want to join the #libreoffice-dev channel on freenode.net IRC and/or the dev mailing list.

However, once the patch is on gerrit we can trigger a Windows build from that to see if it builds, so you don't necessarily have to build on/for Windows yourself.
Comment 11 Robert M Campbell 2013-10-31 11:19:13 UTC
Playing around with patches... At least one of the patches no longer works correctly, as the file has changed. I have disabled all the ICU patches, and will see if it will run without them. It may be wise to take a look at them as well...

The first IndicReordering one is the one it failed on.

Doing a on gerrit would be nice! (my system is good and all, but it's slower than some...)

README.cross doesn't work out-of-the-box for me currently... I could also be doing something wrong. It's complaining:

configure: error: cannot determine MinGW sysroot

Got all of the packages for MinGW-W64 for Ubuntu, but not sure what else I might be missing...

Could a testable snapshot for windows be built from gerrit?
Comment 12 Eike Rathke 2013-10-31 12:04:33 UTC
Patches that don't apply anymore because a file has changed but otherwise does not implement the fix the patch provides will have to be adapted to the new file and a new patch be generated to be included in the build.

Running ICU without the patches probably is possible, but each addresses a different problem.

Sorry, no idea about the MinGW stuff, I suggest to ask on the mailing list or IRC.
Comment 13 Robert M Campbell 2013-10-31 13:12:34 UTC
Good news,  it compiles and runs nicely - at least on my machine. :)

As for the patches, this is what my research has come up with so far...

10318 -> fixed in ICU 52.1, patch not needed, I think

7601 -> Closed, but not may not be fixed
http://bugs.icu-project.org/trac/ticket/7601
The bug was closed, but may not be satisfactory
I think the old patch should be adjusted and used

8198 -> Open - Patch still needed
http://bugs.icu-project.org/trac/ticket/8198

9948 -> Open - Patch still needed
Major Target for 53.1 2014-04-02
http://bugs.icu-project.org/trac/ticket/9948

10129 -> Fixed, patch not needed, I think

---

The rest probably just need to be adjusted to the new code...

I think I will give it a try to add the patches to here once I am done.
Comment 14 Robert M Campbell 2013-11-01 02:34:24 UTC
Sadly, it looks like I won't be able to do the patches the same way as they were done before at this time. 

I downloaded a compressed version of the source previously, due to the slower speeds we have of DSL here in Laos. I wanted to grab the source from git, but it failed last night - will try again, but not certain it will work.

I have not worked with many (maybe one or two) patches, and I think I was mostly applying them as opposed to making them. Does it matter if the file locations noted in the top part is different?

Also, my patches look different than the format used in the previous patches. Not using Git, is it possible for me to replicate the same format here using diff? If so, how?
Comment 15 Eike Rathke 2013-11-02 10:06:16 UTC
The file locations in the diff header may be different, as long as the number of subdirectories (should be one) before icu/... in the paths match, you can edit them to suit the needs. Something like misc/icu/... should do.

The patches applied during build are not generated using git, just plain diff of original and modified source. As for the diff format we use unified diffs, diff -u
Comment 16 Robert M Campbell 2013-11-10 07:51:08 UTC
Created attachment 88959 [details]
Patch 1 of 13 to update to ICU 52.1

This is my first attempt at creating a patch, so please review throughouly. Any suggestions are greatly helpful! (Patch 1 of 13)
Comment 17 Robert M Campbell 2013-11-10 10:37:19 UTC
Created attachment 88962 [details]
Patch 2 of 13 to update to ICU 52.1
Comment 18 Robert M Campbell 2013-11-10 10:43:17 UTC
Created attachment 88963 [details]
Patch 3 of 13 to update to ICU 52.1
Comment 19 Robert M Campbell 2013-11-10 11:22:44 UTC
Created attachment 88965 [details]
Patch 4 of 13 to update to ICU 52.1
Comment 20 Robert M Campbell 2013-11-10 11:27:47 UTC
Created attachment 88966 [details]
Patch 5 of 13 to update to ICU 52.1
Comment 21 Robert M Campbell 2013-11-10 11:33:36 UTC
Created attachment 88967 [details]
Patch 6 of 13 to update to ICU 52.1
Comment 22 Robert M Campbell 2013-11-10 11:40:13 UTC
Created attachment 88968 [details]
Patch 7 of 13 to update to ICU 52.1
Comment 23 Robert M Campbell 2013-11-10 11:43:22 UTC
Created attachment 88970 [details]
Patch 8 of 13 to update to ICU 52.1
Comment 24 Robert M Campbell 2013-11-10 11:54:16 UTC
Created attachment 88972 [details]
Patch 9 of 13 to update to ICU 52.1
Comment 25 Robert M Campbell 2013-11-10 12:01:14 UTC
Created attachment 88973 [details]
Patch 10 of 13 to update to ICU 52.1
Comment 26 Robert M Campbell 2013-11-10 12:04:13 UTC
Created attachment 88974 [details]
Patch 11 of 13 to update to ICU 52.1
Comment 27 Robert M Campbell 2013-11-10 12:07:54 UTC
Created attachment 88975 [details]
Patch 12 of 13 to update to ICU 52.1
Comment 28 Robert M Campbell 2013-11-10 12:10:33 UTC
Created attachment 88976 [details]
Patch 13 of 13 to update to ICU 52.1
Comment 29 Robert M Campbell 2013-11-10 12:11:54 UTC
Ok, got them finished... My first set of patches I've ever submitted, so please review carefully. It seems as though several of these weren't very different than the original patches, so may not need to change.
Comment 30 Robert M Campbell 2013-11-10 13:13:36 UTC
Testing a build on it with the new patches, and for some reason icu4c-build.patch and icu4c-rpath.patch don't run. Tried to output the .rej files, but files were not found. (Not sure on the Android patch, but would like to make sure it works.)

Also, not sure to test the layout patches, which would be good to test. How can I test them?

When built without using icu4c-build.patch, icu4c-rpath.patch, and layout patches, seems to work fine! (Ubuntu 13.10 AMD64)
Comment 31 Eike Rathke 2013-11-12 17:38:27 UTC
Thanks a lot! Though I would had preferred one complete changeset on gerrit (see https://wiki.documentfoundation.org/Gerrit ) that would had included the changes to UnpackedTarball_icu.mk, download.lst, configure.ac and added/removed files, I'll give this a try.

What do you mean with "test the layout patches"?

icu4c-build.patch is for building on Darwin and Arm, so you won't notice a difference on other platforms.

icu4c-rpath.patch sets RPATH=$ORIGIN for relocatable runtime linkage dependencies, so building without you don't notice anything except when installing the package and something isn't found or pulled from the wrong location.
Comment 32 Robert M Campbell 2013-11-13 08:00:30 UTC
https://gerrit.libreoffice.org/6661

Here's what I have so far. I think I found my problem. I may have changed too much in the process of altering the patches (since I am new to modifying patches). In the build and rpath in particular, I think I removed some important code. I will modify and see if I can get them working again.

The layout patches refer to the ICU 51 Layout Tarball. Because it's ICU 51, I am not sure if they need updated?

Not fully understanding everything I am doing, but starting to get the hang of it. Thanks for your coaching! It's making a big difference.
Comment 33 Robert M Campbell 2013-11-13 10:32:30 UTC
I found the problems in the rpath and build patches, and I have tested it on my system and the patches seem to work.

(I wasn't able to amend the changeset correctly, so made a new draft for review...)

https://gerrit.libreoffice.org/6666 (Interesting number combo, eh?)
Comment 34 Eike Rathke 2013-11-13 12:01:33 UTC
The layout "patch" is only for ICU 51 and earlier, ICU 52 already contains the code. Apart from that we don't use the ICU layout engine any longer anyway, but HarfBuzz instead, which is also what ICU recommends.

Patch looks almost good, see my comments there, I'll upload an amended version to gerrit.
Comment 35 Commit Notification 2013-11-13 15:47:27 UTC
Robert M Campbell committed a patch related to this issue.
It has been pushed to "master":

http://cgit.freedesktop.org/libreoffice/core/commit/?id=8ab4752820331ab80410877836093630198812d8

upgrade to ICU 52.1, fdo#70995



The patch should be included in the daily builds available at
http://dev-builds.libreoffice.org/daily/ in the next 24-48 hours. More
information about daily builds can be found at:
http://wiki.documentfoundation.org/Testing_Daily_Builds
Affected users are encouraged to test the fix and report feedback.
Comment 36 Robinson Tryon (qubit) 2015-12-16 00:40:22 UTC
Migrating Whiteboard tags to Keywords: (EasyHack DifficultyInteresting SkillScript )
[NinjaEdit]