Skip to content

Fix OpenHarmony's tzdata parsing#1679

Merged
djc merged 1 commit intochronotope:mainfrom
ldm0:ohos_fix
Jul 8, 2025
Merged

Fix OpenHarmony's tzdata parsing#1679
djc merged 1 commit intochronotope:mainfrom
ldm0:ohos_fix

Conversation

@ldm0
Copy link
Contributor

@ldm0 ldm0 commented Mar 20, 2025

OpenHarmony's tzdata parsing supported was introduced in #1613.

OpenHarmony's tzdata shares the same characters with the ones on Android, except that each timezone index is 4 byte's smaller(the legacy rawUtcOffset was removed).

OpenHarmony's tzdata
Android's tzdata

As their parsing logic are pretty similar, I unified their logic into src/offset/local/tzdata.rs(I actually created a crate for that, but the logic is simple enough to put into chrono). This also allows us to remove the android_tzdata dependency.

BTW, OpenHarmony's tzdata parsing logic was also added in fallback_timezone, that's because OpenHarmony doesn't have /etc/localtime, people need to use it's time service api to get the timezone name. I created a PR for iana-time-zone to support OpenHarmony's timezone name fetching: strawlab/iana-time-zone#150. After getting the timezone name from iana_time_zone::get_timezone, chrono need to find it's timezone information in the tzdata.

@codecov
Copy link

codecov bot commented Mar 20, 2025

Codecov Report

All modified and coverable lines are covered by tests ✅

Project coverage is 90.68%. Comparing base (9245a8f) to head (4ddf8c8).

Additional details and impacted files
@@            Coverage Diff             @@
##             main    #1679      +/-   ##
==========================================
+ Coverage   90.61%   90.68%   +0.07%     
==========================================
  Files          38       39       +1     
  Lines       16055    16180     +125     
==========================================
+ Hits        14548    14673     +125     
  Misses       1507     1507              

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚀 New features to boost your workflow:
  • ❄️ Test Analytics: Detect flaky tests, report on failures, and find test suite problems.

@djc
Copy link
Member

djc commented Mar 20, 2025

This should come with CI coverage for Android and OpenHarmony. The new parser should be cleaned up, have a look at the rustls style guide (which I wrote) and try to simplify (drop the OS constructor, make the constant names more concise, avoid nested types/functions).

@ldm0
Copy link
Contributor Author

ldm0 commented Mar 21, 2025

@djc Hi, all new lines are covered now, and parser was cleaned up. Please take a look.

@ldm0
Copy link
Contributor Author

ldm0 commented May 27, 2025

ping @djc plz take a look

Copy link
Member

@djc djc left a comment

Choose a reason for hiding this comment

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

Okay, thanks for working on this.

Can you rebase it and squash your commits, please?

@ldm0
Copy link
Contributor Author

ldm0 commented Jul 8, 2025

@djc done

@djc djc merged commit 23c132f into chronotope:main Jul 8, 2025
35 checks passed
@ldm0 ldm0 deleted the ohos_fix branch July 8, 2025 15:02
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants