Skip to content

Commit f013ef5

Browse files
authored
Override Cargo.lock checksums when doing a dry-run publish (#15711)
Fixes #15647. When dry-run publishing workspace without bumping versions first, the package-verification step would fail because it would see checksum mismatches between the old lock file (that saw index deps) and the new lock file where those index deps got replaced by local packages with the same version. In this PR, the packaging step modifies the old lock file's checksums before re-resolving, but only in dry-run mode.
2 parents 4a137fa + 340a4f9 commit f013ef5

File tree

6 files changed

+199
-4
lines changed

6 files changed

+199
-4
lines changed

src/bin/cargo/commands/package.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -108,6 +108,7 @@ pub fn exec(gctx: &mut GlobalContext, args: &ArgMatches) -> CliResult {
108108
keep_going: args.keep_going(),
109109
cli_features: args.cli_features()?,
110110
reg_or_index,
111+
dry_run: false,
111112
},
112113
)?;
113114

src/cargo/core/resolver/resolve.rs

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -390,6 +390,10 @@ unable to verify that `{0}` is the same as when the lockfile was generated
390390
&self.checksums
391391
}
392392

393+
pub fn set_checksum(&mut self, pkg_id: PackageId, checksum: String) {
394+
self.checksums.insert(pkg_id, Some(checksum));
395+
}
396+
393397
pub fn metadata(&self) -> &Metadata {
394398
&self.metadata
395399
}

src/cargo/ops/cargo_package/mod.rs

Lines changed: 40 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -86,6 +86,19 @@ pub struct PackageOpts<'gctx> {
8686
pub targets: Vec<String>,
8787
pub cli_features: CliFeatures,
8888
pub reg_or_index: Option<ops::RegistryOrIndex>,
89+
/// Whether this packaging job is meant for a publishing dry-run.
90+
///
91+
/// Packaging on its own has no side effects, so a dry-run doesn't
92+
/// make sense from that point of view. But dry-run publishing needs
93+
/// special packaging behavior, which this flag turns on.
94+
///
95+
/// Specifically, we want dry-run packaging to work even if versions
96+
/// have not yet been bumped. But then if you dry-run packaging in
97+
/// a workspace with some declared versions that are already published,
98+
/// the package verification step can fail with checksum mismatches.
99+
/// So when dry-run is true, the verification step does some extra
100+
/// checksum fudging in the lock file.
101+
pub dry_run: bool,
89102
}
90103

91104
const ORIGINAL_MANIFEST_FILE: &str = "Cargo.toml.orig";
@@ -125,6 +138,7 @@ enum GeneratedFile {
125138
#[tracing::instrument(skip_all)]
126139
fn create_package(
127140
ws: &Workspace<'_>,
141+
opts: &PackageOpts<'_>,
128142
pkg: &Package,
129143
ar_files: Vec<ArchiveFile>,
130144
local_reg: Option<&TmpRegistry<'_>>,
@@ -159,7 +173,7 @@ fn create_package(
159173
gctx.shell()
160174
.status("Packaging", pkg.package_id().to_string())?;
161175
dst.file().set_len(0)?;
162-
let uncompressed_size = tar(ws, pkg, local_reg, ar_files, dst.file(), &filename)
176+
let uncompressed_size = tar(ws, opts, pkg, local_reg, ar_files, dst.file(), &filename)
163177
.context("failed to prepare local package for uploading")?;
164178

165179
dst.seek(SeekFrom::Start(0))?;
@@ -311,7 +325,7 @@ fn do_package<'a>(
311325
}
312326
}
313327
} else {
314-
let tarball = create_package(ws, &pkg, ar_files, local_reg.as_ref())?;
328+
let tarball = create_package(ws, &opts, &pkg, ar_files, local_reg.as_ref())?;
315329
if let Some(local_reg) = local_reg.as_mut() {
316330
if pkg.publish() != &Some(Vec::new()) {
317331
local_reg.add_package(ws, &pkg, &tarball)?;
@@ -720,11 +734,12 @@ fn error_custom_build_file_not_in_package(
720734
/// Construct `Cargo.lock` for the package to be published.
721735
fn build_lock(
722736
ws: &Workspace<'_>,
737+
opts: &PackageOpts<'_>,
723738
publish_pkg: &Package,
724739
local_reg: Option<&TmpRegistry<'_>>,
725740
) -> CargoResult<String> {
726741
let gctx = ws.gctx();
727-
let orig_resolve = ops::load_pkg_lockfile(ws)?;
742+
let mut orig_resolve = ops::load_pkg_lockfile(ws)?;
728743

729744
let mut tmp_ws = Workspace::ephemeral(publish_pkg.clone(), ws.gctx(), None, true)?;
730745

@@ -736,6 +751,18 @@ fn build_lock(
736751
local_reg.upstream,
737752
local_reg.root.as_path_unlocked().to_owned(),
738753
);
754+
if opts.dry_run {
755+
if let Some(orig_resolve) = orig_resolve.as_mut() {
756+
let upstream_in_lock = if local_reg.upstream.is_crates_io() {
757+
SourceId::crates_io(gctx)?
758+
} else {
759+
local_reg.upstream
760+
};
761+
for (p, s) in local_reg.checksums() {
762+
orig_resolve.set_checksum(p.with_source_id(upstream_in_lock), s.to_owned());
763+
}
764+
}
765+
}
739766
}
740767
let mut tmp_reg = tmp_ws.package_registry()?;
741768

@@ -811,6 +838,7 @@ fn check_metadata(pkg: &Package, gctx: &GlobalContext) -> CargoResult<()> {
811838
/// Returns the uncompressed size of the contents of the new archive file.
812839
fn tar(
813840
ws: &Workspace<'_>,
841+
opts: &PackageOpts<'_>,
814842
pkg: &Package,
815843
local_reg: Option<&TmpRegistry<'_>>,
816844
ar_files: Vec<ArchiveFile>,
@@ -868,7 +896,7 @@ fn tar(
868896
GeneratedFile::Manifest(_) => {
869897
publish_pkg.manifest().to_normalized_contents()?
870898
}
871-
GeneratedFile::Lockfile(_) => build_lock(ws, &publish_pkg, local_reg)?,
899+
GeneratedFile::Lockfile(_) => build_lock(ws, opts, &publish_pkg, local_reg)?,
872900
GeneratedFile::VcsInfo(ref s) => serde_json::to_string_pretty(s)?,
873901
};
874902
header.set_entry_type(EntryType::file());
@@ -1062,6 +1090,7 @@ struct TmpRegistry<'a> {
10621090
gctx: &'a GlobalContext,
10631091
upstream: SourceId,
10641092
root: Filesystem,
1093+
checksums: HashMap<PackageId, String>,
10651094
_lock: FileLock,
10661095
}
10671096

@@ -1073,6 +1102,7 @@ impl<'a> TmpRegistry<'a> {
10731102
gctx,
10741103
root,
10751104
upstream,
1105+
checksums: HashMap::new(),
10761106
_lock,
10771107
};
10781108
// If there's an old temporary registry, delete it.
@@ -1118,6 +1148,8 @@ impl<'a> TmpRegistry<'a> {
11181148
.update_file(tar.file())?
11191149
.finish_hex();
11201150

1151+
self.checksums.insert(package.package_id(), cksum.clone());
1152+
11211153
let deps: Vec<_> = new_crate
11221154
.deps
11231155
.into_iter()
@@ -1178,4 +1210,8 @@ impl<'a> TmpRegistry<'a> {
11781210
dst.write_all(index_line.as_bytes())?;
11791211
Ok(())
11801212
}
1213+
1214+
fn checksums(&self) -> impl Iterator<Item = (PackageId, &str)> {
1215+
self.checksums.iter().map(|(p, s)| (*p, s.as_str()))
1216+
}
11811217
}

src/cargo/ops/registry/publish.rs

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -202,6 +202,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
202202
keep_going: opts.keep_going,
203203
cli_features: opts.cli_features.clone(),
204204
reg_or_index: reg_or_index.clone(),
205+
dry_run: opts.dry_run,
205206
},
206207
pkgs,
207208
)?;

tests/testsuite/package.rs

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8082,3 +8082,80 @@ fn unpublished_dependency() {
80828082
(),
80838083
);
80848084
}
8085+
8086+
// This is a companion to `publish::checksum_changed`, but because this one
8087+
// is packaging without dry-run, it should fail.
8088+
#[cargo_test]
8089+
fn checksum_changed() {
8090+
let registry = registry::RegistryBuilder::new()
8091+
.http_api()
8092+
.http_index()
8093+
.build();
8094+
8095+
Package::new("dep", "1.0.0").publish();
8096+
Package::new("transitive", "1.0.0")
8097+
.dep("dep", "1.0.0")
8098+
.publish();
8099+
8100+
let p = project()
8101+
.file(
8102+
"Cargo.toml",
8103+
r#"
8104+
[workspace]
8105+
members = ["dep"]
8106+
8107+
[package]
8108+
name = "foo"
8109+
version = "0.0.1"
8110+
edition = "2015"
8111+
authors = []
8112+
license = "MIT"
8113+
description = "foo"
8114+
documentation = "foo"
8115+
8116+
[dependencies]
8117+
dep = { path = "./dep", version = "1.0.0" }
8118+
transitive = "1.0.0"
8119+
"#,
8120+
)
8121+
.file("src/lib.rs", "")
8122+
.file(
8123+
"dep/Cargo.toml",
8124+
r#"
8125+
[package]
8126+
name = "dep"
8127+
version = "1.0.0"
8128+
edition = "2015"
8129+
"#,
8130+
)
8131+
.file("dep/src/lib.rs", "")
8132+
.build();
8133+
8134+
p.cargo("check").run();
8135+
8136+
p.cargo("package --workspace -Zpackage-workspace")
8137+
.masquerade_as_nightly_cargo(&["package-workspace"])
8138+
.replace_crates_io(registry.index_url())
8139+
.with_status(101)
8140+
.with_stderr_data(str![[r#"
8141+
[WARNING] manifest has no description, license, license-file, documentation, homepage or repository.
8142+
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
8143+
[PACKAGING] dep v1.0.0 ([ROOT]/foo/dep)
8144+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
8145+
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
8146+
[ERROR] failed to prepare local package for uploading
8147+
8148+
Caused by:
8149+
checksum for `dep v1.0.0` changed between lock files
8150+
8151+
this could be indicative of a few possible errors:
8152+
8153+
* the lock file is corrupt
8154+
* a replacement source in use (e.g., a mirror) returned a different checksum
8155+
* the source itself may be corrupt in one way or another
8156+
8157+
unable to verify that `dep v1.0.0` is the same as when the lockfile was generated
8158+
8159+
"#]])
8160+
.run();
8161+
}

tests/testsuite/publish.rs

Lines changed: 76 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -4378,3 +4378,79 @@ fn all_unpublishable_packages() {
43784378
"#]])
43794379
.run();
43804380
}
4381+
4382+
#[cargo_test]
4383+
fn checksum_changed() {
4384+
let registry = RegistryBuilder::new().http_api().http_index().build();
4385+
4386+
Package::new("dep", "1.0.0").publish();
4387+
Package::new("transitive", "1.0.0")
4388+
.dep("dep", "1.0.0")
4389+
.publish();
4390+
4391+
let p = project()
4392+
.file(
4393+
"Cargo.toml",
4394+
r#"
4395+
[workspace]
4396+
members = ["dep"]
4397+
4398+
[package]
4399+
name = "foo"
4400+
version = "0.0.1"
4401+
edition = "2015"
4402+
authors = []
4403+
license = "MIT"
4404+
description = "foo"
4405+
documentation = "foo"
4406+
4407+
[dependencies]
4408+
dep = { path = "./dep", version = "1.0.0" }
4409+
transitive = "1.0.0"
4410+
"#,
4411+
)
4412+
.file("src/lib.rs", "")
4413+
.file(
4414+
"dep/Cargo.toml",
4415+
r#"
4416+
[package]
4417+
name = "dep"
4418+
version = "1.0.0"
4419+
edition = "2015"
4420+
"#,
4421+
)
4422+
.file("dep/src/lib.rs", "")
4423+
.build();
4424+
4425+
p.cargo("check").run();
4426+
4427+
p.cargo("publish --dry-run --workspace -Zpackage-workspace")
4428+
.masquerade_as_nightly_cargo(&["package-workspace"])
4429+
.replace_crates_io(registry.index_url())
4430+
.with_stderr_data(str![[r#"
4431+
[UPDATING] crates.io index
4432+
[WARNING] crate dep@1.0.0 already exists on crates.io index
4433+
[WARNING] manifest has no description, license, license-file, documentation, homepage or repository.
4434+
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
4435+
[PACKAGING] dep v1.0.0 ([ROOT]/foo/dep)
4436+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
4437+
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
4438+
[UPDATING] crates.io index
4439+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
4440+
[VERIFYING] dep v1.0.0 ([ROOT]/foo/dep)
4441+
[COMPILING] dep v1.0.0 ([ROOT]/foo/target/package/dep-1.0.0)
4442+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
4443+
[VERIFYING] foo v0.0.1 ([ROOT]/foo)
4444+
[UNPACKING] dep v1.0.0 (registry `[ROOT]/foo/target/package/tmp-registry`)
4445+
[COMPILING] dep v1.0.0
4446+
[COMPILING] transitive v1.0.0
4447+
[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1)
4448+
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
4449+
[UPLOADING] dep v1.0.0 ([ROOT]/foo/dep)
4450+
[WARNING] aborting upload due to dry run
4451+
[UPLOADING] foo v0.0.1 ([ROOT]/foo)
4452+
[WARNING] aborting upload due to dry run
4453+
4454+
"#]])
4455+
.run();
4456+
}

0 commit comments

Comments
 (0)