Skip to content

Commit 334b7b6

Browse files
authored
fix(index): Apply feedback from Cargo team (#16369)
### What does this PR try to resolve? Questions from #16270 - How should we deal with Summary pubtime parse errors? - Treat the entry as invalid, rather than recovering since we have no reason to allow other layouts in the future - How strict should we be on the Summary pubtime format? - Limit to what we specify which is pretty strict - What precision do we want? - No more than seconds seems the right choice. Through the conversation, it also came up that we should be very explicit that `pubtime` is only for publish time and not updates like `yanked` ### How to test and review this PR?
2 parents 8ff9d67 + 73ae7d5 commit 334b7b6

File tree

8 files changed

+165
-17
lines changed

8 files changed

+165
-17
lines changed

Cargo.lock

Lines changed: 2 additions & 1 deletion
Some generated files are not rendered by default. Learn more about customizing how changed files appear on GitHub.

Cargo.toml

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,7 @@ cargo-platform = { path = "crates/cargo-platform", version = "0.3.0" }
3434
cargo-test-macro = { version = "0.4.8", path = "crates/cargo-test-macro" }
3535
cargo-test-support = { version = "0.9.1", path = "crates/cargo-test-support" }
3636
cargo-util = { version = "0.2.26", path = "crates/cargo-util" }
37-
cargo-util-schemas = { version = "0.11.0", path = "crates/cargo-util-schemas" }
37+
cargo-util-schemas = { version = "0.12.0", path = "crates/cargo-util-schemas" }
3838
cargo_metadata = "0.23.1"
3939
clap = "4.5.53"
4040
clap_complete = { version = "4.5.61", features = ["unstable-dynamic"] }

crates/cargo-util-schemas/Cargo.toml

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,6 @@
11
[package]
22
name = "cargo-util-schemas"
3-
version = "0.11.0"
3+
version = "0.12.0"
44
rust-version = "1.91" # MSRV:1
55
edition.workspace = true
66
license.workspace = true
@@ -9,6 +9,7 @@ repository.workspace = true
99
description = "Deserialization schemas for Cargo"
1010

1111
[dependencies]
12+
jiff = { workspace = true, features = ["std", "serde"] }
1213
schemars = { workspace = true, features = ["preserve_order", "semver1"], optional = true }
1314
semver.workspace = true
1415
serde = { workspace = true, features = ["derive"] }

crates/cargo-util-schemas/index.schema.json

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -69,11 +69,12 @@
6969
]
7070
},
7171
"pubtime": {
72-
"description": "The publish time for the package. Unstable.\n\nIn ISO8601 with UTC timezone (e.g. 2025-11-12T19:30:12Z)",
72+
"description": "The publish time for the package. Unstable.\n\nIn ISO8601 with UTC timezone (e.g. 2025-11-12T19:30:12Z)\n\nThis should be the original publish time and not changed on any status changes,\nlike [`IndexPackage::yanked`].",
7373
"type": [
7474
"string",
7575
"null"
76-
]
76+
],
77+
"default": null
7778
},
7879
"v": {
7980
"description": "The schema version for this entry.\n\nIf this is None, it defaults to version `1`. Entries with unknown\nversions are ignored.\n\nVersion `2` schema adds the `features2` field.\n\nVersion `3` schema adds `artifact`, `bindep_targes`, and `lib` for\nartifact dependencies support.\n\nThis provides a method to safely introduce changes to index entries\nand allow older versions of cargo to ignore newer entries it doesn't\nunderstand. This is honored as of 1.51, so unfortunately older\nversions will ignore it, and potentially misinterpret version 2 and\nnewer entries.\n\nThe intent is that versions older than 1.51 will work with a\npre-existing `Cargo.lock`, but they may not correctly process `cargo\nupdate` or build a lock from scratch. In that case, cargo may\nincorrectly select a new package that uses a new index schema. A\nworkaround is to downgrade any packages that are incompatible with the\n`--precise` flag of `cargo update`.",

crates/cargo-util-schemas/src/index.rs

Lines changed: 115 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,13 @@ pub struct IndexPackage<'a> {
4949
/// The publish time for the package. Unstable.
5050
///
5151
/// In ISO8601 with UTC timezone (e.g. 2025-11-12T19:30:12Z)
52-
pub pubtime: Option<String>,
52+
///
53+
/// This should be the original publish time and not changed on any status changes,
54+
/// like [`IndexPackage::yanked`].
55+
#[cfg_attr(feature = "unstable-schema", schemars(with = "Option<String>"))]
56+
#[serde(with = "serde_pubtime")]
57+
#[serde(default)]
58+
pub pubtime: Option<jiff::Timestamp>,
5359
/// The schema version for this entry.
5460
///
5561
/// If this is None, it defaults to version `1`. Entries with unknown
@@ -117,6 +123,76 @@ pub struct RegistryDependency<'a> {
117123
pub lib: bool,
118124
}
119125

