[Android] Filter legacy time zones without unversioned tzlookup.xml - #132401
Draft
davidnguyen-tech wants to merge 1 commit into
Draft
[Android] Filter legacy time zones without unversioned tzlookup.xml#132401davidnguyen-tech wants to merge 1 commit into
davidnguyen-tech wants to merge 1 commit into
Conversation
Prefer Android's versioned tzlookup metadata and fall back to ICU canonical location IDs when that metadata is unavailable. Co-authored-by: Copilot App <223556219+Copilot@users.noreply.github.com> Copilot-Session: 4da39873-ad89-41cf-a5dd-9753c3ab7f91
|
Azure Pipelines: Successfully started running 3 pipeline(s). 13 pipeline(s) were filtered out due to trigger conditions. There may be pipelines that require an authorized user to comment /azp run to run. |
Contributor
There was a problem hiding this comment.
Pull request overview
This PR updates Android time zone enumeration so TimeZoneInfo.GetSystemTimeZones() can filter legacy/backward aliases more reliably by preferring Android’s versioned tzdata directories when present and falling back when tzlookup.xml isn’t available, while re-enabling related regression tests.
Changes:
- Prefer the APEX versioned tzdata directory (
etc/tz/versioned/<format-major>) when available, with fallback directory probing. - Add an ICU-based native/managed fallback path to obtain canonical-location time zone IDs when
tzlookup.xmlis missing/unreadable. - Re-enable Android time zone regression tests by removing
ActiveIssuegating.
Reviewed changes
Copilot reviewed 7 out of 7 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| src/native/libs/System.Globalization.Native/pal_timeZoneInfo.h | Declares a new native export for enumerating canonical-location zone IDs. |
| src/native/libs/System.Globalization.Native/pal_timeZoneInfo.c | Implements ICU-backed enumeration of canonical-location time zone IDs. |
| src/native/libs/System.Globalization.Native/pal_placeholders.c | Adds a placeholder stub for the new native export on unsupported platforms. |
| src/native/libs/System.Globalization.Native/entrypoints.c | Registers the new export in the globalization native entrypoint table (non-wasm). |
| src/libraries/Common/src/Interop/Interop.TimeZoneInfo.cs | Adds LibraryImport for the new native canonical-location IDs export. |
| src/libraries/System.Private.CoreLib/src/System/TimeZoneInfo.Unix.Android.cs | Uses versioned tzdata directory when available and falls back to ICU canonical-location IDs when tzlookup.xml is missing/unreadable. |
| src/libraries/System.Runtime/tests/System.Runtime.Tests/System/TimeZoneInfoTests.cs | Re-enables Android time zone regression tests by removing ActiveIssue exclusions. |
Comment on lines
+395
to
+415
| char[] buffer = new char[bufferLength]; | ||
| if (Interop.Globalization.GetCanonicalLocationTimeZoneIds(buffer, bufferLength) != bufferLength) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| HashSet<string> ids = new HashSet<string>(); | ||
| int index = 0; | ||
| while (index < bufferLength) | ||
| { | ||
| int idLength = buffer[index++]; | ||
| if (idLength == 0 || idLength > bufferLength - index) | ||
| { | ||
| return null; | ||
| } | ||
|
|
||
| ids.Add(new string(buffer, index, idLength)); | ||
| index += idLength; | ||
| } | ||
|
|
||
| return ids; |
Comment on lines
+251
to
+256
| int32_t GlobalizationNative_GetCanonicalLocationTimeZoneIds( | ||
| UChar* value, int32_t valueLength) | ||
| { | ||
| assert_msg(false, "Not supported on this platform", 0); | ||
| return 0; | ||
| } |
Member
Author
|
/azp run runtime-extra-platforms Note This pipeline trigger was posted by GitHub Copilot at the user's request. |
|
No pipelines are associated with this pull request. |
tarekgh
added
os-android
area-System.DateTime
and removed
area-System.Globalization
labels
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.
Summary
tzlookup.xmlmetadatatzlookup.xmlis missing or unreadableThis removes legacy aliases from
TimeZoneInfo.GetSystemTimeZones()without changing direct lookup throughFindSystemTimeZoneById.Fixes #90269
Fixes #117731
Validation
System.Runtime.Testson a physical Android 16 / API 36 arm64 device: 77,575 run, 0 failedNote
This pull request was prepared with GitHub Copilot.