SerenityOS/serenity

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

Open

#19.030 aperta il 24 mag 2023

Vedi su GitHub
 (0 commenti) (0 reazioni) (0 assegnatari)C++ (3328 fork)batch import
help wantedrefactoring

Metriche repository

Star
 (33.171 star)
Metriche merge PR
 (Merge medio 5g 11h) (35 PR mergiate in 30 g)

Descrizione

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.

Guida contributor