Skip to content

Conversation

@radekdoulik
Copy link
Member

So that binary search works properly.

This updates bash script with similar change as
#122403 did for powershell script.

So that binary search works properly.

This updates bash script with similar change as
dotnet#122403 did for powershell
script.
@radekdoulik radekdoulik added this to the 11.0.0 milestone Dec 11, 2025
Copilot AI review requested due to automatic review settings December 11, 2025 11:10
@dotnet-policy-service
Copy link
Contributor

Tagging subscribers to this area: @dotnet/area-system-resources
See info in area-owners.md if you want to be subscribed.

Copy link
Contributor

Copilot AI left a 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 fixes a bug in the bash script that generates native string resource arrays for CoreCLR by ensuring the resource entries are sorted numerically by ID. Without sorting, binary search operations performed by bsearch() in LoadNativeStringResource() will not work correctly.

Key changes:

  • Modified the bash script to sort resource array keys numerically before generating output, matching the existing PowerShell script behavior

@janvorli
Copy link
Member

The bash change is not needed, it generates things sorted already.

@radekdoulik
Copy link
Member Author

radekdoulik commented Dec 11, 2025

The order is not guaranteed AFAIK. https://www.gnu.org/software/bash/manual/html_node/Arrays.html doesn't specify it, so I think it is implementation specific.

@janvorli
Copy link
Member

Hmm, we never had an issue with that on any Unix OS. Ok, the added sort doesn't hurt.

@radekdoulik
Copy link
Member Author

/ba-g the failing test is timeout, the hidden one is unrelated (failing on windows)

@radekdoulik radekdoulik merged commit 0138cb7 into dotnet:main Dec 12, 2025
92 of 99 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants