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 host tool path #29329

Merged
merged 1 commit into from
Feb 7, 2023
Merged

[icu] Add host tool path #29329

merged 1 commit into from
Feb 7, 2023

Conversation

m-kuhn
Copy link
Contributor

@m-kuhn m-kuhn commented Jan 31, 2023

Fixes #29322

github-actions[bot]
github-actions bot previously approved these changes Jan 31, 2023
@m-kuhn m-kuhn changed the title Fix https://github.com/microsoft/vcpkg/issues/29322 [icu] Add host tool path Jan 31, 2023
@MonicaLiu0311 MonicaLiu0311 added the category:port-bug The issue is with a library, which is something the port should already support label Feb 1, 2023
@m-kuhn
Copy link
Contributor Author

m-kuhn commented Feb 1, 2023

./vcpkg install icu:x64-linux-dynamic

works fine for me

github-actions[bot]
github-actions bot previously approved these changes Feb 1, 2023
@m-kuhn m-kuhn marked this pull request as ready for review February 4, 2023 11:33
@m-kuhn m-kuhn mentioned this pull request Feb 6, 2023
21 tasks
Comment on lines 45 to 49
set(TOOL_PATH "${CURRENT_HOST_INSTALLED_DIR}/tools/${PORT}")
if(CMAKE_HOST_WIN32 AND VCPKG_TARGET_IS_MINGW AND NOT HOST_TRIPLET MATCHES "mingw")
# Assuming no cross compiling because the host (windows) pkgdata tool doesn't
# use the '/' path separator when creating compiler commands for mingw bash.
elseif(VCPKG_CROSSCOMPILING)
set(TOOL_PATH "${CURRENT_HOST_INSTALLED_DIR}/tools/${PORT}")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Does this line need to be moved? AFAICS it is only used near the old location.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I assumed x86-windows or x64-linux-dynamic are not crosscompiling but will still need the host tool path set to build. However, looking at it again now it seems that the build succeeded and only the copy command failed.
Will revert

@MonicaLiu0311 MonicaLiu0311 added the info:reviewed Pull Request changes follow basic guidelines label Feb 7, 2023
@JavierMatosD JavierMatosD merged commit 261f5b1 into microsoft:master Feb 7, 2023
@m-kuhn m-kuhn deleted the icu_host branch March 11, 2023 05:29
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
category:port-bug The issue is with a library, which is something the port should already support info:reviewed Pull Request changes follow basic guidelines
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[icu] Regression - build fails for x64-linux-release
4 participants