mirror of
https://github.com/storytold/photocraft.git
synced 2026-10-24 09:57:08 -06:00
Use chrono instead of hand-rolled UTC conversion (#8234)
* Use chrono for UTC/calendar conversion instead of hand-rolled copies Replaces the civil_from_days copies behind the app's log timestamp, the Measurement Log's date/time, and the perf report date with chrono as a workspace dependency (reusing the version already in the tree). Output formats are unchanged and pinned by the existing tests. AGENTS.md now says time goes through chrono. Follow-up to storytold/cadcraft#1785 * Never panic on a clock outside chrono's range; keep 5-digit years unprefixed chrono's From<SystemTime> unwraps internally, so a clock past year 262143 panicked. logging::timestamp and analysis_cmds::now_iso now read the duration since the epoch and go through the fallible DateTime::from_timestamp, reading an out-of-range or pre-epoch clock as the epoch as the old saturating code did. All three ISO helpers (timestamp, now_iso, perf::iso_utc) write the year with {:04} rather than %Y, which prefixes a '+' past year 9999. Tests cover the far-future and year-10000 cases for timestamp and iso_utc. * Don't build an unrepresentable SystemTime in the far-future test UNIX_EPOCH + Duration::from_secs(u64::MAX / 2) overflows Windows' SystemTime and panicked before timestamp() ran, failing the Windows test job. Build it with checked_add and only assert where it exists. * time: preserve old out-of-range date conversion (no epoch fallback) `DateTime::from_timestamp(...).unwrap_or_default()` silently reset any instant past chrono's year range to 1970-01-01, changing output the old hand-rolled Howard-Hinnant code converted exactly (e.g. 10_000_000_000_000 s was 318857-05-20 17:46:40, not the epoch). Restore the old conversion domain with a cycle-based helper that routes the calendar through chrono (the calendar repeats every 400 years = 146_097 days, so month/day/time come from chrono and only the year is carried in i128, keeping every i64 exact). - apps/photocraft/src/logging.rs: `timestamp` via `utc_parts`; keep the pre-1970 epoch clamp; u64 secs past i64::MAX saturate, not epoch. - crates/engine/src/analysis_cmds.rs: extract pure `iso_from_unix` (cycle based) from `now_iso` so the conversion is tested directly. - xtask/src/perf.rs: `iso_utc` cycle-based, accepts the full u64 range the old code did. Pinned old outputs in tests: logging u64::MAX/2 s -> 292277026596-12-04T15:30:07.000Z (was epoch), 10_000_000_000_000 s -> 318857-05-20T17:46:40.000Z iso_from_unix 10_000_000_000_000 -> 318857-05-20T17:46:40Z, -10_000_000_000_000 -> -314918-08-13T06:13:20Z, i64 extremes iso_utc u64::MAX -> 584554051223-11-09T07:00:15Z (was epoch), 10_000_000_000_000 -> 318857-05-20T17:46:40Z * Don't build a year-318857 SystemTime unchecked in the logging test Windows' SystemTime only reaches about year 30828, so UNIX_EPOCH + Duration::from_secs(10_000_000_000_000) panicked before timestamp() ran and failed the Windows test job. Build it with checked_add and assert where the platform can hold it. The year-10000 case fits everywhere and stays unconditional. * Name the 400-year range extension as the one exception to the time rule The rule said never to hand-roll calendar math, but the range fix (shifting by whole 400-year Gregorian cycles so dates past chrono's year range keep their old output) is cycle arithmetic, so the rule and the code disagreed. The rule now names that exception: the library still computes month, day and time, and only whole cycles are carried in the year. * Say the library computes month and day; time-of-day is plain arithmetic Narrows the range-extension wording: the date library computes the calendar fields, and splitting seconds into hours, minutes and seconds is ordinary arithmetic, which is what the code does.
This commit is contained in:
@@ -70,6 +70,7 @@ People trust PhotoCraft with their work, and a crash loses it. A malformed file,
|
||||
8. **Performance is a feature.** Benchmark heavy operations on a 24–36 MP image in release. Work per tile in parallel (rayon), skip empty tiles, never scan a full surface per frame (cache per revision), and record before/after timings in the dev log.
|
||||
|
||||
9. **Never panic on input** (see *Never crash* above). A command's `run` closure and anything it calls must return `Err`, never panic, for *any* params or document state: validate params, check bounds before indexing or dividing, and reject absurd sizes before allocating. The `panic_hunt` integration test fuzzes every command with adversarial params and must stay green.
|
||||
10. **Time goes through `chrono`** (workspace dependency). Never hand-roll UTC/calendar conversion (days-to-date algorithms, leap-year math); a crate owns only its output format string. The one exception is range extension: past the years chrono can represent, shift the date by whole 400-year Gregorian cycles (146,097 days, after which the calendar repeats), let chrono compute the month and day, and add the cycles back to the year (splitting seconds into hours, minutes and seconds is plain arithmetic).
|
||||
|
||||
## 4. Picking work
|
||||
|
||||
|
||||
Generated
+3
@@ -4642,6 +4642,7 @@ version = "0.12.0"
|
||||
dependencies = [
|
||||
"arboard",
|
||||
"async-io",
|
||||
"chrono",
|
||||
"core-graphics2",
|
||||
"dispatch2",
|
||||
"eframe",
|
||||
@@ -4810,6 +4811,7 @@ name = "photocraft-engine"
|
||||
version = "0.12.0"
|
||||
dependencies = [
|
||||
"blake3",
|
||||
"chrono",
|
||||
"craft-print",
|
||||
"exr",
|
||||
"flate2",
|
||||
@@ -8713,6 +8715,7 @@ checksum = "ec7a2a501ed189703dba8b08142f057e887dfc4b2cc4db2d343ac6376ba3e0b9"
|
||||
name = "xtask"
|
||||
version = "0.12.0"
|
||||
dependencies = [
|
||||
"chrono",
|
||||
"serde",
|
||||
"serde_json",
|
||||
"toml",
|
||||
|
||||
@@ -59,6 +59,8 @@ egui = { version = "0.36", default-features = false }
|
||||
egui_kittest = "0.36"
|
||||
rfd = "0.17"
|
||||
log = "0.4"
|
||||
# All date/time conversion goes through chrono (no hand-rolled calendar code beyond the 400-year range extension; AGENTS.md).
|
||||
chrono = { version = "0.4.45", default-features = false, features = ["std"] }
|
||||
rayon = "1"
|
||||
criterion = { version = "0.5", default-features = false }
|
||||
getrandom = "0.3"
|
||||
|
||||
@@ -33,6 +33,7 @@ eframe = { workspace = true, features = ["wgpu", "accesskit", "wayland", "x11",
|
||||
egui = { workspace = true }
|
||||
serde_json = { workspace = true }
|
||||
log = { workspace = true }
|
||||
chrono = { workspace = true }
|
||||
|
||||
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
|
||||
rfd = { workspace = true }
|
||||
|
||||
@@ -91,27 +91,36 @@ fn matches(name: &str, target: &str) -> bool {
|
||||
}
|
||||
|
||||
/// `2026-10-08T07:59:17.728Z` (UTC, milliseconds). A clock before 1970 reads as the epoch.
|
||||
/// A clock far past chrono's year range still converts (the calendar repeats every 400 years), so
|
||||
/// logging never panics and never silently resets to the epoch for a convertible instant.
|
||||
pub fn timestamp(t: SystemTime) -> String {
|
||||
let since = t.duration_since(std::time::UNIX_EPOCH).unwrap_or_default();
|
||||
let secs = since.as_secs();
|
||||
let (days, rem) = (secs / 86_400, secs % 86_400);
|
||||
let (y, m, d) = civil_from_days(days);
|
||||
format!("{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}.{:03}Z", rem / 3600, rem % 3600 / 60, rem % 60, since.subsec_millis())
|
||||
// Pre-1970 is clamped to the epoch by `unwrap_or_default` above. `as_secs` is `u64`; a value
|
||||
// past `i64::MAX` seconds saturates rather than falling back to the epoch.
|
||||
let secs = i64::try_from(since.as_secs()).unwrap_or(i64::MAX);
|
||||
let (y, t) = utc_parts(secs, since.subsec_nanos());
|
||||
// `{:04}` rather than `%Y`, which writes a `+` before years past 9999.
|
||||
format!("{y:04}-{}", t.format("%m-%dT%H:%M:%S%.3fZ"))
|
||||
}
|
||||
|
||||
/// Days since 1970-01-01 to a proleptic Gregorian (year, month, day) (Howard Hinnant's
|
||||
/// `civil_from_days`, unsigned because `timestamp` clamps at the epoch).
|
||||
fn civil_from_days(days: u64) -> (u64, u64, u64) {
|
||||
let z = days + 719_468;
|
||||
let era = z / 146_097;
|
||||
let doe = z % 146_097;
|
||||
let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365;
|
||||
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
|
||||
let mp = (5 * doy + 2) / 153;
|
||||
let d = doy - (153 * mp + 2) / 5 + 1;
|
||||
let m = if mp < 10 { mp + 3 } else { mp - 9 };
|
||||
let y = yoe + era * 400 + u64::from(m <= 2);
|
||||
(y, m, d)
|
||||
/// Unix seconds (+ nanos) → (proleptic Gregorian year, the same wall-clock instant moved into
|
||||
/// chrono's range by whole 400-year cycles). The calendar repeats every 400 years (146_097 days),
|
||||
/// so month, day and time of day come from chrono and only the year is carried, which keeps every
|
||||
/// `i64` exact. Replaces the old hand-rolled `civil_from_days`; chrono is the only calendar logic.
|
||||
fn utc_parts(secs: i64, nanos: u32) -> (i64, chrono::NaiveDateTime) {
|
||||
use chrono::Datelike;
|
||||
let (days, sod) = (secs.div_euclid(86_400), secs.rem_euclid(86_400));
|
||||
let (cycles, rest) = (days.div_euclid(146_097), days.rem_euclid(146_097));
|
||||
// `rest + 719_163` (1970-01-01 is CE day 719_163) is always a valid CE day, and `sod` < 86_400.
|
||||
let date = i32::try_from(rest + 719_163).ok().and_then(chrono::NaiveDate::from_num_days_from_ce_opt);
|
||||
let time = u32::try_from(sod).ok().and_then(|s| chrono::NaiveTime::from_num_seconds_from_midnight_opt(s, nanos.min(999_999_999)));
|
||||
match (date, time) {
|
||||
(Some(d), Some(t)) => {
|
||||
let y = i128::from(d.year()) + i128::from(cycles) * 400;
|
||||
(i64::try_from(y).unwrap_or(if y < 0 { i64::MIN } else { i64::MAX }), d.and_time(t))
|
||||
}
|
||||
_ => (1970, chrono::NaiveDateTime::default()), // unreachable
|
||||
}
|
||||
}
|
||||
|
||||
fn numbered(dir: &Path, n: usize) -> PathBuf {
|
||||
@@ -416,6 +425,20 @@ mod tests {
|
||||
assert_eq!(timestamp(UNIX_EPOCH + Duration::from_secs(1_709_164_800)), "2024-02-29T00:00:00.000Z");
|
||||
// Before the epoch (a clock set wrong) clamps instead of panicking.
|
||||
assert_eq!(timestamp(UNIX_EPOCH - Duration::from_secs(5)), "1970-01-01T00:00:00.000Z");
|
||||
// Past chrono's year range still converts via the 400-year cycle (the old code did too,
|
||||
// through `civil_from_days`): never a silent reset to the epoch. Platform-safe: Windows'
|
||||
// SystemTime can't hold this instant, so it is only checked where it exists.
|
||||
// `u64::MAX / 2` seconds == `i64::MAX` seconds → year 292277026596 (old output).
|
||||
if let Some(far) = UNIX_EPOCH.checked_add(Duration::from_secs(u64::MAX / 2)) {
|
||||
assert_eq!(timestamp(far), "292277026596-12-04T15:30:07.000Z");
|
||||
}
|
||||
// Years past 9999 keep the plain zero-padded year (no `+`).
|
||||
assert_eq!(timestamp(UNIX_EPOCH + Duration::from_secs(253_402_300_800)), "10000-01-01T00:00:00.000Z");
|
||||
// 10_000_000_000_000 s is far beyond chrono's range; old output is year 318857.
|
||||
// Windows' SystemTime holds only up to about year 30828, so this is checked where it fits.
|
||||
if let Some(far) = UNIX_EPOCH.checked_add(Duration::from_secs(10_000_000_000_000)) {
|
||||
assert_eq!(timestamp(far), "318857-05-20T17:46:40.000Z");
|
||||
}
|
||||
}
|
||||
|
||||
#[test]
|
||||
|
||||
@@ -42,6 +42,7 @@ roxmltree = "0.20"
|
||||
|
||||
[target.'cfg(not(target_arch = "wasm32"))'.dependencies]
|
||||
tempfile = "3"
|
||||
chrono = { workspace = true }
|
||||
|
||||
# `file_cmds::same_file` on Windows: the volume serial number and file index, so a hard link to a
|
||||
# file is that file (std's `MetadataExt::file_index` is unstable).
|
||||
|
||||
@@ -231,18 +231,7 @@ pub fn now_iso() -> String {
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
{
|
||||
let secs = std::time::SystemTime::now().duration_since(std::time::UNIX_EPOCH).map(|d| d.as_secs() as i64).unwrap_or(0);
|
||||
let (days, rem) = (secs.div_euclid(86_400), secs.rem_euclid(86_400));
|
||||
// Civil date from days since 1970-01-01 (Howard Hinnant's algorithm).
|
||||
let z = days + 719_468;
|
||||
let era = z.div_euclid(146_097);
|
||||
let doe = z - era * 146_097;
|
||||
let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365;
|
||||
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
|
||||
let mp = (5 * doy + 2) / 153;
|
||||
let d = doy - (153 * mp + 2) / 5 + 1;
|
||||
let m = if mp < 10 { mp + 3 } else { mp - 9 };
|
||||
let y = yoe + era * 400 + i64::from(m <= 2);
|
||||
format!("{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}Z", rem / 3600, rem % 3600 / 60, rem % 60)
|
||||
iso_from_unix(secs)
|
||||
}
|
||||
#[cfg(target_arch = "wasm32")]
|
||||
{
|
||||
@@ -250,6 +239,29 @@ pub fn now_iso() -> String {
|
||||
}
|
||||
}
|
||||
|
||||
/// `YYYY-MM-DDTHH:MM:SSZ` for Unix seconds `secs` (UTC, proleptic Gregorian). A time outside
|
||||
/// chrono's year range still converts (the calendar repeats every 400 years, so the instant is
|
||||
/// moved into range by whole cycles and only the year is carried, matching the old hand-rolled
|
||||
/// code), never a silent reset to the epoch. chrono is the only calendar logic.
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn iso_from_unix(secs: i64) -> String {
|
||||
use chrono::Datelike;
|
||||
let (days, sod) = (secs.div_euclid(86_400), secs.rem_euclid(86_400));
|
||||
let (cycles, rest) = (days.div_euclid(146_097), days.rem_euclid(146_097));
|
||||
// `rest + 719_163` (1970-01-01 is CE day 719_163) is always a valid CE day, and `sod` < 86_400.
|
||||
let date = i32::try_from(rest + 719_163).ok().and_then(chrono::NaiveDate::from_num_days_from_ce_opt);
|
||||
let time = u32::try_from(sod).ok().and_then(|s| chrono::NaiveTime::from_num_seconds_from_midnight_opt(s, 0));
|
||||
match (date, time) {
|
||||
(Some(d), Some(t)) => {
|
||||
let y = i128::from(d.year()) + i128::from(cycles) * 400;
|
||||
let y = i64::try_from(y).unwrap_or(if y < 0 { i64::MIN } else { i64::MAX });
|
||||
// `{:04}` rather than `%Y`, which writes a `+` before years past 9999.
|
||||
format!("{y:04}-{}", d.and_time(t).format("%m-%dT%H:%M:%SZ"))
|
||||
}
|
||||
_ => "1970-01-01T00:00:00Z".to_string(), // unreachable
|
||||
}
|
||||
}
|
||||
|
||||
fn round4(v: f64) -> f64 {
|
||||
(v * 10_000.0).round() / 10_000.0
|
||||
}
|
||||
|
||||
@@ -535,3 +535,20 @@ fn count_add_keeps_first_group_creation_and_targets_the_requested_group() {
|
||||
let r = s.execute("count.add", json!({"x": 6, "y": 7})).unwrap();
|
||||
assert_eq!(r["groups"][1]["points"], json!([[6.0, 7.0]]));
|
||||
}
|
||||
|
||||
#[test]
|
||||
#[cfg(not(target_arch = "wasm32"))]
|
||||
fn iso_from_unix_matches_the_old_cycle_conversion_including_beyond_chrono() {
|
||||
use super::iso_from_unix;
|
||||
assert_eq!(iso_from_unix(0), "1970-01-01T00:00:00Z");
|
||||
assert_eq!(iso_from_unix(951_782_400), "2000-02-29T00:00:00Z");
|
||||
assert_eq!(iso_from_unix(1_791_446_357), "2026-10-08T07:59:17Z");
|
||||
// Years past 9999 keep the plain zero-padded year (no `+`).
|
||||
assert_eq!(iso_from_unix(253_402_300_800), "10000-01-01T00:00:00Z");
|
||||
// Beyond chrono's year range the old hand-rolled code still converted; pin that output
|
||||
// (never the epoch) in both directions and at the i64 extremes.
|
||||
assert_eq!(iso_from_unix(10_000_000_000_000), "318857-05-20T17:46:40Z");
|
||||
assert_eq!(iso_from_unix(-10_000_000_000_000), "-314918-08-13T06:13:20Z");
|
||||
assert_eq!(iso_from_unix(i64::MAX), "292277026596-12-04T15:30:07Z");
|
||||
assert_eq!(iso_from_unix(i64::MIN), "-292277022657-01-27T08:29:52Z");
|
||||
}
|
||||
|
||||
@@ -11,5 +11,6 @@ workspace = true
|
||||
[dependencies]
|
||||
serde_json = { workspace = true }
|
||||
serde = { workspace = true }
|
||||
chrono = { workspace = true }
|
||||
# perf/budgets.toml and scorecard/*.toml (MIT OR Apache-2.0).
|
||||
toml = "1"
|
||||
|
||||
+32
-13
@@ -541,20 +541,27 @@ pub fn machine() -> Value {
|
||||
json!({"os": os, "os_version": os_version, "arch": arch, "cpu": cpu, "cores": cores, "ram_bytes": ram})
|
||||
}
|
||||
|
||||
/// `YYYY-MM-DDTHH:MM:SSZ` for a Unix time (civil-from-days, proleptic Gregorian).
|
||||
/// `YYYY-MM-DDTHH:MM:SSZ` for a Unix time (proleptic Gregorian, UTC). A time past chrono's year
|
||||
/// range still converts (the calendar repeats every 400 years, so the instant is moved into range
|
||||
/// by whole cycles and only the year is carried, matching the old hand-rolled `iso_utc`), never a
|
||||
/// silent reset to the epoch. Accepts the full `u64` range the old code did.
|
||||
pub fn iso_utc(secs: u64) -> String {
|
||||
let days = (secs / 86_400) as i64;
|
||||
let rem = secs % 86_400;
|
||||
let z = days + 719_468;
|
||||
let era = z.div_euclid(146_097);
|
||||
let doe = z.rem_euclid(146_097);
|
||||
let yoe = (doe - doe / 1460 + doe / 36_524 - doe / 146_096) / 365;
|
||||
let doy = doe - (365 * yoe + yoe / 4 - yoe / 100);
|
||||
let mp = (5 * doy + 2) / 153;
|
||||
let d = doy - (153 * mp + 2) / 5 + 1;
|
||||
let m = if mp < 10 { mp + 3 } else { mp - 9 };
|
||||
let y = yoe + era * 400 + i64::from(m <= 2);
|
||||
format!("{y:04}-{m:02}-{d:02}T{:02}:{:02}:{:02}Z", rem / 3600, rem % 3600 / 60, rem % 60)
|
||||
use chrono::Datelike;
|
||||
// `secs / 86_400` is at most 213_503_982_334_601, so `days` always fits in `i64` (old code too).
|
||||
let (days, sod) = ((secs / 86_400) as i64, (secs % 86_400) as i64);
|
||||
let (cycles, rest) = (days.div_euclid(146_097), days.rem_euclid(146_097));
|
||||
// `rest + 719_163` (1970-01-01 is CE day 719_163) is always a valid CE day, and `sod` < 86_400.
|
||||
let date = i32::try_from(rest + 719_163).ok().and_then(chrono::NaiveDate::from_num_days_from_ce_opt);
|
||||
let time = u32::try_from(sod).ok().and_then(|s| chrono::NaiveTime::from_num_seconds_from_midnight_opt(s, 0));
|
||||
match (date, time) {
|
||||
(Some(d), Some(t)) => {
|
||||
let y = i128::from(d.year()) + i128::from(cycles) * 400;
|
||||
let y = i64::try_from(y).unwrap_or(if y < 0 { i64::MIN } else { i64::MAX });
|
||||
// `{:04}` rather than `%Y`, which writes a `+` before years past 9999.
|
||||
format!("{y:04}-{}", d.and_time(t).format("%m-%dT%H:%M:%SZ"))
|
||||
}
|
||||
_ => "1970-01-01T00:00:00Z".to_string(), // unreachable
|
||||
}
|
||||
}
|
||||
|
||||
fn now_iso() -> String {
|
||||
@@ -927,6 +934,18 @@ pub fn render_summary(results: &Value, outcomes: &[Outcome], baseline_note: Opti
|
||||
mod tests {
|
||||
use super::*;
|
||||
|
||||
#[test]
|
||||
fn iso_utc_handles_boundaries() {
|
||||
assert_eq!(iso_utc(0), "1970-01-01T00:00:00Z");
|
||||
assert_eq!(iso_utc(1_791_446_357), "2026-10-08T07:59:17Z");
|
||||
// Years past 9999 keep the plain zero-padded year (no `+`).
|
||||
assert_eq!(iso_utc(253_402_300_800), "10000-01-01T00:00:00Z");
|
||||
// A value past chrono's range still converts via the 400-year cycle (the old hand-rolled
|
||||
// `iso_utc` did too): pin that output, never the epoch.
|
||||
assert_eq!(iso_utc(10_000_000_000_000), "318857-05-20T17:46:40Z");
|
||||
assert_eq!(iso_utc(u64::MAX), "584554051223-11-09T07:00:15Z");
|
||||
}
|
||||
|
||||
const TOML: &str = r#"
|
||||
[settings]
|
||||
regression_pct = 15.0
|
||||
|
||||
Reference in New Issue
Block a user