Skip to content

uefi: improve handling of Time#1896

Closed
phip1611 wants to merge 11 commits intomainfrom
uefi-raw-time
Closed

uefi: improve handling of Time#1896
phip1611 wants to merge 11 commits intomainfrom
uefi-raw-time

Conversation

@phip1611
Copy link
Copy Markdown
Member

@phip1611 phip1611 commented Feb 9, 2026

This is a larger PR improving the handling of struct Time from uefi-raw. On a very high level, we can convert a UEFI time to UNIX UTC Timestamps in nanoseconds and also create it from such a timestamp.

This allows:

  • integration into time libraries on higher-levels
  • PartialOrd
  • elapsed_since(&self, future: &Self) -> Option<Duration> {}

Hints for Reviewers

  • It would make sense to only discuss the higher-level approach here and taking a coarse-grained look onto the commit history
  • I will then split this into more reviewable commits

Checklist

  • Sensible git history (for example, squash "typo" or "fix" commits). See the Rewriting History guide for help.
  • Update the changelog (if necessary)

@phip1611 phip1611 self-assigned this Feb 9, 2026
Comment thread uefi-raw/src/time.rs
month += 1;
}

let day = (days + 1) as u8;
Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

todo self.validate?

@nicholasbishop
Copy link
Copy Markdown
Member

Since handling time correctly is complicated and nuanced, I wonder if we should just delegate to one of the existing popular time libraries. I'm not particularly confident in my ability to correctly identify bugs in time handling, so I'd rather limit our code to simple conversions if possible.

Currently I think the main ecosystem time libraries are time and jiff. (chrono is soft deprecated, and they recommend using jiff.)

So, rather than implementing any time logic ourselves, we could do something like add an optional dep on jiff in uefi-raw, and if that feature is enabled, implement TryFrom between uefi_raw::Time and types like jiff::Zoned, jiff::civil::Time, etc. Potentially we could add two optional deps and allow conversions with both jiff and time types.

In uefi, we could either also add an optional feature to enable jiff in uefi-raw, or we could decide to be more opinionated and always enable the feature in uefi so that we can rely on it internally.

@phip1611
Copy link
Copy Markdown
Member Author

phip1611 commented Feb 17, 2026

Since handling time correctly is complicated and nuanced, I wonder if we should just delegate to one of the existing popular time libraries.

I agree. I see the proposed to_utc_unix_timestamp_nanos() and from_utc_unix_timestamp_nanos() as the necessary preliminary step and glue to add a dependency to time or any other time library in a follow-up.

@phip1611
Copy link
Copy Markdown
Member Author

Since handling time correctly is complicated and nuanced, I wonder if we should just delegate to one of the existing popular time libraries.

I agree. I see the proposed to_utc_unix_timestamp_nanos() and from_utc_unix_timestamp_nanos() as the necessary preliminary step and glue to add a dependency to time or any other time library in a follow-up.

I could replace this with a clearly better implementation: #1899

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