SerenityOS/serenity

AK/Time: Get rid of `UnixDateTime::from_unix_time_parts`

Open

#19,030 opened on 2023年5月24日

GitHub で見る
 (0 comments) (0 reactions) (0 assignees)C++ (33,171 stars) (3,328 forks)batch import
help wantedrefactoring

説明

This API has multiple naivety issues, here are just the "big" ones I can remember off the top of my head:

  • Unix time is always Gregorian even before that calendar was really introduced, so we don't have to observe the Julian/Gregorian calendar discontinuity in 1500-something.
  • Unix time doesn't know that year 0 doesn't exist, so all negative years are shifted by 1
  • Unix time doesn't know that leap-year rules change before the Gregorian/Julian change

A good "from time parts" API must always take calendars into account accurately, and at that point, you also probably want to work with (a) a timezone and (b) a non-naive time object, i.e. not UnixDateTime. It would clearly be a good idea to remove this API altogether, but what about its replacement?

For reference, here are all users of the API and some ideas of how they could be fixed:

  • DOSPackedTime uses the API for conversion into Serenity's time formats. This API will likely require us to take a good look at the actual time format specification of DOS time as used in e.g. FAT file systems.
  • The Kernel uses the API for conversion from ISO 9660 (CD-ROM) (@sin-ack ) file system timestamps. The usage is already marked with a FIXME since it ignores timezone information, so this usage should also be a separate spec-aligned conversion method.
  • Time zone data itself is generated which uses this API. We have to consider where we get the partwise time data in this instance and whether it's a better idea to convert it at compile time, even things like year information for the daylight savings database. (@trflynn89 )
  • JS ISO 8601 parsing uses this API, which should rather be using a more aware time API such as UTC zoned time. I know that everything around Date is horrible and underspecified, and I hope that Temporal doesn't call into the ISO 8601 parser, but we might want a custom parser here as well, depending on what calendar definition ECMA-262 uses here.
  • JS Intl has exactly one use of this API; it could probably instead use something from Temporal or something more aware?
  • Cookie parsing; I hope there's a spec for that as well.
  • touch uses this API for user-defined mtime/atime, we should definitely use aware APIs here and convert things to timestamps at the end for passing to the Kernel.

Please leave further suggestions below, there are a couple of tricky things to hash out which I have little knowledge about. They can be tackled individually and many will require more aware date time types.

コントリビューターガイド