Skip to content

Commit

Permalink
Auto merge of #13979 - tweag:issue-12425, r=epage
Browse files Browse the repository at this point in the history
Add `cargo update --breaking`

Related to #12425.

There are two kinds of manifest mutations here. In `upgrade_manifests` we have to mutate `cargo::core::manifest::Manifest` so that `resolve_ws` does what it needs to do, and outputs a correct lock file. Then, in `write_manifest_upgrades` we mutate `cargo::util::toml_mut::manifest::Manifest`, because that is how we can preserve the existing formatting in the manifest files on disk.

Some of the code here is copied from `cargo-edit`.

# Comparison with `cargo upgrade`

Running on the Cargo source itself gives the following:

```
❯ cargo upgrade --dry-run --incompatible allow --compatible ignore
    Updating 'https://github.com/rust-lang/crates.io-index' index
    Checking benchsuite's dependencies
    Checking capture's dependencies
    Checking cargo's dependencies
name               old req compatible latest  new req note
====               ======= ========== ======  ======= ====
anstream           0.6.13  0.6.14     0.6.14  0.6.13  compatible
anstyle            1.0.6   1.0.7      1.0.7   1.0.6   compatible
anyhow             1.0.82  1.0.86     1.0.86  1.0.82  compatible
itertools          0.12.1  0.12.1     0.13.0  0.13.0
libc               0.2.154 0.2.155    0.2.155 0.2.154 compatible
opener             0.7.0   0.7.1      0.7.1   0.7.0   compatible
openssl            0.10.57 0.10.64    0.10.64 0.10.57 compatible
openssl-sys        =0.9.92 0.9.92     0.9.102 =0.9.92 pinned
pulldown-cmark     0.10.3  0.10.3     0.11.0  0.11.0
security-framework 2.10.0  2.11.0     2.11.0  2.10.0  compatible
semver             1.0.22  1.0.23     1.0.23  1.0.22  compatible
serde              1.0.199 1.0.203    1.0.203 1.0.199 compatible
serde-untagged     0.1.5   0.1.6      0.1.6   0.1.5   compatible
serde_json         1.0.116 1.0.117    1.0.117 1.0.116 compatible
tar                0.4.40  0.4.41     0.4.41  0.4.40  compatible
thiserror          1.0.59  1.0.61     1.0.61  1.0.59  compatible
toml               0.8.12  0.8.14     0.8.14  0.8.12  compatible
toml_edit          0.22.12 0.22.14    0.22.14 0.22.12 compatible
unicode-width      0.1.12  0.1.13     0.1.13  0.1.12  compatible
    Checking cargo-credential's dependencies
    Checking cargo-credential-1password's dependencies
    Checking cargo-credential-libsecret's dependencies
    Checking cargo-credential-macos-keychain's dependencies
    Checking cargo-credential-wincred's dependencies
    Checking cargo-platform's dependencies
    Checking cargo-test-macro's dependencies
    Checking cargo-test-support's dependencies
    Checking cargo-util's dependencies
    Checking cargo-util-schemas's dependencies
    Checking crates-io's dependencies
    Checking home's dependencies
    Checking mdman's dependencies
    Checking resolver-tests's dependencies
    Checking rustfix's dependencies
    Checking semver-check's dependencies
    Checking xtask-build-man's dependencies
    Checking xtask-bump-check's dependencies
    Checking xtask-stale-label's dependencies
note: Re-run with `--pinned` to upgrade pinned version requirements
note: Re-run with `--verbose` to show all dependencies
  local: cargo
  unchanged: annotate-snippets, base64, bytesize, cargo-credential, cargo-credential-libsecret, cargo-credential-macos-keychain, cargo-credential-wincred, cargo-platform, cargo-test-macro, cargo-test-support, cargo-util, cargo-util-schemas, cargo_metadata, clap, color-print, core-foundation, crates-io, criterion, curl, curl-sys, filetime, flate2, git2, git2-curl, gix, glob, handlebars, hex, hmac, home, http-auth, humantime, ignore, im-rc, indexmap, jobserver, lazycell, libgit2-sys, libloading, memchr, miow, os_info, pasetors, pathdiff, percent-encoding, pkg-config, proptest, rand, regex, rusqlite, rustfix, same-file, serde-value, serde_ignored, sha1, sha2, shell-escape, similar, snapbox, supports-hyperlinks, supports-unicode, tempfile, time, tracing, tracing-chrome, tracing-subscriber, unicase, unicode-xid, url, varisat, walkdir, windows-sys
warning: aborting upgrade due to dry run

❯ target/debug/cargo update --breaking --dry-run -Zunstable-options
    Updating crates.io index
   Upgrading itertools ^0.12.1 -> ^0.13.0
   Upgrading pulldown-cmark ^0.10.3 -> ^0.11.0
    Updating crates.io index
     Locking 3 packages to latest compatible versions
    Updating itertools v0.12.1 -> v0.13.0
    Updating pulldown-cmark v0.10.3 -> v0.11.0
    Updating pulldown-cmark-escape v0.10.0 -> v0.11.0
warning: not updating any files due to dry run
```