126+
pub fn parse_pubtime(s: &str) -> Result<jiff::Timestamp, jiff::Error> {
127+
let dt = jiff::civil::DateTime::strptime("%Y-%m-%dT%H:%M:%SZ", s)?;
128+
if s.len() == 20 {
129+
let zoned = dt.to_zoned(jiff::tz::TimeZone::UTC)?;
130+
let timestamp = zoned.timestamp();
131+
Ok(timestamp)
132+
} else {
133+
Err(jiff::Error::from_args(format_args!(
134+
"padding required for `{s}`"
135+
)))
136+
}
137+
}
138+
139+
pub fn format_pubtime(t: jiff::Timestamp) -> String {
140+
t.strftime("%Y-%m-%dT%H:%M:%SZ").to_string()
141+
}
142+
143+
mod serde_pubtime {
144+
#[inline]
145+
pub(super) fn serialize<S: serde::Serializer>(
146+
timestamp: &Option<jiff::Timestamp>,
147+
se: S,
148+
) -> Result<S::Ok, S::Error> {
149+
match *timestamp {
150+
None => se.serialize_none(),
151+
Some(ref ts) => {
152+
let s = super::format_pubtime(*ts);
153+
se.serialize_str(&s)
154+
}
155+
}
156+
}
157+
158+
#[inline]
159+
pub(super) fn deserialize<'de, D: serde::Deserializer<'de>>(
160+
de: D,
161+
) -> Result<Option<jiff::Timestamp>, D::Error> {
162+
de.deserialize_option(OptionalVisitor(
163+
serde_untagged::UntaggedEnumVisitor::new()
164+
.expecting("date time")
165+
.string(|value| super::parse_pubtime(&value).map_err(serde::de::Error::custom)),
166+
))
167+
}
168+
169+
/// A generic visitor for `Option<DateTime>`.
170+
struct OptionalVisitor<V>(V);
171+
172+
impl<'de, V: serde::de::Visitor<'de, Value = jiff::Timestamp>> serde::de::Visitor<'de>
173+
for OptionalVisitor<V>
174+
{
175+
type Value = Option<jiff::Timestamp>;
176+
177+
fn expecting(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
178+
f.write_str("date time")
179+
}
180+
181+
#[inline]
182+
fn visit_some<D: serde::de::Deserializer<'de>>(
183+
self,
184+
de: D,
185+
) -> Result<Option<jiff::Timestamp>, D::Error> {
186+
de.deserialize_str(self.0).map(Some)
187+
}
188+
189+
#[inline]
190+
fn visit_none<E: serde::de::Error>(self) -> Result<Option<jiff::Timestamp>, E> {
191+
Ok(None)
192+
}
193+
}
194+
}
195+
120196
fn default_true() -> bool {
121197
true
122198
}
@@ -161,3 +237,41 @@ fn dump_index_schema() {
161237
let dump = serde_json::to_string_pretty(&schema).unwrap();
162238
snapbox::assert_data_eq!(dump, snapbox::file!("../index.schema.json").raw());
163239
}
240+
241+
#[test]
242+
fn pubtime_format() {
243+
use snapbox::str;
244+
245+
let input = [
246+
("2025-11-12T19:30:12Z", Some(str!["2025-11-12T19:30:12Z"])),
247+
// Padded values
248+
("2025-01-02T09:03:02Z", Some(str!["2025-01-02T09:03:02Z"])),
249+
// Alt timezone format
250+
("2025-11-12T19:30:12-04", None),
251+
// Alt date/time separator
252+
("2025-11-12 19:30:12Z", None),
253+
// Non-padded values
254+
("2025-11-12T19:30:12+4", None),
255+
("2025-1-12T19:30:12+4", None),
256+
("2025-11-2T19:30:12+4", None),
257+
("2025-11-12T9:30:12Z", None),
258+
("2025-11-12T19:3:12Z", None),
259+
("2025-11-12T19:30:2Z", None),
260+
];
261+
for (input, expected) in input {
262+
let (parsed, expected) = match (parse_pubtime(input), expected) {
263+
(Ok(_), None) => {
264+
panic!("`{input}` did not error");
265+
}
266+
(Ok(parsed), Some(expected)) => (parsed, expected),
267+
(Err(err), Some(_)) => {
268+
panic!("`{input}` did not parse successfully: {err}");
269+
}
270+
_ => {
271+
continue;
272+
}
273+
};
274+
let output = format_pubtime(parsed);
275+
snapbox::assert_data_eq!(output, expected);
276+
}
277+
}

src/bin/cargo/commands/generate_lockfile.rs

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
5050
if let Some(publish_time) = publish_time {
5151
gctx.cli_unstable()
5252
.fail_if_stable_opt("--publish-time", 5221)?;
53-
ws.set_resolve_publish_time(publish_time.parse().map_err(anyhow::Error::from)?);
53+
let publish_time =
54+
cargo_util_schemas::index::parse_pubtime(publish_time).map_err(anyhow::Error::from)?;
55+
ws.set_resolve_publish_time(publish_time);
5456
}
5557
ops::generate_lockfile(&ws)?;
5658
Ok(())

src/cargo/sources/registry/index/mod.rs

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -217,7 +217,7 @@ fn index_package_to_summary(pkg: &IndexPackage<'_>, source_id: SourceId) -> Carg
217217
let links: Option<InternedString> = pkg.links.as_ref().map(|l| l.as_ref().into());
218218
let mut summary = Summary::new(pkgid, deps, &features, links, pkg.rust_version.clone())?;
219219
summary.set_checksum(pkg.cksum.clone());
220-
if let Some(pubtime) = pkg.pubtime.as_ref().and_then(|p| p.parse().ok()) {
220+
if let Some(pubtime) = pkg.pubtime {
221221
summary.set_pubtime(pubtime);
222222
}
223223
Ok(summary)

tests/testsuite/generate_lockfile.rs

Lines changed: 38 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -320,12 +320,8 @@ fn publish_time() {
320320
Package::new("has_time", "2025.6.1")
321321
.pubtime("2025-06-01T06:00:00Z")
322322
.publish();
323-
Package::new("no_time", "2025.1.1")
324-
.pubtime("2025-01-01T06:00:00Z")
325-
.publish();
326-
Package::new("no_time", "2025.6.1")
327-
.pubtime("2025-06-01T06:00:00Z")
328-
.publish();
323+
Package::new("no_time", "2025.1.1").publish();
324+
Package::new("no_time", "2025.6.1").publish();
329325

330326
let p = project()
331327
.file(
@@ -348,7 +344,6 @@ fn publish_time() {
348344
[UPDATING] `dummy-registry` index
349345
[LOCKING] 2 packages to latest compatible versions as of 2025-03-01T06:00:00Z
350346
[ADDING] has_time v2025.1.1 (available: v2025.6.1)
351-
[ADDING] no_time v2025.1.1 (available: v2025.6.1)
352347
353348
"#]])
354349
.run();
@@ -377,9 +372,9 @@ checksum = "105ca3acbc796da3e728ff310cafc6961cebc48d0106285edb8341015b5cc2d7"
377372
378373
[[package]]
379374
name = "no_time"
380-
version = "2025.1.1"
375+
version = "2025.6.1"
381376
source = "registry+https://github.com/rust-lang/crates.io-index"
382-
checksum = "fd3832e543b832e9270f27f9a56a3f3170aeaf32debe355b2fa4e61ede80a39f"
377+
checksum = "01e688c07975f1e85f526c033322273181a4d8fe97800543d813d0a0adc134e3"
383378
384379
"##]],
385380
);
@@ -418,3 +413,37 @@ required by package `foo v0.0.0 ([ROOT]/foo)`
418413
"#]])
419414
.run();
420415
}
416+
417+
#[cargo_test]
418+
fn publish_time_invalid() {
419+
Package::new("has_time", "2025.6.1")
420+
.pubtime("2025-06-01T06:00:00")
421+
.publish();
422+
423+
let p = project()
424+
.file(
425+
"Cargo.toml",
426+
r#"
427+
[package]
428+
name = "foo"
429+
430+
[dependencies]
431+
has_time = "2025.0"
432+
"#,
433+
)
434+
.file("src/lib.rs", "")
435+
.build();
436+
437+
p.cargo("generate-lockfile --publish-time 2025-03-01T06:00:00Z -Zunstable-options")
438+
.masquerade_as_nightly_cargo(&["publish-time"])
439+
.with_status(101)
440+
.with_stderr_data(str![[r#"
441+
[UPDATING] `dummy-registry` index
442+
[ERROR] failed to select a version for the requirement `has_time = "^2025.0"`
443+
version 2025.6.1's index entry is invalid
444+
location searched: `dummy-registry` index (which is replacing registry `crates-io`)
445+
required by package `foo v0.0.0 ([ROOT]/foo)`
446+
447+
"#]])
448+
.run();
449+
}

0 commit comments

Comments
 (0)