Skip to content

Commit a1f47ec

Browse files
borsbors
committed
Auto merge of #14074 - tweag:verification-order, r=epage
Change verification order during packaging. Once we support packaging workspaces with dependencies, dependency packages need to be built before anything is verified. In addition to a little refactoring, this commit reorders the console messages so that package metadata (archive size, etc.) is reported before verification results. As [suggested](ecba15c#r1640182916) on #13947 this, splits out the first commit (which has a lot of test output churn) as a separate PR.
2 parents d990adb + c0287be commit a1f47ec

13 files changed

+152
-134
lines changed

‎src/cargo/ops/cargo_package.rs

+75-56
Original file line numberDiff line numberDiff line change
@@ -28,6 +28,7 @@ use tar::{Archive, Builder, EntryType, Header, HeaderMode};
2828
use tracing::debug;
2929
use unicase::Ascii as UncasedAscii;
3030

31+
#[derive(Clone)]
3132
pub struct PackageOpts<'gctx> {
3233
pub gctx: &'gctx GlobalContext,
3334
pub list: bool,
@@ -82,48 +83,39 @@ struct GitVcsInfo {
8283
sha1: String,
8384
}
8485

86+
/// Packages a single package in a workspace, returning the resulting tar file.
87+
///
88+
/// # Panics
89+
/// Panics if `opts.list` is true. In that case you probably don't want to
90+
/// actually build the package tarball; you should just make and print the list
91+
/// of files. (We don't currently provide a public API for that, but see how
92+
/// [`package`] does it.)
8593
pub fn package_one(
8694
ws: &Workspace<'_>,
8795
pkg: &Package,
8896
opts: &PackageOpts<'_>,
89-
) -> CargoResult<Option<FileLock>> {
90-
let gctx = ws.gctx();
91-
let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), gctx);
92-
src.update()?;
97+
) -> CargoResult<FileLock> {
98+
assert!(!opts.list);
9399

94-
if opts.check_metadata {
95-
check_metadata(pkg, gctx)?;
96-
}
100+
let ar_files = prepare_archive(ws, pkg, opts)?;
101+
let tarball = create_package(ws, pkg, ar_files)?;
97102

98-
if !pkg.manifest().exclude().is_empty() && !pkg.manifest().include().is_empty() {
99-
gctx.shell().warn(
100-
"both package.include and package.exclude are specified; \
101-
the exclude list will be ignored",
102-
)?;
103+
if opts.verify {
104+
run_verify(ws, pkg, &tarball, opts)?;
103105
}
104-
let src_files = src.list_files(pkg)?;
105106

106-
// Check (git) repository state, getting the current commit hash if not
107-
// dirty.
108-
let vcs_info = if !opts.allow_dirty {
109-
// This will error if a dirty repo is found.
110-
check_repo_state(pkg, &src_files, gctx)?
111-
} else {
112-
None
113-
};
114-
115-
let ar_files = build_ar_list(ws, pkg, src_files, vcs_info)?;
107+
Ok(tarball)
108+
}
116109

110+
// Builds a tarball and places it in the output directory.
111+
fn create_package(
112+
ws: &Workspace<'_>,
113+
pkg: &Package,
114+
ar_files: Vec<ArchiveFile>,
115+
) -> CargoResult<FileLock> {
116+
let gctx = ws.gctx();
117117
let filecount = ar_files.len();
118118

119-
if opts.list {
120-
for ar_file in ar_files {
121-
drop_println!(gctx, "{}", ar_file.rel_str);
122-
}
123-
124-
return Ok(None);
125-
}
126-
127119
// Check that the package dependencies are safe to deploy.
128120
for dep in pkg.dependencies() {
129121
super::check_dep_has_version(dep, false)?;
@@ -145,10 +137,6 @@ pub fn package_one(
145137
dst.file().set_len(0)?;
146138
let uncompressed_size = tar(ws, pkg, ar_files, dst.file(), &filename)
147139
.with_context(|| "failed to prepare local package for uploading")?;
148-
if opts.verify {
149-
dst.seek(SeekFrom::Start(0))?;
150-
run_verify(ws, pkg, &dst, opts).with_context(|| "failed to verify package tarball")?
151-
}
152140

153141
dst.seek(SeekFrom::Start(0))?;
154142
let src_path = dst.path();
@@ -172,7 +160,7 @@ pub fn package_one(
172160
// It doesn't really matter if this fails.
173161
drop(gctx.shell().status("Packaged", message));
174162

175-
return Ok(Some(dst));
163+
return Ok(dst);
176164
}
177165

178166
pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option<Vec<FileLock>>> {
@@ -185,7 +173,6 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
185173
}
186174
}
187175
let pkgs = ws.members_with_features(specs, &opts.cli_features)?;
188-
189176
let mut dsts = Vec::with_capacity(pkgs.len());
190177

191178
if ws.root().join("Cargo.lock").exists() {
@@ -197,25 +184,24 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
197184
}
198185

199186
for (pkg, cli_features) in pkgs {
200-
let result = package_one(
201-
ws,
202-
pkg,
203-
&PackageOpts {
204-
gctx: opts.gctx,
205-
list: opts.list,
206-
check_metadata: opts.check_metadata,
207-
allow_dirty: opts.allow_dirty,
208-
verify: opts.verify,
209-
jobs: opts.jobs.clone(),
210-
keep_going: opts.keep_going,
211-
to_package: ops::Packages::Default,
212-
targets: opts.targets.clone(),
213-
cli_features: cli_features,
214-
},
215-
)?;
187+
let opts = PackageOpts {
188+
to_package: ops::Packages::Default,
189+
cli_features,
190+
..opts.clone()
191+
};
192+
let ar_files = prepare_archive(ws, pkg, &opts)?;
216193

217-
if !opts.list {
218-
dsts.push(result.unwrap());
194+
if opts.list {
195+
for ar_file in ar_files {
196+
drop_println!(ws.gctx(), "{}", ar_file.rel_str);
197+
}
198+
} else {
199+
let tarball = create_package(ws, pkg, ar_files)?;
200+
if opts.verify {
201+
run_verify(ws, pkg, &tarball, &opts)
202+
.with_context(|| "failed to verify package tarball")?;
203+
}
204+
dsts.push(tarball);
219205
}
220206
}
221207

@@ -227,6 +213,40 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
227213
}
228214
}
229215

216+
/// Performs pre-archiving checks and builds a list of files to archive.
217+
fn prepare_archive(
218+
ws: &Workspace<'_>,
219+
pkg: &Package,
220+
opts: &PackageOpts<'_>,
221+
) -> CargoResult<Vec<ArchiveFile>> {
222+
let gctx = ws.gctx();
223+
let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), gctx);
224+
src.update()?;
225+
226+
if opts.check_metadata {
227+
check_metadata(pkg, gctx)?;
228+
}
229+
230+
if !pkg.manifest().exclude().is_empty() && !pkg.manifest().include().is_empty() {
231+
gctx.shell().warn(
232+
"both package.include and package.exclude are specified; \
233+
the exclude list will be ignored",
234+
)?;
235+
}
236+
let src_files = src.list_files(pkg)?;
237+
238+
// Check (git) repository state, getting the current commit hash if not
239+
// dirty.
240+
let vcs_info = if !opts.allow_dirty {
241+
// This will error if a dirty repo is found.
242+
check_repo_state(pkg, &src_files, gctx)?
243+
} else {
244+
None
245+
};
246+
247+
build_ar_list(ws, pkg, src_files, vcs_info)
248+
}
249+
230250
/// Builds list of files to archive.
231251
fn build_ar_list(
232252
ws: &Workspace<'_>,
@@ -236,7 +256,6 @@ fn build_ar_list(
236256
) -> CargoResult<Vec<ArchiveFile>> {
237257
let mut result = HashMap::new();
238258
let root = pkg.root();
239-
240259
for src_file in &src_files {
241260
let rel_path = src_file.strip_prefix(&root)?;
242261
check_filename(rel_path, &mut ws.gctx().shell())?;

‎src/cargo/ops/registry/publish.rs

+1-2
Original file line numberDiff line numberDiff line change
@@ -156,8 +156,7 @@ pub fn publish(ws: &Workspace<'_>, opts: &PublishOpts<'_>) -> CargoResult<()> {
156156
keep_going: opts.keep_going,
157157
cli_features,
158158
},
159-
)?
160-
.unwrap();
159+
)?;
161160

162161
if !opts.dry_run {
163162
let hash = cargo_util::Sha256::new()

‎tests/testsuite/alt_registry.rs

+3-3
Original file line numberDiff line numberDiff line change
@@ -346,13 +346,13 @@ fn publish_with_registry_dependency() {
346346
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
347347
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
348348
[UPDATING] `alternative` index
349+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
349350
[VERIFYING] foo v0.0.1 ([ROOT]/foo)
350351
[DOWNLOADING] crates ...
351352
[DOWNLOADED] bar v0.0.1 (registry `alternative`)
352353
[COMPILING] bar v0.0.1 (registry `alternative`)
353354
[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1)
354355
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
355-
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
356356
[UPLOADING] foo v0.0.1 ([ROOT]/foo)
357357
[UPLOADED] foo v0.0.1 to registry `alternative`
358358
[NOTE] waiting for `foo v0.0.1` to be available at registry `alternative`.
@@ -512,10 +512,10 @@ fn publish_to_alt_registry() {
512512
[WARNING] manifest has no description, license, license-file, documentation, homepage or repository.
513513
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
514514
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
515+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
515516
[VERIFYING] foo v0.0.1 ([ROOT]/foo)
516517
[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1)
517518
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
518-
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
519519
[UPLOADING] foo v0.0.1 ([ROOT]/foo)
520520
[UPLOADED] foo v0.0.1 to registry `alternative`
521521
[NOTE] waiting for `foo v0.0.1` to be available at registry `alternative`.
@@ -591,13 +591,13 @@ fn publish_with_crates_io_dep() {
591591
See https://doc.rust-lang.org/cargo/reference/manifest.html#package-metadata for more info.
592592
[PACKAGING] foo v0.0.1 ([ROOT]/foo)
593593
[UPDATING] `dummy-registry` index
594+
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
594595
[VERIFYING] foo v0.0.1 ([ROOT]/foo)
595596
[DOWNLOADING] crates ...
596597
[DOWNLOADED] bar v0.0.1 (registry `dummy-registry`)
597598
[COMPILING] bar v0.0.1
598599
[COMPILING] foo v0.0.1 ([ROOT]/foo/target/package/foo-0.0.1)
599600
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [ELAPSED]s
600-
[PACKAGED] 4 files, [FILE_SIZE]B ([FILE_SIZE]B compressed)
601601
[UPLOADING] foo v0.0.1 ([ROOT]/foo)
602602
[UPLOADED] foo v0.0.1 to registry `alternative`
603603
[NOTE] waiting for `foo v0.0.1` to be available at registry `alternative`.

‎tests/testsuite/cargo_features.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -664,10 +664,10 @@ fn publish_allowed() {
664664
[WARNING] [..]
665665
[..]
666666
[PACKAGING] a v0.0.1 [..]
667+
[PACKAGED] [..]
667668
[VERIFYING] a v0.0.1 [..]
668669
[COMPILING] a v0.0.1 [..]
669670
[FINISHED] [..]
670-
[PACKAGED] [..]
671671
[UPLOADING] a v0.0.1 [..]
672672
[UPLOADED] a v0.0.1 to registry `crates-io`
673673
[NOTE] waiting for `a v0.0.1` to be available at registry `crates-io`.

‎tests/testsuite/cross_publish.rs

+2-2
Original file line numberDiff line numberDiff line change
@@ -46,10 +46,10 @@ fn simple_cross_package() {
4646
.with_stderr(
4747
"\
4848
[PACKAGING] foo v0.0.0 ([CWD])
49+
[PACKAGED] 4 files, [..] ([..] compressed)
4950
[VERIFYING] foo v0.0.0 ([CWD])
5051
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
5152
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
52-
[PACKAGED] 4 files, [..] ([..] compressed)
5353
",
5454
)
5555
.run();
@@ -111,10 +111,10 @@ fn publish_with_target() {
111111
"\
112112
[UPDATING] crates.io index
113113
[PACKAGING] foo v0.0.0 ([CWD])
114+
[PACKAGED] [..]
114115
[VERIFYING] foo v0.0.0 ([CWD])
115116
[COMPILING] foo v0.0.0 ([CWD]/target/package/foo-0.0.0)
116117
[FINISHED] `dev` profile [unoptimized + debuginfo] target(s) in [..]
117-
[PACKAGED] [..]
118118
[UPLOADING] foo v0.0.0 ([CWD])
119119
[UPLOADED] foo v0.0.0 to registry `crates-io`
120120
[NOTE] waiting [..]

‎tests/testsuite/features_namespaced.rs

+1-1
Original file line numberDiff line numberDiff line change
@@ -1055,11 +1055,11 @@ fn publish() {
10551055
"\
10561056
[UPDATING] [..]
10571057
[PACKAGING] foo v0.1.0 [..]
1058+
[PACKAGED] [..]
10581059
[VERIFYING] foo v0.1.0 [..]
10591060
[UPDATING] [..]
10601061
[COMPILING] foo v0.1.0 [..]
10611062
[FINISHED] [..]
1062-
[PACKAGED] [..]
10631063
[UPLOADING] foo v0.1.0 [..]
10641064
[UPLOADED] foo v0.1.0 [..]
10651065
[NOTE] waiting [..]

‎tests/testsuite/inheritable_workspace_fields.rs

+5-5
Original file line numberDiff line numberDiff line change
@@ -164,10 +164,10 @@ fn inherit_own_workspace_fields() {
164164
[UPDATING] [..]
165165
[WARNING] [..]
166166
[..]
167+
[PACKAGED] [..]
167168
[VERIFYING] foo v1.2.3 [..]
168169
[COMPILING] foo v1.2.3 [..]
169170
[FINISHED] [..]
170-
[PACKAGED] [..]
171171
[UPLOADING] foo v1.2.3 [..]
172172
[UPLOADED] foo v1.2.3 to registry `crates-io`
173173
[NOTE] waiting for `foo v1.2.3` to be available at registry `crates-io`.
@@ -319,11 +319,11 @@ fn inherit_own_dependencies() {
319319
[..]
320320
[PACKAGING] bar v0.2.0 [..]
321321
[UPDATING] [..]
322+
[PACKAGED] [..]
322323
[VERIFYING] bar v0.2.0 [..]
323324
[COMPILING] dep v0.1.2
324325
[COMPILING] bar v0.2.0 [..]
325326
[FINISHED] [..]
326-
[PACKAGED] [..]
327327
[UPLOADING] bar v0.2.0 [..]
328328
[UPLOADED] bar v0.2.0 to registry `crates-io`
329329
[NOTE] waiting for `bar v0.2.0` to be available at registry `crates-io`.
@@ -478,11 +478,11 @@ fn inherit_own_detailed_dependencies() {
478478
[..]
479479
[PACKAGING] bar v0.2.0 [..]
480480
[UPDATING] [..]
481+
[PACKAGED] [..]
481482
[VERIFYING] bar v0.2.0 [..]
482483
[COMPILING] dep v0.1.2
483484
[COMPILING] bar v0.2.0 [..]
484485
[FINISHED] [..]
485-
[PACKAGED] [..]
486486
[UPLOADING] bar v0.2.0 [..]
487487
[UPLOADED] bar v0.2.0 to registry `crates-io`
488488
[NOTE] waiting for `bar v0.2.0` to be available at registry `crates-io`.
@@ -726,14 +726,14 @@ fn inherit_workspace_fields() {
726726
[UPDATING] [..]
727727
[WARNING] [..]
728728
[..]
729+
[PACKAGED] [..]
729730
[VERIFYING] bar v1.2.3 [..]
730731
[WARNING] [..]
731732
[..]
732733
[..]
733734
[..]
734735
[COMPILING] bar v1.2.3 [..]
735736
[FINISHED] [..]
736-
[PACKAGED] [..]
737737
[UPLOADING] bar v1.2.3 [..]
738738
[UPLOADED] bar v1.2.3 to registry `crates-io`
739739
[NOTE] waiting for `bar v1.2.3` to be available at registry `crates-io`.
@@ -892,11 +892,11 @@ fn inherit_dependencies() {
892892
[..]
893893
[PACKAGING] bar v0.2.0 [..]
894894
[UPDATING] [..]
895+
[PACKAGED] [..]
895896
[VERIFYING] bar v0.2.0 [..]
896897
[COMPILING] dep v0.1.2
897898
[COMPILING] bar v0.2.0 [..]
898899
[FINISHED] [..]
899-
[PACKAGED] [..]
900900
[UPLOADING] bar v0.2.0 [..]
901901
[UPLOADED] bar v0.2.0 to registry `crates-io`
902902
[NOTE] waiting for `bar v0.2.0` to be available at registry `crates-io`.

0 commit comments

Comments
 (0)