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

Support icu provided by Windows #248

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

paperchalice
Copy link

Windows provided icu.lib since Windows 10 Version 1903.

@paperchalice paperchalice force-pushed the icu-win branch 6 times, most recently from ffc98e2 to 7bb32ed Compare January 9, 2025 14:17
meson.build Outdated
if enable_runtime == 'auto'
enable_runtime = 'libicu'
endif
elif libicu_dep.found()
Copy link
Author

Choose a reason for hiding this comment

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

Should there be an option here to force the use of external icu?

Copy link
Collaborator

Choose a reason for hiding this comment

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

I'd say yes, since otherwise it sounds unpredictable to assume the resulting build will work. e.g you provide static ICU libraries in your build environment, upgrade your libpsl clone, and suddenly it starts linking to a different DLL shipped with only certain versions of Windows, so application distribution to older Windows systems breaks.

(Why can't Windows just ship actual forreal ICU...)

@@ -57,6 +58,16 @@ if ['libicu', 'auto'].contains(enable_runtime)
endif
endif

if ['libicu-win', 'auto'].contains(enable_runtime)
libicu_win_dep = cc.find_library('icu', required: false)
Copy link
Collaborator

Choose a reason for hiding this comment

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

Please also skip this for "auto" when the host machine isn't Windows, as it can't be found anyway. Result: configuring on Linux or macOS or BSD with runtime == auto is a bit faster due to running one less check.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants