From 28ac6572722f7ea31795dc0417521c70bcb6ec8f Mon Sep 17 00:00:00 2001 From: Andrew Gallant Date: Sat, 27 Jul 2024 09:33:11 -0400 Subject: [PATCH] gix-{archive,date}: switch from `time` to `jiff` This swaps out `time` for `jiff`. It doesn't completely remove `time` from the dependency tree. The last remaining use of `time` is in `prodash`, outside of the gitoxide project. --- Cargo.lock | 35 ++++++++++-- Cargo.toml | 2 - gix-archive/Cargo.toml | 6 +- gix-archive/src/write.rs | 18 +++++- gix-date/Cargo.toml | 2 +- gix-date/src/parse.rs | 103 ++++++++++++++++++++++------------- gix-date/src/time/format.rs | 50 ++++------------- gix-date/src/time/init.rs | 28 ++-------- gix-date/src/time/mod.rs | 2 +- gix-date/tests/time/parse.rs | 19 +++++-- src/plumbing/main.rs | 5 -- src/porcelain/main.rs | 5 -- 12 files changed, 144 insertions(+), 131 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 4cf9cfc1a95..8441cbda888 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1240,7 +1240,6 @@ dependencies = [ "prodash", "serde_derive", "terminal_size", - "time", "tracing", "tracing-forest", "tracing-subscriber", @@ -1401,9 +1400,9 @@ dependencies = [ "gix-testtools", "gix-worktree 0.34.1", "gix-worktree-stream", + "jiff", "tar", "thiserror", - "time", "zip", ] @@ -1615,10 +1614,10 @@ dependencies = [ "gix-hash 0.14.2", "gix-testtools", "itoa", + "jiff", "once_cell", "serde", "thiserror", - "time", ] [[package]] @@ -3077,9 +3076,9 @@ dependencies = [ [[package]] name = "indoc" -version = "2.0.4" +version = "2.0.5" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "1e186cfbae8084e513daff4240b4797e342f988cecda4fb6c939150f96315fd8" +checksum = "b248f5224d1d606005e02c97f5aa4e88eeb230488bcc03bc9ca4d7991399f2b5" [[package]] name = "instant" @@ -3183,6 +3182,31 @@ version = "1.0.10" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "b1a46d1a171d865aa5f83f92695765caa047a9b4cbae2cbf37dbd613a793fd4c" +[[package]] +name = "jiff" +version = "0.1.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "0c33f2189d126c374d29641af39a0dc73daa1c8292403645575ce32e22e768a4" +dependencies = [ + "jiff-tzdb-platform", + "windows-sys 0.52.0", +] + +[[package]] +name = "jiff-tzdb" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "05fac328b3df1c0f18a3c2ab6cb7e06e4e549f366017d796e3e66b6d6889abe6" + +[[package]] +name = "jiff-tzdb-platform" +version = "0.1.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f8da387d5feaf355954c2c122c194d6df9c57d865125a67984bb453db5336940" +dependencies = [ + "jiff-tzdb", +] + [[package]] name = "js-sys" version = "0.3.66" @@ -5499,7 +5523,6 @@ dependencies = [ "indexmap", "memchr", "thiserror", - "time", "zopfli", ] diff --git a/Cargo.toml b/Cargo.toml index 4dc1037437a..fb25223a704 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -164,14 +164,12 @@ http-client-reqwest = ["gix/blocking-http-transport-reqwest-rust-tls"] ## Use async client networking. gitoxide-core-async-client = ["gitoxide-core/async-client", "futures-lite"] - [dependencies] anyhow = "1.0.42" gitoxide-core = { version = "^0.39.1", path = "gitoxide-core" } gix-features = { version = "^0.38.2", path = "gix-features" } gix = { version = "^0.64.0", path = "gix", default-features = false } -time = "0.3.23" clap = { version = "4.1.1", features = ["derive", "cargo"] } clap_complete = "4.4.3" diff --git a/gix-archive/Cargo.toml b/gix-archive/Cargo.toml index e932caf1717..84c6329ee15 100644 --- a/gix-archive/Cargo.toml +++ b/gix-archive/Cargo.toml @@ -21,7 +21,7 @@ tar = ["dep:tar", "dep:gix-path"] tar_gz = ["tar", "dep:flate2"] ## Enable the `zip` archive format. -zip = ["dep:zip", "dep:time"] +zip = ["dep:zip"] [dependencies] @@ -31,8 +31,8 @@ gix-path = { version = "^0.10.9", path = "../gix-path", optional = true } gix-date = { version = "^0.8.7", path = "../gix-date" } flate2 = { version = "1.0.26", optional = true } -zip = { version = "2.1.0", optional = true, default-features = false, features = ["deflate", "time"] } -time = { version = "0.3.23", optional = true, default-features = false, features = ["std"] } +zip = { version = "2.1.0", optional = true, default-features = false, features = ["deflate"] } +jiff = { version = "0.1.2", default-features = false, features = ["std"] } thiserror = "1.0.26" bstr = { version = "1.5.0", default-features = false } diff --git a/gix-archive/src/write.rs b/gix-archive/src/write.rs index 244b40554e0..0c6fd435f25 100644 --- a/gix-archive/src/write.rs +++ b/gix-archive/src/write.rs @@ -134,10 +134,22 @@ where { let mut ar = zip::write::ZipWriter::new(out); let mut buf = Vec::new(); - let mtime = time::OffsetDateTime::from_unix_timestamp(opts.modification_time) + let zdt = jiff::Timestamp::from_second(opts.modification_time) .map_err(|err| Error::InvalidModificationTime(Box::new(err)))? - .try_into() - .map_err(|err| Error::InvalidModificationTime(Box::new(err)))?; + .to_zoned(jiff::tz::TimeZone::UTC); + let mtime = zip::DateTime::from_date_and_time( + zdt.year() + .try_into() + .map_err(|err| Error::InvalidModificationTime(Box::new(err)))?, + // These are all OK because month, day, hour, minute and second + // are always positive. + zdt.month().try_into().expect("non-negative"), + zdt.day().try_into().expect("non-negative"), + zdt.hour().try_into().expect("non-negative"), + zdt.minute().try_into().expect("non-negative"), + zdt.second().try_into().expect("non-negative"), + ) + .map_err(|err| Error::InvalidModificationTime(Box::new(err)))?; while let Some(entry) = next_entry(stream)? { append_zip_entry( &mut ar, diff --git a/gix-date/Cargo.toml b/gix-date/Cargo.toml index 48795e8a246..d20a93b439a 100644 --- a/gix-date/Cargo.toml +++ b/gix-date/Cargo.toml @@ -20,7 +20,7 @@ serde= ["dep:serde", "bstr/serde"] bstr = { version = "1.3.0", default-features = false, features = ["std"]} serde = { version = "1.0.114", optional = true, default-features = false, features = ["derive"]} itoa = "1.0.1" -time = { version = "0.3.23", default-features = false, features = ["local-offset", "formatting", "macros", "parsing"] } +jiff = "0.1.1" thiserror = "1.0.32" document-features = { version = "0.2.0", optional = true } diff --git a/gix-date/src/parse.rs b/gix-date/src/parse.rs index 1f79076174f..449eea0ac87 100644 --- a/gix-date/src/parse.rs +++ b/gix-date/src/parse.rs @@ -14,7 +14,7 @@ pub enum Error { pub(crate) mod function { use std::{str::FromStr, time::SystemTime}; - use time::{format_description::well_known, Date, OffsetDateTime}; + use jiff::{civil::Date, fmt::rfc2822, tz::TimeZone, Zoned}; use crate::{ parse::{relative, Error}, @@ -32,27 +32,27 @@ pub(crate) mod function { return Ok(Time::new(42, 1800)); } - Ok(if let Ok(val) = Date::parse(input, SHORT.0) { - let val = val.with_hms(0, 0, 0).expect("date is in range").assume_utc(); - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) - } else if let Ok(val) = OffsetDateTime::parse(input, &well_known::Rfc2822) { - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) - } else if let Ok(val) = OffsetDateTime::parse(input, ISO8601.0) { - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) - } else if let Ok(val) = OffsetDateTime::parse(input, ISO8601_STRICT.0) { - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) - } else if let Ok(val) = OffsetDateTime::parse(input, GITOXIDE.0) { - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) - } else if let Ok(val) = OffsetDateTime::parse(input, DEFAULT.0) { - Time::new(val.unix_timestamp(), val.offset().whole_seconds()) + Ok(if let Ok(val) = Date::strptime(SHORT.0, input) { + let val = val.to_zoned(TimeZone::UTC).expect("date is in range"); + Time::new(val.timestamp().as_second(), val.offset().seconds()) + } else if let Ok(val) = rfc2822_relaxed(input) { + Time::new(val.timestamp().as_second(), val.offset().seconds()) + } else if let Ok(val) = strptime_relaxed(ISO8601.0, input) { + Time::new(val.timestamp().as_second(), val.offset().seconds()) + } else if let Ok(val) = strptime_relaxed(ISO8601_STRICT.0, input) { + Time::new(val.timestamp().as_second(), val.offset().seconds()) + } else if let Ok(val) = strptime_relaxed(GITOXIDE.0, input) { + Time::new(val.timestamp().as_second(), val.offset().seconds()) + } else if let Ok(val) = strptime_relaxed(DEFAULT.0, input) { + Time::new(val.timestamp().as_second(), val.offset().seconds()) } else if let Ok(val) = SecondsSinceUnixEpoch::from_str(input) { // Format::Unix Time::new(val, 0) } else if let Some(val) = parse_raw(input) { // Format::Raw val - } else if let Some(time) = relative::parse(input, now).transpose()? { - Time::new(time.unix_timestamp(), time.offset().whole_seconds()) + } else if let Some(val) = relative::parse(input, now).transpose()? { + Time::new(val.timestamp().as_second(), val.offset().seconds()) } else { return Err(Error::InvalidDateString { input: input.into() }); }) @@ -83,52 +83,79 @@ pub(crate) mod function { }; Some(time) } + + /// This is just like `Zoned::strptime`, but it allows parsing datetimes + /// whose weekdays are inconsistent with the date. While the day-of-week + /// still must be parsed, it is otherwise ignored. This seems to be + /// consistent with how `git` behaves. + fn strptime_relaxed(fmt: &str, input: &str) -> Result { + let mut tm = jiff::fmt::strtime::parse(fmt, input)?; + tm.set_weekday(None); + tm.to_zoned() + } + + /// This is just like strptime_relaxed, except for RFC 2822 parsing. + /// Namely, it permits the weekday to be inconsistent with the date. + fn rfc2822_relaxed(input: &str) -> Result { + static P: rfc2822::DateTimeParser = rfc2822::DateTimeParser::new().relaxed_weekday(true); + P.parse_zoned(input) + } } mod relative { use std::{str::FromStr, time::SystemTime}; - use time::{Duration, OffsetDateTime}; + use jiff::{tz::TimeZone, Span, Timestamp, Zoned}; use crate::parse::Error; - fn parse_inner(input: &str) -> Option { + fn parse_inner(input: &str) -> Option> { let mut split = input.split_whitespace(); - let multiplier = i64::from_str(split.next()?).ok()?; + let units = i64::from_str(split.next()?).ok()?; let period = split.next()?; if split.next()? != "ago" { return None; } - duration(period, multiplier) + span(period, units) } - pub(crate) fn parse(input: &str, now: Option) -> Option> { - parse_inner(input).map(|offset| { - let offset = std::time::Duration::from_secs(offset.whole_seconds().try_into()?); + pub(crate) fn parse(input: &str, now: Option) -> Option> { + parse_inner(input).map(|result| { + let span = result?; + // This was an error case in a previous version of this code, where + // it would fail when converting from a negative signed integer + // to an unsigned integer. This preserves that failure case even + // though the code below handles it okay. + if span.is_negative() { + return Err(Error::RelativeTimeConversion); + } now.ok_or(Error::MissingCurrentTime).and_then(|now| { - std::panic::catch_unwind(|| { - now.checked_sub(offset) - .expect("BUG: values can't be large enough to cause underflow") - .into() - }) - .map_err(|_| Error::RelativeTimeConversion) + let ts = Timestamp::try_from(now).map_err(|_| Error::RelativeTimeConversion)?; + // N.B. This matches the behavior of this code when it was + // written with `time`, but we might consider using the system + // time zone here. If we did, then it would implement "1 day + // ago" correctly, even when it crosses DST transitions. Since + // we're in the UTC time zone here, which has no DST, 1 day is + // in practice always 24 hours. ---AG + let zdt = ts.to_zoned(TimeZone::UTC); + zdt.checked_sub(span).map_err(|_| Error::RelativeTimeConversion) }) }) } - fn duration(period: &str, multiplier: i64) -> Option { + fn span(period: &str, units: i64) -> Option> { let period = period.strip_suffix('s').unwrap_or(period); - let seconds: i64 = match period { - "second" => 1, - "minute" => 60, - "hour" => 60 * 60, - "day" => 24 * 60 * 60, - "week" => 7 * 24 * 60 * 60, + let result = match period { + "second" => Span::new().try_seconds(units), + "minute" => Span::new().try_minutes(units), + "hour" => Span::new().try_hours(units), + "day" => Span::new().try_days(units), + "week" => Span::new().try_weeks(units), // TODO months & years? YES // Ignore values you don't know, assume seconds then (so does git) _ => return None, }; - seconds.checked_mul(multiplier).map(Duration::seconds) + Some(result.map_err(|_| Error::RelativeTimeConversion)) } #[cfg(test)] @@ -137,7 +164,7 @@ mod relative { #[test] fn two_weeks_ago() { - assert_eq!(parse_inner("2 weeks ago"), Some(Duration::weeks(2))); + assert_eq!(parse_inner("2 weeks ago").unwrap().unwrap(), Span::new().weeks(2)); } } } diff --git a/gix-date/src/time/format.rs b/gix-date/src/time/format.rs index d0ad58f0d97..bfe5583ec93 100644 --- a/gix-date/src/time/format.rs +++ b/gix-date/src/time/format.rs @@ -1,37 +1,22 @@ -use time::macros::format_description; - use crate::{ time::{CustomFormat, Format}, Time, }; /// E.g. `2018-12-24` -pub const SHORT: CustomFormat = CustomFormat(format_description!("[year]-[month]-[day]")); +pub const SHORT: CustomFormat = CustomFormat("%Y-%m-%d"); /// E.g. `Thu, 18 Aug 2022 12:45:06 +0800` -pub const RFC2822: CustomFormat = CustomFormat(format_description!( - "[weekday repr:short], [day] [month repr:short] [year] [hour]:[minute]:[second] [offset_hour sign:mandatory][offset_minute]" -)); +pub const RFC2822: CustomFormat = CustomFormat("%a, %d %b %Y %H:%M:%S %z"); /// E.g. `Thu, 8 Aug 2022 12:45:06 +0800`. This is output by `git log --pretty=%aD`. -pub const GIT_RFC2822: CustomFormat = CustomFormat(format_description!( - "[weekday repr:short], \ - [day padding:none] \ - [month repr:short] \ - [year] \ - [hour]:[minute]:[second] \ - [offset_hour sign:mandatory][offset_minute]" -)); +pub const GIT_RFC2822: CustomFormat = CustomFormat("%a, %-d %b %Y %H:%M:%S %z"); /// E.g. `2022-08-17 22:04:58 +0200` -pub const ISO8601: CustomFormat = CustomFormat(format_description!( - "[year]-[month]-[day] [hour]:[minute]:[second] [offset_hour sign:mandatory][offset_minute]" -)); +pub const ISO8601: CustomFormat = CustomFormat("%Y-%m-%d %H:%M:%S %z"); /// E.g. `2022-08-17T21:43:13+08:00` -pub const ISO8601_STRICT: CustomFormat = CustomFormat(format_description!( - "[year]-[month]-[day]T[hour]:[minute]:[second][offset_hour sign:mandatory]:[offset_minute]" -)); +pub const ISO8601_STRICT: CustomFormat = CustomFormat("%Y-%m-%dT%H:%M:%S%:z"); /// E.g. `123456789` pub const UNIX: Format = Format::Unix; @@ -40,19 +25,10 @@ pub const UNIX: Format = Format::Unix; pub const RAW: Format = Format::Raw; /// E.g. `Thu Sep 04 2022 10:45:06 -0400`, like the git `DEFAULT`, but with the year and time fields swapped. -pub const GITOXIDE: CustomFormat = CustomFormat(format_description!( - "[weekday repr:short] [month repr:short] [day] [year] [hour]:[minute]:[second] [offset_hour sign:mandatory][offset_minute]" -)); +pub const GITOXIDE: CustomFormat = CustomFormat("%a %b %d %Y %H:%M:%S %z"); /// E.g. `Thu Sep 4 10:45:06 2022 -0400`. This is output by `git log --pretty=%ad`. -pub const DEFAULT: CustomFormat = CustomFormat(format_description!( - "[weekday repr:short] \ - [month repr:short] \ - [day padding:none] \ - [hour]:[minute]:[second] \ - [year] \ - [offset_hour sign:mandatory][offset_minute]" -)); +pub const DEFAULT: CustomFormat = CustomFormat("%a %b %-d %H:%M:%S %Y %z"); /// Formatting impl Time { @@ -66,10 +42,7 @@ impl Time { fn format_inner(&self, format: Format) -> String { match format { - Format::Custom(CustomFormat(format)) => self - .to_time() - .format(format) - .expect("well-known format into memory never fails"), + Format::Custom(CustomFormat(format)) => self.to_time().strftime(format).to_string(), Format::Unix => self.seconds.to_string(), Format::Raw => self.to_bstring().to_string(), } @@ -77,9 +50,10 @@ impl Time { } impl Time { - fn to_time(self) -> time::OffsetDateTime { - time::OffsetDateTime::from_unix_timestamp(self.seconds) + fn to_time(self) -> jiff::Zoned { + let offset = jiff::tz::Offset::from_seconds(self.offset).expect("valid offset"); + jiff::Timestamp::from_second(self.seconds) .expect("always valid unix time") - .to_offset(time::UtcOffset::from_whole_seconds(self.offset).expect("valid offset")) + .to_zoned(offset.to_time_zone()) } } diff --git a/gix-date/src/time/init.rs b/gix-date/src/time/init.rs index 7df1e7aee45..e7d9adaf62d 100644 --- a/gix-date/src/time/init.rs +++ b/gix-date/src/time/init.rs @@ -1,5 +1,3 @@ -use std::ops::Sub; - use crate::{time::Sign, OffsetInSeconds, SecondsSinceUnixEpoch, Time}; /// Instantiation @@ -15,9 +13,7 @@ impl Time { /// Return the current time without figuring out a timezone offset pub fn now_utc() -> Self { - let seconds = time::OffsetDateTime::now_utc() - .sub(std::time::SystemTime::UNIX_EPOCH) - .whole_seconds(); + let seconds = jiff::Timestamp::now().as_second(); Self { seconds, offset: 0, @@ -27,28 +23,14 @@ impl Time { /// Return the current local time, or `None` if the local time wasn't available. pub fn now_local() -> Option { - let now = time::OffsetDateTime::now_utc(); - let seconds = now.sub(std::time::SystemTime::UNIX_EPOCH).whole_seconds(); - // TODO: make this work without cfg(unsound_local_offset), see - // https://github.com/time-rs/time/issues/293#issuecomment-909158529 - let offset = time::UtcOffset::local_offset_at(now).ok()?.whole_seconds(); - Self { - seconds, - offset, - sign: offset.into(), - } - .into() + Some(Self::now_local_or_utc()) } /// Return the current local time, or the one at UTC if the local time wasn't available. pub fn now_local_or_utc() -> Self { - let now = time::OffsetDateTime::now_utc(); - let seconds = now.sub(std::time::SystemTime::UNIX_EPOCH).whole_seconds(); - // TODO: make this work without cfg(unsound_local_offset), see - // https://github.com/time-rs/time/issues/293#issuecomment-909158529 - let offset = time::UtcOffset::local_offset_at(now) - .map(time::UtcOffset::whole_seconds) - .unwrap_or(0); + let zdt = jiff::Zoned::now(); + let seconds = zdt.timestamp().as_second(); + let offset = zdt.offset().seconds(); Self { seconds, offset, diff --git a/gix-date/src/time/mod.rs b/gix-date/src/time/mod.rs index fefd740f23a..b4d20089a0b 100644 --- a/gix-date/src/time/mod.rs +++ b/gix-date/src/time/mod.rs @@ -31,7 +31,7 @@ pub enum Format { /// A custom format for printing and parsing time. #[derive(Clone, Copy, Debug)] -pub struct CustomFormat(pub(crate) &'static [time::format_description::FormatItem<'static>]); +pub struct CustomFormat(pub(crate) &'static str); impl From for Format { fn from(custom_format: CustomFormat) -> Format { diff --git a/gix-date/tests/time/parse.rs b/gix-date/tests/time/parse.rs index 8b96cb16ed2..f29457336a8 100644 --- a/gix-date/tests/time/parse.rs +++ b/gix-date/tests/time/parse.rs @@ -146,7 +146,7 @@ mod relative { use std::time::SystemTime; use gix_date::time::Sign; - use time::{Duration, OffsetDateTime}; + use jiff::{ToSpan, Zoned}; #[test] fn large_offsets() { @@ -173,12 +173,19 @@ mod relative { let two_weeks_ago = gix_date::parse("2 weeks ago", Some(now)).unwrap(); assert_eq!(Sign::Plus, two_weeks_ago.sign); assert_eq!(0, two_weeks_ago.offset); - let expected = OffsetDateTime::from(now).saturating_sub(Duration::weeks(2)); - // account for the loss of precision when creating `Time` with seconds - let expected = expected.replace_nanosecond(0).unwrap(); + let expected = Zoned::try_from(now) + .unwrap() + // account for the loss of precision when creating `Time` with seconds + .round( + jiff::ZonedRound::new() + .smallest(jiff::Unit::Second) + .mode(jiff::RoundMode::Trunc), + ) + .unwrap() + .saturating_sub(2.weeks()); assert_eq!( - OffsetDateTime::from_unix_timestamp(two_weeks_ago.seconds).unwrap(), - expected, + jiff::Timestamp::from_second(two_weeks_ago.seconds).unwrap(), + expected.timestamp(), "relative times differ" ); } diff --git a/src/plumbing/main.rs b/src/plumbing/main.rs index c1a41d160c6..02e3b974a15 100644 --- a/src/plumbing/main.rs +++ b/src/plumbing/main.rs @@ -54,11 +54,6 @@ pub mod async_util { pub fn main() -> Result<()> { let args: Args = Args::parse_from(gix::env::args_os()); - #[allow(unsafe_code)] - unsafe { - // SAFETY: we don't manipulate the environment from any thread - time::util::local_offset::set_soundness(time::util::local_offset::Soundness::Unsound); - } let thread_limit = args.threads; let verbose = args.verbose; let format = args.format; diff --git a/src/porcelain/main.rs b/src/porcelain/main.rs index 95645d843d5..3bd15362b07 100644 --- a/src/porcelain/main.rs +++ b/src/porcelain/main.rs @@ -12,11 +12,6 @@ use crate::porcelain::options::{Args, Subcommands}; pub fn main() -> Result<()> { let args: Args = Args::parse_from(gix::env::args_os()); - #[allow(unsafe_code)] - unsafe { - // SAFETY: we don't manipulate the environment from any thread - time::util::local_offset::set_soundness(time::util::local_offset::Soundness::Unsound); - } let should_interrupt = Arc::new(AtomicBool::new(false)); #[allow(unsafe_code)] unsafe {