tz: change arrangement of internal time zone databases

Jiff has three different kinds of databases that it can draw time zone
transitions from: the standard Unix zoneinfo database, a bundled or
embedded zoneinfo database (compiled into the binary), or the special
Android concatenated zoneinfo database. Previously, we would try to
create as many of these databases as possible, and then look all time
zones up in each.

I think that this type of semantic is very messy, because you can wind
up drawing from one db for one time zone and another db for another
(although in theory this shouldn't happen if they're all in sync). It
also requires that you always look for all three, which feels wrong.

Instead, we now just look for one and stop when we find it. Effectively,
we changed the internals from a product to a sum.

On Android, we check for a concatenated tzdb first, since that's likely
what we'll find.

Fixes #213

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

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)

src/tz/db/mod.rs

@@ -185,16 +185,16 @@ pub fn db() -> &'static TimeZoneDatabase {
 /// ```
 #[derive(Clone)]
 pub struct TimeZoneDatabase {
-    inner: Option<Arc<TimeZoneDatabaseInner>>,
+    inner: Option<Arc<Kind>>,
 }
 
 #[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<TimeZoneDatabase, Error> {
         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<TimeZoneDatabase, Error> {
         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, ")")
     }
 }
 

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