-
Notifications
You must be signed in to change notification settings - Fork 5k
[release/8.0-staging] Add support for more libicu versions #115378
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
[release/8.0-staging] Add support for more libicu versions #115378
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Pull Request Overview
This PR adds support for additional ICU library versions to accommodate changes in the Debian Trixie package repository.
- Introduces ICU versions 78, 77, and 76 into the KnownLibIcuVersion list
- Extends the dependency matching to reduce future maintenance effort
src/installer/pkg/sfx/installers/dotnet-runtime-deps/dotnet-runtime-deps-debian.proj
Show resolved
Hide resolved
It would be nice to devise a more flexible scheme matching its native counterpart: runtime/src/native/libs/System.Globalization.Native/pal_icushim.c Lines 242 to 243 in 8d07196
Max version is relative to the libicu version available on build machine. |
Yeah, it would be great to not touch this again. I'll create an issue to track the long-term fix. |
@@ -6,7 +6,7 @@ | |||
<ItemGroup> | |||
<LinuxPackageDependency Include="libc6;libgcc1;libgssapi-krb5-2;libstdc++6;zlib1g"/> | |||
<LinuxPackageDependency Include="libssl1.0.0 | libssl1.0.2 | libssl1.1 | libssl3" /> | |||
<KnownLibIcuVersion Include="74;72;71;70;69;68;67;66;65;63;60;57;55;52" /> | |||
<KnownLibIcuVersion Include="78;77;76;74;72;71;70;69;68;67;66;65;63;60;57;55;52" /> |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I gave a similar suggestion above, @NikolaMilosavljevic will track it separately. I think there we could also revisit if we need the version guard any longer, and consider switching to feature detection.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Future improvement is now tracked in #115407
This was approved for servicing - updating the label. |
4a0fef3
into
dotnet:release/8.0-staging
Fixes: #115274
Debian Trixie package repository was updated to include just
libicu76
. We requirelibicu72
.Besides version 76, this PR also adds versions 77 and 78, to limit the need to change this file often.