In both cases we see an upgrade of `itertools` to `^0.13.0` and `pulldown-cmark` to `^0.11.0`.

The diff to the manifest (when it isn't a dry run) is as follows:

```
--- a/Cargo.toml
+++ b/Cargo.toml
`@@` -57,7 +57,7 `@@` humantime = "2.1.0"
 ignore = "0.4.22"
 im-rc = "15.1.0"
 indexmap = "2.2.6"
-itertools = "0.12.1"
+itertools = "0.13.0"
 jobserver = "0.1.31"
 lazycell = "1.3.0"
 libc = "0.2.154"
`@@` -74,7 +74,7 `@@` pathdiff = "0.2.1"
 percent-encoding = "2.3.1"
 pkg-config = "0.3.30"
 proptest = "1.4.0"
-pulldown-cmark = { version = "0.10.3", default-features = false, features = ["html"] }
+pulldown-cmark = { version = "0.11.0", default-features = false, features = ["html"] }
 rand = "0.8.5"
 regex = "1.10.4"
 rusqlite = { version = "0.31.0", features = ["bundled"] }
```

# TODO

- [x] In the case of `--incompatible`, we also need to let `update_lockfile` use `upgrades` in order to only update the incompatible dependencies.
- [x] Testing all the different cases of package sources, version requirements, pinned versions, renamed dependencies, inherited workspace dependencies, multiple versions of the same dependency, selecting a subset `--package`, etc.
- [x] Passing tests.
- [x] Implement suggestions from reviews.
- [x] The preservation of formatting in manifest files should be improved.
- [x] Compare with `cargo upgrade`.
  • Loading branch information
bors committed Jun 7, 2024
2 parents e8e50d0 + 031b410 commit fb123c6
Show file tree
Hide file tree
Showing 26 changed files with 1,127 additions and 74 deletions.
4 changes: 4 additions & 0 deletions benches/benchsuite/benches/resolve.rs
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ fn do_resolve<'gctx>(gctx: &'gctx GlobalContext, ws_root: &Path) -> ResolveInfo<
let force_all_targets = ForceAllTargets::No;
// Do an initial run to download anything necessary so that it does
// not confuse criterion's warmup.
let dry_run = false;
let ws_resolve = cargo::ops::resolve_ws_with_opts(
&ws,
&mut target_data,
Expand All @@ -41,6 +42,7 @@ fn do_resolve<'gctx>(gctx: &'gctx GlobalContext, ws_root: &Path) -> ResolveInfo<
&specs,
has_dev_units,
force_all_targets,
dry_run,
)
.unwrap();
ResolveInfo {
Expand Down Expand Up @@ -71,6 +73,7 @@ fn resolve_ws(c: &mut Criterion) {
// iterator once, and we don't want to call `do_resolve` in every
// "step", since that would just be some useless work.
let mut lazy_info = None;
let dry_run = false;
group.bench_function(&ws_name, |b| {
let ResolveInfo {
ws,
Expand All @@ -91,6 +94,7 @@ fn resolve_ws(c: &mut Criterion) {
specs,
*has_dev_units,
*force_all_targets,
dry_run,
)
.unwrap();
})
Expand Down
1 change: 1 addition & 0 deletions crates/cargo-test-support/src/compare.rs
Original file line number Diff line number Diff line change
Expand Up @@ -176,6 +176,7 @@ static E2E_LITERAL_REDACTIONS: &[(&str, &str)] = &[
("[DIRTY]", " Dirty"),
("[LOCKING]", " Locking"),
("[UPDATING]", " Updating"),
("[UPGRADING]", " Upgrading"),
("[ADDING]", " Adding"),
("[REMOVING]", " Removing"),
("[REMOVED]", " Removed"),
Expand Down
8 changes: 3 additions & 5 deletions src/bin/cargo/commands/add.rs
Original file line number Diff line number Diff line change
Expand Up @@ -214,11 +214,9 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
};
add(&ws, &options)?;

if !dry_run {
// Reload the workspace since we've changed dependencies
let ws = args.workspace(gctx)?;
resolve_ws(&ws)?;
}
// Reload the workspace since we've changed dependencies
let ws = args.workspace(gctx)?;
resolve_ws(&ws, dry_run)?;

Ok(())
}
Expand Down
4 changes: 2 additions & 2 deletions src/bin/cargo/commands/remove.rs
Original file line number Diff line number Diff line change
Expand Up @@ -121,15 +121,15 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
ws.gctx()
.shell()
.set_verbosity(cargo::core::Verbosity::Quiet);
let resolve = resolve_ws(&ws);
let resolve = resolve_ws(&ws, dry_run);
ws.gctx().shell().set_verbosity(verbosity);
resolve?.1
};

// Attempt to gc unused patches and re-resolve if anything is removed
if gc_unused_patches(&workspace, &resolve)? {
let ws = args.workspace(gctx)?;
resolve_ws(&ws)?;
resolve_ws(&ws, dry_run)?;
}
}
Ok(())
Expand Down
30 changes: 28 additions & 2 deletions src/bin/cargo/commands/update.rs
Original file line number Diff line number Diff line change
Expand Up @@ -35,6 +35,13 @@ pub fn cli() -> Command {
.value_name("PRECISE")
.requires("package-group"),
)
.arg(
flag(
"breaking",
"Upgrade [SPEC] to latest breaking versions, unless pinned (unstable)",
)
.short('b'),
)
.arg_silent_suggestion()
.arg(
flag("workspace", "Only update the workspace packages")
Expand All @@ -59,7 +66,8 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
gctx.cli_unstable().msrv_policy,
)?;
}
let ws = args.workspace(gctx)?;

