icu
icu copied to clipboard
unicode-org
ICU-21628 MinGW/Windows incorrect build
Checklist
- [x] Required: Issue filed: https://unicode-org.atlassian.net/browse/ICU-21628
- [x] Required: The PR title must be prefixed with a JIRA Issue number.
- [x] Required: The PR description must include the link to the Jira Issue, for example by completing the URL in the first checklist item
- [x] Required: Each commit message must be prefixed with a JIRA Issue number.
- [ ] Issue accepted (done by Technical Committee after discussion)
- [x] Tests included, if applicable
- [x] API docs and/or User Guide docs changed or added, if applicable
Mingw* configuration/build/setup has some pitfalls:
1. pkgdata utility produces incorrect datastub filenames when LIBPREFIX is set.
With
LIBPREFIX=lib
createFileNames (from tools/pkgdata/pkgdata.cpp:926) produces liblibicu* names
2. Less important, also pkgdata utility:
--help option message contains incorrect prefixes/suffixes (from tools/pkgdata/pkgtypes.h:139) defined at struct modes (from tools/pkgdata/pkgdata.cpp:134)
3. Default build PKGDATA_MODE (= dll ) with ENABLE_STATIC
Makefile puts libicudtXX.a static library to bindir (data/Makefile.in:201 install-local target)
4. Platform-dependant definitions of linker flags are inconsistent (example)
ICU Issue link: https://unicode-org.atlassian.net/browse/ICU-21628
Mingw* configuration/build/setup has some pitfalls:
1. pkgdata utility produces incorrect datastub filenames when LIBPREFIX is set.
With
LIBPREFIX=lib
createFileNames (from tools/pkgdata/pkgdata.cpp:926) produces liblibicu* names
2. Less important, also pkgdata utility:
--help option message contains incorrect prefixes/suffixes (from tools/pkgdata/pkgtypes.h:139) defined at struct modes (from tools/pkgdata/pkgdata.cpp:134)
3. Default build PKGDATA_MODE (= dll ) with ENABLE_STATIC
Makefile puts libicudtXX.a static library to bindir (data/Makefile.in:201 install-local target)
4. Platform-dependant definitions of linker flags are inconsistent
ICU Issue link: https://unicode-org.atlassian.net/browse/ICU-21628
Thank you very much for creating this PR, signing the CLA, and also for filing a ticket! :)
To be honest, I'm not super familiar with MinGW and its conventions, so I would love if someone else with MinGW experience and expertise could help take a look at these changes.
Recently, there have been other pull-requests from a number of people that also made changes to the ICU MinGW build/config: @longnguyen2004 @autoantwort @pchemguy. (I'm mentioning them here, in case they are interested in taking a look).
I don't know if any of them would have time or not -- but if so, it would be great if they might be able to take a look at the changes in this pull request (since it also changes the MinGW build as well).
Also, I think this change will conflict with some of the changes in PR #1732. At minimum, we'd need to resolve the merge conflicts, but I wonder if maybe the PR author could look at this change as well.
Regarding this:
- Platform-dependant definitions of linker flags are inconsistent
FWIW, I think the PR #1732 was also trying to fix this as well maybe.
Hello @LiteratimBi, and thanks for making this PR!
I just did a test build with --enable-static and everything seems good so far, I'll try a build with --enable-shared.
EDIT:
Shared build looks fine for lib, but not for bin
lib folder:
icu
libicudt.dll.a
libicuin.dll.a
libicuio.dll.a
libicutest.dll.a
libicutu.dll.a
libicuuc.dll.a
pkgconfig
bin folder:
derb.exe
escapesrc.exe
genbrk.exe
genccode.exe
gencfu.exe
gencmn.exe
gencnval.exe
gendict.exe
gennorm2.exe
genrb.exe
gensprep.exe
icu-config
icuinfo.exe
icupkg.exe
libicudt69.dll
libicuin69.dll
libicuio69.dll
libicutest69.dll
libicutu69.dll
libicuuc69.dll
liblibicudt69.dll
makeconv.exe
pkgdata.exe
uconv.exe
There's an extra liblibicudt69.dll. Can you check where it comes from?
Hello @LiteratimBi, and thanks for making this PR! I just did a test build with
--enable-staticand everything seems good so far, I'll try a build with--enable-shared.EDIT: Shared build looks fine for
lib, but not forbinlibfolder:icu libicudt.dll.a libicuin.dll.a libicuio.dll.a libicutest.dll.a libicutu.dll.a libicuuc.dll.a pkgconfig
binfolder:derb.exe escapesrc.exe genbrk.exe genccode.exe gencfu.exe gencmn.exe gencnval.exe gendict.exe gennorm2.exe genrb.exe gensprep.exe icu-config icuinfo.exe icupkg.exe libicudt69.dll libicuin69.dll libicuio69.dll libicutest69.dll libicutu69.dll libicuuc69.dll liblibicudt69.dll makeconv.exe pkgdata.exe uconv.exeThere's an extra
liblibicudt69.dll. Can you check where it comes from?
According to size, libicudt69.dll is pre-build wrapper and liblibicudt69.dll is the correct datastub
Are you shure that you using correct branch (git checkout mingw-make)? If yes, autoconf output might be helpfull
Mine bindir looks so:
total 42368
327900 May 27 09:23 derb.exe
534607 May 27 09:23 escapesrc.exe
317569 May 27 09:23 genbrk.exe
297824 May 27 09:23 genccode.exe
315461 May 27 09:23 gencfu.exe
296130 May 27 09:23 gencmn.exe
311228 May 27 09:23 gencnval.exe
325227 May 27 09:23 gendict.exe
398208 May 27 09:23 gennorm2.exe
510968 May 27 09:23 genrb.exe
311538 May 27 09:23 gensprep.exe
25605 May 27 09:23 icu-config
298983 May 27 09:23 icuinfo.exe
308523 May 27 09:23 icupkg.exe
28870610 May 27 09:23 libicudt69.dll
4878882 May 27 09:23 libicuin69.dll
176751 May 27 09:23 libicuio69.dll
352897 May 27 09:23 libicutest69.dll
772231 May 27 09:23 libicutu69.dll
2648525 May 27 09:23 libicuuc69.dll
348183 May 27 09:23 makeconv.exe
344525 May 27 09:23 pkgdata.exe
354520 May 27 09:23 uconv.exe
I merged your branch on top of main. I'll try doing a build without it and see what happens.
Hmm, maybe I did something wrong. I'll do some more investigation. If anyone's interested in reproducing it, I'm cross-compiling from WSL2, using llvm-mingw.
Hooray! The files in the branch are the same across the force-push. 😃
~ Your Friendly Jira-GitHub PR Checker Bot
![jira-pull-request-webhook[bot] avatar](https://avatars.githubusercontent.com/u/13873561?v=4 "Published by a pub.dev verified publisher"){kind=small}
I'm still getting liblibicudt69.dll for some reason. Can someone do a cross-compile from WSL/Ubuntu and see if the problem persists?
I'm still getting
liblibicudt69.dllfor some reason. Can someone do a cross-compile from WSL/Ubuntu and see if the problem persists?
Did you also get this problem with #1735?
It seems to be less intrusive than this PR, I'll do a build today and see if it works
It seems to be less intrusive than this PR, I'll do a build today and see if it works
Do you already have results? :)
Oh sorry, I forgot to comment on this one :)
The build works fine, but as expected, the static build puts the data library in the bin folder, and is also named icudt.a, which is incorrect. This is probably a problem in pkgdata.
And the native vs cross compile problem is still present on both PR, once again due to pkgdata (remember what I said about compile-time macros?), so neither of these would be a complete fix.
@longnguyen2004 when I understand the last comments on the other pr correctly this pr has less problems than the other pr right?
@longnguyen2004 when I understand the last comments on the other pr correctly this pr has less problems than the other pr right?
Yes that's correct, but this PR is also changing a lot of other things that I don't fully understand, so I say we should explore other approaches first, like fixing up pkgdata by itself, before settling on this.
Yes that's correct, but this PR is also changing a lot of other things that I don't fully understand, so I say we should explore other approaches first, like fixing up pkgdata by itself, before settling on this.
Imho this should be merged anyway because it fixes the icu static mingw builds that are currently broken and does not build at all.
@LiteratimBi Can you apply my suggestion in https://github.com/unicode-org/icu/pull/1733#discussion_r641305290 before moving on? This is a workaround for the cross compiling problem I had before.