feat: -Zmin-publish-age (RFC 3923)#17012
Conversation
So it can be reused by the upcoming `-Zmin-publish-age`.
Most call sites constructed with an empty allowlist, and `PackageRegistry::load` was the only place that passed a real list so we do a post construction after PackageRegistry construction in `load`. The parameter was only load-bearing for `PackageRegistry`.
|
r? @epage rustbot has assigned @epage. Use Why was this reviewer chosen?The reviewer was selected based on:
|
rustbot
added
A-cli
rustbot
removed
the
S-waiting-on-review
|
Found something wrong. Wait a sec. |
weihanglo
force-pushed
the
min-publish-age
branch
2 times, most recently
from
2477e6c to
bc3e9c0
Compare
| IndexSummary::TooNew(summary, age) => { | ||
| let opts = jiff::SpanRound::new() | ||
| .largest(jiff::Unit::Day) | ||
| .smallest(jiff::Unit::Minute) | ||
| .relative(jiff::SpanRelativeTo::days_are_24_hours()); |
There was a problem hiding this comment.
The version 1.99.0 is too new (published 2d ago) message currenly show the time span in always down to minute, and up to day precise. It is annoying that you would get something like 2d 8h 23m long. We might want to round it more for better UX. I personally would leave it for follow-up.
There was a problem hiding this comment.
@clouatre commented in #17012 (review):
The
largest=Day, smallest=Minuterounding produces output likepublished 2d 8h 23m ago. Suggest simplifying to a single unit based on magnitude:>= 2 daysrounds to nearest day (published 3 days ago),< 2 daysrounds to nearest hour (published 11 hours ago). The sub-day precision is noise at the day scale and the user cannot act on the minutes component.For context: we currently enforce this in CI with a bash script that diffs
Cargo.lockagainst the base branch and calls the crates.io API to check publish timestamps on net-new crates. This feature would replace that entirely. The error message is the main user-facing surface, so getting the formatting right matters.
There was a problem hiding this comment.
Moved it here so it is better to discuss in a thread.
| } | ||
| } | ||
|
|
||
| fn warn_unused_min_publish_age(gctx: &GlobalContext) -> CargoResult<()> { |
There was a problem hiding this comment.
We warn all unused min-publish-age all at once, regardless of the precedence rule.
There was a problem hiding this comment.
I didn't do an extensive testing for different age span. If it is needed let me know
weihanglo
force-pushed
the
min-publish-age
branch
from
bc3e9c0 to
1ea1f55
Compare
So we have a global shaed invocation for `-Zbuild-analysis` and other upcoming features like `-Zmin-publish-age`.
Also warn when `-Zmin-publish-age` is absent
This implements RFC 3923 min-publish-age. Currently, this works as yanked version though the logic may diverge when needed. Some highlights: * `-Z min-publish-age` nightly feature gate * `IndexSummary::TooNew` variant that mirrors `Yanked` semantics * Report "version X is too new (published N ago)" in resolution errors
weihanglo
force-pushed
the
min-publish-age
branch
from
1ea1f55 to
ddba374
Compare
There was a problem hiding this comment.
The largest=Day, smallest=Minute rounding produces output like published 2d 8h 23m ago. Suggest simplifying to a single unit based on magnitude: >= 2 days rounds to nearest day (published 3 days ago), < 2 days rounds to nearest hour (published 11 hours ago). The sub-day precision is noise at the day scale and the user cannot act on the minutes component.
For context: we currently enforce this in CI with a bash script that diffs Cargo.lock against the base branch and calls the crates.io API to check publish timestamps on net-new crates. This feature would replace that entirely. The error message is the main user-facing surface, so getting the formatting right matters.
|
Blocked on some refactor works. See #17014 (comment) |
|
☔ The latest upstream changes (possibly #17014) made this pull request unmergeable. Please resolve the merge conflicts. |
rustbot
added
the
S-waiting-on-author
### What does this PR try to resolve? Split off from <rust-lang#17012> for an easier review experience I guess. Most call sites constructed with an empty allowlist, and `PackageRegistry::load` was the only place that passed a real list so we do a post construction after `PackageRegistry` in `load`. The parameter was only load-bearing for `PackageRegistry`. ### How to test and review this PR? This has no user-facing behavior change.
4 participants
What does this PR try to resolve?
This implements RFC 3923 min-publish-age (rust-lang/rfcs#3923, #17009)
Currently, this works as yanked version
though the logic may diverge when needed.
Some highlights:
-Z min-publish-agenightly feature gateIndexSummary::TooNewvariant that kind mirrorsYankedsemanticsHow to test and review this PR?
23 new tests are added covering
Open questions
version 1.99.0 is too new (published 2d ago)message currenly show the time span in always down to minute, and up to day precise. It is annoying that you would get something like2d 8h 23mlong. We might want to round it more for better UX. I personally would leave it for follow-up.registry.min-publish-age/registries.min-publish-ageprecedence rule basically follows howregistry.tokenis handle. Forregistry.min-publish-agewe only check if the default registyr is crates.io for consistency withtoken, though it is open to change.