let mut ws = args.workspace(gctx)?;

if args.is_present_with_zero_values("package") {
print_available_packages(&ws)?;
Expand Down Expand Up @@ -89,6 +97,24 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
workspace: args.flag("workspace"),
gctx,
};
ops::update_lockfile(&ws, &update_opts)?;

if args.flag("breaking") {
gctx.cli_unstable()
.fail_if_stable_opt("--breaking", 12425)?;

let upgrades = ops::upgrade_manifests(&mut ws, &update_opts.to_update)?;
ops::resolve_ws(&ws, update_opts.dry_run)?;
ops::write_manifest_upgrades(&ws, &upgrades, update_opts.dry_run)?;

if update_opts.dry_run {
update_opts
.gctx
.shell()
.warn("aborting update due to dry run")?;
}
} else {
ops::update_lockfile(&ws, &update_opts)?;
}

Ok(())
}
2 changes: 2 additions & 0 deletions src/cargo/core/compiler/standard_lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -149,6 +149,7 @@ pub fn resolve_std<'gctx>(
let cli_features = CliFeatures::from_command_line(
&features, /*all_features*/ false, /*uses_default_features*/ false,
)?;
let dry_run = false;
let resolve = ops::resolve_ws_with_opts(
&std_ws,
target_data,
Expand All @@ -157,6 +158,7 @@ pub fn resolve_std<'gctx>(
&specs,
HasDevUnits::No,
crate::core::resolver::features::ForceAllTargets::No,
dry_run,
)?;
Ok((
resolve.pkg_set,
Expand Down
16 changes: 13 additions & 3 deletions src/cargo/core/summary.rs
Original file line number Diff line number Diff line change
Expand Up @@ -103,15 +103,25 @@ impl Summary {
Rc::make_mut(&mut self.inner).checksum = Some(cksum);
}

pub fn map_dependencies<F>(mut self, f: F) -> Summary
pub fn map_dependencies<F>(self, mut f: F) -> Summary
where
F: FnMut(Dependency) -> Dependency,
{
self.try_map_dependencies(|dep| Ok(f(dep))).unwrap()
}

pub fn try_map_dependencies<F>(mut self, f: F) -> CargoResult<Summary>
where
F: FnMut(Dependency) -> CargoResult<Dependency>,
{
{
let slot = &mut Rc::make_mut(&mut self.inner).dependencies;
*slot = mem::take(slot).into_iter().map(f).collect();
*slot = mem::take(slot)
.into_iter()
.map(f)
.collect::<CargoResult<_>>()?;
}
self
Ok(self)
}

pub fn map_source(self, to_replace: SourceId, replace_with: SourceId) -> Summary {
Expand Down
12 changes: 10 additions & 2 deletions src/cargo/ops/cargo_clean.rs
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,14 @@ pub fn clean(ws: &Workspace<'_>, opts: &CleanOptions<'_>) -> CargoResult<()> {
if opts.spec.is_empty() {
clean_ctx.remove_paths(&[target_dir.into_path_unlocked()])?;
} else {
clean_specs(&mut clean_ctx, &ws, &profiles, &opts.targets, &opts.spec)?;
clean_specs(
&mut clean_ctx,
&ws,
&profiles,
&opts.targets,
&opts.spec,
opts.dry_run,
)?;
}
}

Expand All @@ -91,11 +98,12 @@ fn clean_specs(
profiles: &Profiles,
targets: &[String],
spec: &[String],
dry_run: bool,
) -> CargoResult<()> {
// Clean specific packages.
let requested_kinds = CompileKind::from_requested_targets(clean_ctx.gctx, targets)?;
let target_data = RustcTargetData::new(ws, &requested_kinds)?;
let (pkg_set, resolve) = ops::resolve_ws(ws)?;
let (pkg_set, resolve) = ops::resolve_ws(ws, dry_run)?;
let prof_dir_name = profiles.get_dir_name();
let host_layout = Layout::new(ws, None, &prof_dir_name)?;
// Convert requested kinds to a Vec of layouts.
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/cargo_compile/mod.rs
Original file line number Diff line number Diff line change
Expand Up @@ -264,6 +264,7 @@ pub fn create_bcx<'a, 'gctx>(
HasDevUnits::No
}
};
let dry_run = false;
let resolve = ops::resolve_ws_with_opts(
ws,
&mut target_data,
Expand All @@ -272,6 +273,7 @@ pub fn create_bcx<'a, 'gctx>(
&specs,
has_dev_units,
crate::core::resolver::features::ForceAllTargets::No,
dry_run,
)?;
let WorkspaceResolve {
mut pkg_set,
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/cargo_fetch.rs
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,8 @@ pub fn fetch<'a>(
options: &FetchOptions<'a>,
) -> CargoResult<(Resolve, PackageSet<'a>)> {
ws.emit_warnings()?;
let (mut packages, resolve) = ops::resolve_ws(ws)?;
let dry_run = false;
let (mut packages, resolve) = ops::resolve_ws(ws, dry_run)?;

let jobs = Some(JobsConfig::Integer(1));
let keep_going = false;
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/cargo_install.rs
Original file line number Diff line number Diff line change
Expand Up @@ -561,7 +561,8 @@ impl<'gctx> InstallablePackage<'gctx> {
// It would be best if `source` could be passed in here to avoid a
// duplicate "Updating", but since `source` is taken by value, then it
// wouldn't be available for `compile_ws`.
let (pkg_set, resolve) = ops::resolve_ws(&self.ws)?;
let dry_run = false;
let (pkg_set, resolve) = ops::resolve_ws(&self.ws, dry_run)?;
ops::check_yanked(
self.ws.gctx(),
&pkg_set,
Expand Down
2 changes: 2 additions & 0 deletions src/cargo/ops/cargo_output_metadata.rs
Original file line number Diff line number Diff line change
Expand Up @@ -142,6 +142,7 @@ fn build_resolve_graph(

// Note that even with --filter-platform we end up downloading host dependencies as well,
// as that is the behavior of download_accessible.
let dry_run = false;
let ws_resolve = ops::resolve_ws_with_opts(
ws,
&mut target_data,
Expand All @@ -150,6 +151,7 @@ fn build_resolve_graph(
&specs,
HasDevUnits::Yes,
force_all,
dry_run,
)?;

let package_map: BTreeMap<PackageId, Package> = ws_resolve
Expand Down
3 changes: 2 additions & 1 deletion src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -190,7 +190,8 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option

if ws.root().join("Cargo.lock").exists() {
// Make sure the Cargo.lock is up-to-date and valid.
let _ = ops::resolve_ws(ws)?;
let dry_run = false;
let _ = ops::resolve_ws(ws, dry_run)?;
// If Cargo.lock does not exist, it will be generated by `build_lock`
// below, and will be validated during the verification step.
}
Expand Down
Loading

0 comments on commit fb123c6

Please sign in to comment.