icu icon indicating copy to clipboard operation
icu copied to clipboard

ICU-21152 Add static configuration for Windows projects

Open ermshiperete opened this issue 4 years ago • 4 comments

When building on Linux it is possible to create static libraries by providing a command line option to runConfigureICU. However, when building using Visual Studio or msbuild we don't use this tool and so there is no command line option. This change adds a new configuration (DebugStatic and ReleaseStatic) that links against the static runtime libraries and thus allows to create static version of the ICU libraries by selecting the desired configuration.

Checklist
  • [x] Issue filed: https://unicode-org.atlassian.net/browse/ICU-21152
  • [x] Updated PR title and link in previous line to include Issue number
  • [x] Issue accepted
  • [ ] Tests included
  • [ ] Documentation is changed or added

ermshiperete avatar Jun 10 '20 14:06 ermshiperete

CLA assistant check
All committers have signed the CLA.

CLAassistant avatar Jun 10 '20 14:06 CLAassistant

Thank you kindly for creating this PR, and also for filing a ticket! 🎉

It's sort of a nit, but the ICU project requires that commits be prefixed with the ticket number in the commit message.

So instead of this:

Add static configuration for Windows projects

The commit title/message should look something like this:

ICU-21151 Add static configuration for Windows projects
  • Tip: You can use the command git commit --amend to edit the commit message for an existing commit. (Then you can force-push to update the branch and PR).
  • There's also some more detailed info on the page here: http://site.icu-project.org/repository/gitdev

However, it looks like the changes as-is don't build though.

If I attempt to build the "ReleaseStatic" configuration as-is using a clean check-out then I get the following 2 errors:

30>Unable to find "D:\c\jefgen-icu\icu4c\source\extra\uconv\..\..\..\bin\pkgdata.exe"
30>makedata.mak(77): fatal error U1050: The tool 'pkgdata.exe' does not exist! (Have you built all of ICU yet?).
30>Stop.
30>Done building project "uconv.vcxproj" -- FAILED.

32>NMAKE : fatal error U1073: don't know how to make '"D:\c\jefgen-icu\icu4c\source\tools\genrb\x64\Release\genrb.exe"'
32>Stop.
32>E:\vs2019\IDE\Enterprise\MSBuild\Microsoft\VC\v160\Microsoft.MakeFile.Targets(46,5): error MSB3073: The command "chcp 65001 >NUL && NMAKE /f makedata.mak ICUMAKE="D:\c\jefgen-icu\icu4c\source\data\\" CFG=x64\Release UWP=UWP" exited with code 2.
32>Done building project "makedata_uwp.vcxproj" -- FAILED.

If I first build the "Release" configuration (which builds successfully) and then to build the "ReleaseStatic" configuration, I get the following error:

1>Unable to find "D:\c\jefgen-icu\icu4c\source\extra\uconv\..\..\..\bin\pkgdata.exe"
1>makedata.mak(77): fatal error U1050: The tool 'pkgdata.exe' does not exist! (Have you built all of ICU yet?).
1>Stop.
1>Done building project "uconv.vcxproj" -- FAILED.

(The error about genrb.exe is no longer present).

It isn't clear to me from the PR how this is supposed to be built or used though. The generated *.lib files aren't actually static libraries, and are still import libraries.

I'm not sure, but if the intent (I think) is that we'd need to build the Release configuration first, before building the ReleaseStatic configuration, then it might be worth adding a note to the Readme.html file to explain. (Or perhaps file a follow-up ticket for updating the documentation).

jefgen avatar Jun 10 '20 23:06 jefgen

@jefgen Thanks for taking a look!

When I tried it I got the build errors with my changes as I got without my changes, so I assumed it'd work :smile:. Later that day I discovered it's caused by my build environment which was/is missing some dependencies...

I'll work through the problems and your comments, but I don't know when I'll get to it.

ermshiperete avatar Jun 12 '20 18:06 ermshiperete

Thanks for taking a look!

You're welcome -- thanks for creating a pull-request! :)

I'll work through the problems and your comments, but I don't know when I'll get to it.

No need to rush. I'll mark this PR as 'incomplete' for now so that it doesn't show up in any queries, and you can work on it when you get time.

Thanks again!

jefgen avatar Jun 12 '20 19:06 jefgen

No movement in a couple of years, and still outstanding negative feedback. Closing.

markusicu avatar Jan 12 '23 17:01 markusicu