[release/v7.4] Mirror .NET/runtime ICU version range in PowerShell#27137
Closed
adityapatwardhan wants to merge 1 commit intoPowerShell:release/v7.4from
Closed
[release/v7.4] Mirror .NET/runtime ICU version range in PowerShell#27137adityapatwardhan wants to merge 1 commit intoPowerShell:release/v7.4from
adityapatwardhan wants to merge 1 commit intoPowerShell:release/v7.4from
Conversation
Member
Author
|
Closing as the target branch is wrong |
Contributor
There was a problem hiding this comment.
Pull request overview
Backport to release/v7.4 updating Debian package dependency generation to mirror the .NET runtime’s ICU version range approach, replacing a hardcoded libicuXX alternative list with a computed range for improved forward compatibility.
Changes:
- Add ICU version range constants (
MinICUVersion,BuildICUVersion,MaxICUVersion) to drive dependency generation. - Replace the hardcoded Debian
libicualternative list with a dynamically generatedlibicu<max> | ... | libicu<min>string. - Minor formatting-only adjustment at end of file.
Comment on lines
+2133
to
+2138
| # - The runtime supports ICU versions >= the version it was built against | ||
| # and <= that version + 30, to allow sufficient headroom for future releases. | ||
| # - ICU typically releases about twice per year, so +30 provides roughly | ||
| # 15 years of forward compatibility. | ||
| # - On some platforms, the minimum supported version may be lower | ||
| # than the build version and we know that older versions just works. |
There was a problem hiding this comment.
The rationale comment says “supports ICU versions >= the version it was built against and <= that version + 30”, but the implementation explicitly allows versions down to $MinICUVersion (60) which is lower than the build version (74). Please reword the comment to match the actual range being generated (MinICUVersion..(BuildICUVersion+30)) and the .NET runtime behavior described by the linked source.
Suggested change
| # - The runtime supports ICU versions >= the version it was built against | |
| # and <= that version + 30, to allow sufficient headroom for future releases. | |
| # - ICU typically releases about twice per year, so +30 provides roughly | |
| # 15 years of forward compatibility. | |
| # - On some platforms, the minimum supported version may be lower | |
| # than the build version and we know that older versions just works. | |
| # - The effective supported ICU version range here is: | |
| # MinICUVersion (runtime minimum) .. (BuildICUVersion + 30). | |
| # - The upper bound follows the .NET runtime policy of supporting | |
| # ICU versions up to the version it was built against plus 30, | |
| # to allow sufficient headroom for future releases. | |
| # - ICU typically releases about twice per year, so +30 provides roughly | |
| # 15 years of forward compatibility beyond the build version. | |
| # - The lower bound is set to the runtime's minimum supported ICU | |
| # version, which may be lower than the build version on some | |
| # platforms where we know older versions work. |
| # - ICU typically releases about twice per year, so +30 provides roughly | ||
| # 15 years of forward compatibility. | ||
| # - On some platforms, the minimum supported version may be lower | ||
| # than the build version and we know that older versions just works. |
There was a problem hiding this comment.
Grammar: “older versions just works” should be “older versions just work”.
Suggested change
| # than the build version and we know that older versions just works. | |
| # than the build version and we know that older versions just work. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Backport of #26304 to release/v7.4
Triggered by @adityapatwardhan on behalf of @kasperk81
Original CL Label: CL-BuildPackaging
/cc @PowerShell/powershell-maintainers
Impact
REQUIRED: Choose either Tooling Impact or Customer Impact (or both). At least one checkbox must be selected.
Tooling Impact
Updates the Get-PackageDependencies function to use a dynamic ICU version range instead of a hardcoded list. This change applies to deb package dependencies and provides forward compatibility for future ICU releases while maintaining compatibility down to ICU 60.
Customer Impact
Regression
REQUIRED: Check exactly one box.
This is not a regression.
Testing
Verified that the dynamic ICU version range calculation works correctly with $BuildICUVersion=74 for v7.4, generating the expected version range from 60 to 104 for deb packages. This mirrors the .NET runtime approach for broader compatibility with different ICU versions.
Risk
REQUIRED: Check exactly one box.
The change expands the acceptable ICU versions for deb packages from a fixed list to a dynamic range (60-104 for v7.4), which increases compatibility without breaking existing functionality. The .NET runtime uses the same approach, providing validation for this pattern.