Skip to content

Conversation

@kasperk81
Copy link
Contributor

PR Summary

The .NET runtime defines ICU version ranges instead of tracking exact “running” versions.

This PR mirrors that approach in PowerShell: rather than hardcoding specific ICU versions, it provides headroom above the build version so Debian packages flow smoothly and remain compatible across a range of ICU versions.

Better fix for #25865.

PR Context

PR Checklist

@kasperk81 kasperk81 requested review from a team and jshigetomi as code owners October 26, 2025 11:43
@iSazonov iSazonov requested a review from TravisEz13 October 27, 2025 03:05
@iSazonov iSazonov added the CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log label Oct 27, 2025
@iSazonov
Copy link
Collaborator

For discussion. Could we take these constants from this file automatically?

@kasperk81
Copy link
Contributor Author

For discussion. Could we take these constants from this file automatically?

I don't know how to parse C source in PowerShell. Manual parsing sounds like a bad idea. Also, U_ICU_VERSION_MAJOR_NUM is defined in libicu-dev header.

@TravisEz13
Copy link
Member

For discussion. Could we take these constants from this file automatically?

I don't know how to parse C source in PowerShell. Manual parsing sounds like a bad idea. Also, U_ICU_VERSION_MAJOR_NUM is defined in libicu-dev header.

We should not assume what platform we are building on.

@kasperk81
Copy link
Contributor Author

We should not assume what platform we are building on.

@TravisEz13, this is agnostic of build platform? Or can you clarify what you meant?

@TravisEz13
Copy link
Member

We should not assume what platform we are building on.

@TravisEz13, this is agnostic of build platform? Or can you clarify what you meant?

We should be agnostic of build platform. So, if we do pull any information in, it needs to be from public .NET information.

@kasperk81
Copy link
Contributor Author

We should not assume what platform we are building on.

@TravisEz13 — is this agnostic of the build platform, or could you clarify what you meant?

We should be agnostic of the build platform. If we do pull any information in, it needs to come from public .NET information.

I’m happy with the current state of the PR, where we align with the version range used by the .NET runtime; which I believe you’re also okay with. The $BuildICUVersion can be updated once a year before each release. This implies the package remains compatible with up to 30 future ICU versions supported by the .NET runtime. Given ICU’s roughly biannual release cadence, that translates to about "15 years of compatibility with the latest libicu" for a given PowerShell version.

@TravisEz13
Copy link
Member

We should not assume what platform we are building on.

@TravisEz13 — is this agnostic of the build platform, or could you clarify what you meant?

We should be agnostic of the build platform. If we do pull any information in, it needs to come from public .NET information.

I’m happy with the current state of the PR, where we align with the version range used by the .NET runtime; which I believe you’re also okay with. The $BuildICUVersion can be updated once a year before each release. This implies the package remains compatible with up to 30 future ICU versions supported by the .NET runtime. Given ICU’s roughly biannual release cadence, that translates to about "15 years of compatibility with the latest libicu" for a given PowerShell version.

It would be wonderful if .NET exposed this and we could generate the code, but otherwise this seems like a step forward.
Maybe add comments on what the plan is to the script?

@TravisEz13
Copy link
Member

LGTM, but packaging has been undergoing some changes. CI is broken. I have a PR to fix it. We need to take that PR first and rerun tests.

#26315

@kasperk81
Copy link
Contributor Author

@TravisEz13, if it looks good, can this be merged? Will it be part of the upcoming release?

@TravisEz13
Copy link
Member

@kasperk81 we have all the changes to the packaging commited that we expect. I'll run the CI again and verify.

@kasperk81
Copy link
Contributor Author

@TravisEz13 i've fixed the typo from suggestion #26304 (comment). can you rerun the CI?

@microsoft-github-policy-service microsoft-github-policy-service bot added the Review - Needed The PR is being reviewed label Nov 7, 2025
@kasperk81
Copy link
Contributor Author

@TravisEz13 good to merge?

@daxian-dbw daxian-dbw merged commit bb44aa1 into PowerShell:master Nov 17, 2025
37 checks passed
@daxian-dbw
Copy link
Member

daxian-dbw commented Nov 17, 2025

Mark as 7.5-consider and 7.4-consider, because .NET changed the supported OS for .NET 8 and .NET 9 to include Debian 13 in the matrix.

https://github.com/dotnet/core/blob/main/release-notes/8.0/supported-os.md#linux
https://github.com/dotnet/core/blob/main/release-notes/9.0/supported-os.md#linux

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

BackPort-7.4.x-Consider BackPort-7.5.x-Consider BackPort-7.6.x-Consider CL-BuildPackaging Indicates that a PR should be marked as a build or packaging change in the Change Log

Projects

None yet

Development

Successfully merging this pull request may close these issues.

5 participants