diff --git a/CHANGELOG.md b/CHANGELOG.md index 343bd51..2fcc141 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -34,6 +34,14 @@ the future is serialized before a change in the daylight saving time rules. For more examples, see `jiff::fmt::temporal::DateTimeParser::offset_conflict` for details on how to change Jiff's default behavior. This behavior change also applies to `tz::OffsetConflict::PreferOffset`. +* [#213](https://github.com/BurntSushi/jiff/issues/213): +Tweak the semantics of `tz::TimeZoneDatabase` so that it only initializes one +internal tzdb instead of trying to find as many as possible. It is unlikely +that you'll be impacted by this change, but it's meant to make the semantics +be a bit more sensible. (In `jiff 0.1`, it was in theory possible for one tz +lookup to succeed in the system zoneinfo and then another tz lookup to fail +in zoneinfo but succeed automatically via the bundled copy. But this seems +confusing and possibly not desirable. Hence the change.) * [#218](https://github.com/BurntSushi/jiff/issues/218): In order to make naming a little more consistent between `Zoned` and `civil::Date`, the `civil::Date::to_iso_week_date` and diff --git a/src/fmt/rfc9557.rs b/src/fmt/rfc9557.rs index 817992f..cbe529c 100644 --- a/src/fmt/rfc9557.rs +++ b/src/fmt/rfc9557.rs @@ -1026,6 +1026,12 @@ mod tests { #[cfg(feature = "std")] #[test] fn err_time_zone_db_lookup() { + // The error message snapshotted below can vary based on tzdb + // config, so only run this when we know we've got a real tzdb. + if crate::tz::db().is_definitively_empty() { + return; + } + let p = |input| { Parser::new() .parse(input) diff --git a/src/tz/db/mod.rs b/src/tz/db/mod.rs index 23f313c..ea40c72 100644 --- a/src/tz/db/mod.rs +++ b/src/tz/db/mod.rs @@ -185,16 +185,16 @@ pub fn db() -> &'static TimeZoneDatabase { /// ``` #[derive(Clone)] pub struct TimeZoneDatabase { - inner: Option>, + inner: Option>, } #[derive(Debug)] // Needed for core-only "dumb" `Arc`. #[cfg_attr(not(feature = "alloc"), derive(Clone))] -struct TimeZoneDatabaseInner { - zoneinfo: ZoneInfo, - concatenated: Concatenated, - bundled: BundledZoneInfo, +enum Kind { + ZoneInfo(ZoneInfo), + Concatenated(Concatenated), + Bundled(BundledZoneInfo), } impl TimeZoneDatabase { @@ -245,18 +245,46 @@ impl TimeZoneDatabase { /// return an embedded copy of the Time Zone Database since Windows does /// not have a canonical installation of the Time Zone Database. pub fn from_env() -> TimeZoneDatabase { - let zoneinfo = ZoneInfo::from_env(); - let concatenated = Concatenated::from_env(); - let bundled = BundledZoneInfo::new(); - let inner = TimeZoneDatabaseInner { zoneinfo, concatenated, bundled }; - let db = TimeZoneDatabase { inner: Some(Arc::new(inner)) }; - if db.is_definitively_empty() { - warn!( - "could not find zoneinfo, concatenated tzdata or \ - bundled time zone database", - ); + // On Android, try the concatenated database first, since that's + // typically what is used. + // + // Overall this logic might be sub-optimal. Like, does it really make + // sense to check for the zoneinfo or concatenated database on non-Unix + // platforms? Probably not to be honest. But these should only be + // executed ~once generally, so it doesn't seem like a big deal to try. + // And trying makes things a little more flexible I think. + if cfg!(target_os = "android") { + let db = Concatenated::from_env(); + if !db.is_definitively_empty() { + return TimeZoneDatabase::new(Kind::Concatenated(db)); + } + + let db = ZoneInfo::from_env(); + if !db.is_definitively_empty() { + return TimeZoneDatabase::new(Kind::ZoneInfo(db)); + } + } else { + let db = ZoneInfo::from_env(); + if !db.is_definitively_empty() { + return TimeZoneDatabase::new(Kind::ZoneInfo(db)); + } + + let db = Concatenated::from_env(); + if !db.is_definitively_empty() { + return TimeZoneDatabase::new(Kind::Concatenated(db)); + } + } + + let db = BundledZoneInfo::new(); + if !db.is_definitively_empty() { + return TimeZoneDatabase::new(Kind::Bundled(db)); } - db + + warn!( + "could not find zoneinfo, concatenated tzdata or \ + bundled time zone database", + ); + TimeZoneDatabase::none() } /// Returns a time zone database initialized from the given directory. @@ -279,19 +307,14 @@ impl TimeZoneDatabase { path: P, ) -> Result { let path = path.as_ref(); - let zoneinfo = ZoneInfo::from_dir(path)?; - let concatenated = Concatenated::none(); - let bundled = BundledZoneInfo::new(); - let inner = TimeZoneDatabaseInner { zoneinfo, concatenated, bundled }; - let db = TimeZoneDatabase { inner: Some(Arc::new(inner)) }; + let db = ZoneInfo::from_dir(path)?; if db.is_definitively_empty() { warn!( - "could not find zoneinfo data at directory {path} \ - (and there is no bundled time zone database)", + "could not find zoneinfo data at directory {path}", path = path.display(), ); } - Ok(db) + Ok(TimeZoneDatabase::new(Kind::ZoneInfo(db))) } /// Returns a time zone database initialized from a path pointing to a @@ -322,19 +345,43 @@ impl TimeZoneDatabase { path: P, ) -> Result { let path = path.as_ref(); - let zoneinfo = ZoneInfo::none(); - let concatenated = Concatenated::from_path(path)?; - let bundled = BundledZoneInfo::new(); - let inner = TimeZoneDatabaseInner { zoneinfo, concatenated, bundled }; - let db = TimeZoneDatabase { inner: Some(Arc::new(inner)) }; + let db = Concatenated::from_path(path)?; if db.is_definitively_empty() { warn!( - "could not find concatenated tzdata in file {path} \ - (and there is no bundled time zone database)", + "could not find concatenated tzdata in file {path}", path = path.display(), ); } - Ok(db) + Ok(TimeZoneDatabase::new(Kind::Concatenated(db))) + } + + /// Returns a time zone database initialized from the bundled copy of + /// the [IANA Time Zone Database]. + /// + /// While this API is always available, in order to get a non-empty + /// database back, this requires that one of the crate features + /// `tzdb-bundle-always` or `tzdb-bundle-platform` is enabled. In the + /// latter case, the bundled database is only available on platforms known + /// to lack a system copy of the IANA Time Zone Database (i.e., non-Unix + /// systems). + /// + /// This routine is infallible, but it may return a database + /// that is definitively empty if the bundled data is not + /// available. To query whether the data is empty or not, use + /// [`TimeZoneDatabase::is_definitively_empty`]. + /// + /// [IANA Time Zone Database]: https://en.wikipedia.org/wiki/Tz_database + pub fn bundled() -> TimeZoneDatabase { + let db = BundledZoneInfo::new(); + if db.is_definitively_empty() { + warn!("could not find embedded/bundled zoneinfo"); + } + TimeZoneDatabase::new(Kind::Bundled(db)) + } + + /// Creates a new DB from the internal kind. + fn new(kind: Kind) -> TimeZoneDatabase { + TimeZoneDatabase { inner: Some(Arc::new(kind)) } } /// Returns a [`TimeZone`] corresponding to the IANA time zone identifier @@ -372,17 +419,25 @@ impl TimeZoneDatabase { ) } })?; - if let Some(tz) = inner.zoneinfo.get(name) { - trace!("found time zone `{name}` in {:?}", inner.zoneinfo); - return Ok(tz); - } - if let Some(tz) = inner.concatenated.get(name) { - trace!("found time zone `{name}` in {:?}", inner.concatenated); - return Ok(tz); - } - if let Some(tz) = inner.bundled.get(name) { - trace!("found time zone `{name}` in {:?}", inner.bundled); - return Ok(tz); + match *inner { + Kind::ZoneInfo(ref db) => { + if let Some(tz) = db.get(name) { + trace!("found time zone `{name}` in {db:?}", db = self); + return Ok(tz); + } + } + Kind::Concatenated(ref db) => { + if let Some(tz) = db.get(name) { + trace!("found time zone `{name}` in {db:?}", db = self); + return Ok(tz); + } + } + Kind::Bundled(ref db) => { + if let Some(tz) = db.get(name) { + trace!("found time zone `{name}` in {db:?}", db = self); + return Ok(tz); + } + } } Err(err!("failed to find time zone `{name}` in time zone database")) } @@ -407,16 +462,16 @@ impl TimeZoneDatabase { /// ``` #[cfg(feature = "alloc")] pub fn available(&self) -> TimeZoneNameIter { - let Some(ref inner) = self.inner else { + let Some(inner) = self.inner.as_deref() else { return TimeZoneNameIter { it: alloc::vec::Vec::new().into_iter(), }; }; - let mut all = inner.zoneinfo.available(); - all.extend(inner.concatenated.available()); - all.extend(inner.bundled.available()); - all.sort(); - all.dedup(); + let all = match *inner { + Kind::ZoneInfo(ref db) => db.available(), + Kind::Concatenated(ref db) => db.available(), + Kind::Bundled(ref db) => db.available(), + }; TimeZoneNameIter { it: all.into_iter() } } @@ -431,9 +486,11 @@ impl TimeZoneDatabase { /// invalidation heuristics to kick in. pub fn reset(&self) { let Some(inner) = self.inner.as_deref() else { return }; - inner.zoneinfo.reset(); - inner.concatenated.reset(); - inner.bundled.reset(); + match *inner { + Kind::ZoneInfo(ref db) => db.reset(), + Kind::Concatenated(ref db) => db.reset(), + Kind::Bundled(ref db) => db.reset(), + } } /// Returns true if it is known that this time zone database is empty. @@ -455,9 +512,11 @@ impl TimeZoneDatabase { /// ``` pub fn is_definitively_empty(&self) -> bool { let Some(inner) = self.inner.as_deref() else { return true }; - inner.zoneinfo.is_definitively_empty() - && inner.concatenated.is_definitively_empty() - && inner.bundled.is_definitively_empty() + match *inner { + Kind::ZoneInfo(ref db) => db.is_definitively_empty(), + Kind::Concatenated(ref db) => db.is_definitively_empty(), + Kind::Bundled(ref db) => db.is_definitively_empty(), + } } } @@ -467,12 +526,12 @@ impl core::fmt::Debug for TimeZoneDatabase { let Some(inner) = self.inner.as_deref() else { return write!(f, "unavailable)"); }; - write!( - f, - "{:?}, {:?}, {:?}", - inner.zoneinfo, inner.concatenated, inner.bundled - )?; - Ok(()) + match *inner { + Kind::ZoneInfo(ref db) => write!(f, "{db:?}")?, + Kind::Concatenated(ref db) => write!(f, "{db:?}")?, + Kind::Bundled(ref db) => write!(f, "{db:?}")?, + } + write!(f, ")") } } diff --git a/src/tz/mod.rs b/src/tz/mod.rs index be07904..7a641f7 100644 --- a/src/tz/mod.rs +++ b/src/tz/mod.rs @@ -210,6 +210,14 @@ mod zic; /// `TimeZone` from a `TimeZoneDatabase`. (Or, in the case of the system time /// zone, by calling `TimeZone::system`.) /// +/// # A `TimeZone` is cheap to clone +/// +/// A `TimeZone` can be cheaply cloned. It uses automic reference counting +/// internally. When `alloc` is disabled, cloning a `TimeZone` is still cheap +/// because POSIX time zones and TZif time zones are unsupported. Therefore, +/// cloning a time zone does a deep copy (since automic reference counting is +/// not available), but the data being copied is small. +/// /// # Time zone equality /// /// `TimeZone` provides an imperfect notion of equality. That is, when two time