Skip to content

Fix loading app-local ICU - #77065

Merged
tarekgh merged 7 commits into
dotnet:mainfrom
tarekgh:FixLoadingAppLocalIcu
Oct 17, 2022
Merged

Fix loading app-local ICU#77065
tarekgh merged 7 commits into
dotnet:mainfrom
tarekgh:FixLoadingAppLocalIcu

Conversation

@tarekgh

Copy link
Copy Markdown
Member

#77045

Awhile ago we had a change #68847 to fix the build breaks because of obsoleted ICU APIs. The fix was changing static binding to the API during the compile time to be a runtime binding. This change work fine except we missed to do some pointer initialization when having app-local ICU .

@ghost

Copy link
Copy Markdown

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

Issue Details

#77045

Awhile ago we had a change #68847 to fix the build breaks because of obsoleted ICU APIs. The fix was changing static binding to the API during the compile time to be a runtime binding. This change work fine except we missed to do some pointer initialization when having app-local ICU .

Author:tarekgh
Assignees:-
Labels:

area-System.Globalization

Milestone:-

@janvorlijanvorli left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM, thank you!

@tarekgh

Copy link
Copy Markdown
MemberAuthor

/azp run

@azure-pipelines

Copy link
Copy Markdown
You have several pipelines (over 10) configured to build pull requests in this repository. Specify which pipelines you would like to run by using /azp run [pipelines] command. You can specify multiple pipelines using a comma separated list.

@tarekgh

Copy link
Copy Markdown
MemberAuthor

/azp run runtime, dotnet-linker-tests, runtime-dev-innerloop

@azure-pipelines

Copy link
Copy Markdown
Azure Pipelines successfully started running 3 pipeline(s).

Comment threadsrc/libraries/System.Globalization/tests/IcuAppLocal/IcuAppLocal.cs Outdated
Comment threadsrc/libraries/System.Globalization/tests/IcuAppLocal/IcuAppLocal.cs Outdated
Comment threadsrc/libraries/System.Globalization/tests/IcuAppLocal/IcuAppLocal.cs Outdated
Comment threadsrc/libraries/System.Globalization/tests/IcuAppLocal/IcuAppLocal.cs Outdated

@jkotasjkotas left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

LGTM

@tarekgh

Copy link
Copy Markdown
MemberAuthor

The runtime (Build Build windows x64 checked NativeAOT) failure is unrelated and reported in the build analysis.

@tarekgh
tarekgh merged commit a54293e into dotnet:mainOct 17, 2022
@tarekgh
tarekgh deleted the FixLoadingAppLocalIcu branch October 17, 2022 14:42
@tarekgh

Copy link
Copy Markdown
MemberAuthor

/backport to release/6.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/6.0: https://github.com/dotnet/runtime/actions/runs/3266237250

@tarekgh

Copy link
Copy Markdown
MemberAuthor

/backport to release/7.0

@github-actions

Copy link
Copy Markdown
Contributor

Started backporting to release/7.0: https://github.com/dotnet/runtime/actions/runs/3266244962

@ghostghost locked as resolved and limited conversation to collaborators Nov 16, 2022
Sign up for freeto subscribe to this conversation on GitHub. Already have an account? Sign in.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@tarekgh@jkotas@janvorli