Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[icu] Add tools #11897

Merged

Conversation

NancyLi1013
Copy link
Contributor

Currently, the executables generated in _vcpkg\buildtrees\icu\x64-windows-rel\bin are not provided in vcpkg.
Some ports require some of these executables to build with ICU support enabled.

So I add these to tools/icu directory.

Fix #11782

Note: No feature needs to test.

@NancyLi1013 NancyLi1013 added the info:internal This PR or Issue was filed by the vcpkg team. label Jun 12, 2020
@JackBoosY JackBoosY added the category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist label Jun 12, 2020
@rleigh-codelibre
Copy link
Contributor

Tested this branch with VS2019 Community and Xerces-C++ master with -Dtranscoder=icu -Dmsgloader=icu and it successfully detected all the needed libraries and executables:

//ICU data library (debug)
ICU_DATA_LIBRARY_DEBUG:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/debug/lib/icudtd.lib

//ICU data library (release)
ICU_DATA_LIBRARY_RELEASE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/lib/icudt.lib

//ICU derb executable
ICU_DERB_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/derb.exe

//ICU genbrk executable
ICU_GENBRK_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/genbrk.exe

//ICU genccode executable
ICU_GENCCODE_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/genccode.exe

//ICU gencfu executable
ICU_GENCFU_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gencfu.exe

//ICU gencmn executable
ICU_GENCMN_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gencmn.exe

//ICU gencnval executable
ICU_GENCNVAL_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gencnval.exe

//ICU gendict executable
ICU_GENDICT_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gendict.exe

//ICU gennorm2 executable
ICU_GENNORM2_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gennorm2.exe

//ICU genrb executable
ICU_GENRB_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/genrb.exe

//ICU gensprep executable
ICU_GENSPREP_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/gensprep.exe

//ICU icu-config executable
ICU_ICU-CONFIG_EXECUTABLE:FILEPATH=ICU_ICU-CONFIG_EXECUTABLE-NOTFOUND

//ICU icuinfo executable
ICU_ICUINFO_EXECUTABLE:FILEPATH=ICU_ICUINFO_EXECUTABLE-NOTFOUND

//ICU icupkg executable
ICU_ICUPKG_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/icupkg.exe

//ICU include directory
ICU_INCLUDE_DIR:PATH=/Users/rleigh/code/vcpkg/installed/x64-windows/include

//ICU makeconv executable
ICU_MAKECONV_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/makeconv.exe

//ICU pkgdata executable
ICU_PKGDATA_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/pkgdata.exe

//ICU uconv executable
ICU_UCONV_EXECUTABLE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/tools/icu/uconv.exe

//ICU uc library (debug)
ICU_UC_LIBRARY_DEBUG:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/debug/lib/icuucd.lib

//ICU uc library (release)
ICU_UC_LIBRARY_RELEASE:FILEPATH=/Users/rleigh/code/vcpkg/installed/x64-windows/lib/icuuc.lib

So the change looks fine as is. But if you need to have the tools optional as a [tools] component that would also work fine.

@NancyLi1013
Copy link
Contributor Author

@rleigh-codelibre
Thanks for your test and suggestion.
It must be better if we add tools as a feature for icu. But since icu is not built via CMake. So I have no idea about how to implement this. Do you have any suggestions?

@NancyLi1013
Copy link
Contributor Author

If there are no any suggestions about this, I will add tools as current statues.

@rleigh-codelibre
Copy link
Contributor

I think this PR is fine as it is in terms of adding the tool support.

Making it a separate component would be nice, but I'll have to have a dig into the vcpkg internals to see how to do it. Could that be done as a followup?

@NancyLi1013
Copy link
Contributor Author

Of course you can do that. In my opinion, we can consider to add it as a separate component in the future. Now what we need to do is to make sure that it can be used.

@rleigh-codelibre
Copy link
Contributor

My testing above with the CMake FindICU module using find_package(ICU) demonstrated that all the executables are correctly found and are properly functional in the context of a Xerces-C++ package build.

@NancyLi1013 NancyLi1013 marked this pull request as ready for review June 18, 2020 09:17
@JackBoosY JackBoosY added the info:reviewed Pull Request changes follow basic guidelines label Jun 19, 2020
@vicroms vicroms merged commit e33e3fc into microsoft:master Jun 23, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-feature The issue is with a library, which is requesting new capabilities that didn’t exist info:internal This PR or Issue was filed by the vcpkg team. info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[icu] Essential tools not provided
4 participants