Skip to content

Commit a3ccf83

Browse files
committed
Refactor verification order
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.
1 parent 4dcbca1 commit a3ccf83

13 files changed

+140
-136
lines changed

src/cargo/ops/cargo_package.rs

+62-57
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,33 @@ struct GitVcsInfo {
8283
sha1: String,
8384
}
8485

86+
/// Packages a single package in a workspace, returning the resulting tar file.
87+
///
88+
/// Ignores `opts.list`.
8589
pub fn package_one(
8690
ws: &Workspace<'_>,
8791
pkg: &Package,
8892
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()?;
93+
) -> CargoResult<FileLock> {
94+
let ar_files = build_ar_list(ws, pkg, opts)?;
95+
let tarball = create_package(ws, pkg, ar_files)?;
9396

94-
if opts.check_metadata {
95-
check_metadata(pkg, gctx)?;
96-
}
97-
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-
)?;
97+
if opts.verify {
98+
run_verify(ws, pkg, &tarball, opts)?;
10399
}
104-
let src_files = src.list_files(pkg)?;
105-
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-
};
114100

115-
let ar_files = build_ar_list(ws, pkg, src_files, vcs_info)?;
101+
Ok(tarball)
102+
}
116103

104+
// Builds a tarball and places it in the output directory.
105+
fn create_package(
106+
ws: &Workspace<'_>,
107+
pkg: &Package,
108+
ar_files: Vec<ArchiveFile>,
109+
) -> CargoResult<FileLock> {
110+
let gctx = ws.gctx();
117111
let filecount = ar_files.len();
118112

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-
127113
// Check that the package dependencies are safe to deploy.
128114
for dep in pkg.dependencies() {
129115
super::check_dep_has_version(dep, false)?;
@@ -145,10 +131,6 @@ pub fn package_one(
145131
dst.file().set_len(0)?;
146132
let uncompressed_size = tar(ws, pkg, ar_files, dst.file(), &filename)
147133
.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-
}
152134

153135
dst.seek(SeekFrom::Start(0))?;
154136
let src_path = dst.path();
@@ -172,7 +154,7 @@ pub fn package_one(
172154
// It doesn't really matter if this fails.
173155
drop(gctx.shell().status("Packaged", message));
174156

175-
return Ok(Some(dst));
157+
return Ok(dst);
176158
}
177159

178160
pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option<Vec<FileLock>>> {
@@ -197,25 +179,24 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
197179
}
198180

199181
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-
)?;
182+
let opts = PackageOpts {
183+
to_package: ops::Packages::Default,
184+
cli_features,
185+
..opts.clone()
186+
};
187+
let ar_files = build_ar_list(ws, pkg, &opts)?;
216188

217-
if !opts.list {
218-
dsts.push(result.unwrap());
189+
if opts.list {
190+
for ar_file in ar_files {
191+
drop_println!(ws.gctx(), "{}", ar_file.rel_str);
192+
}
193+
} else {
194+
let tarball = create_package(ws, pkg, ar_files)?;
195+
if opts.verify {
196+
run_verify(ws, pkg, &tarball, &opts)
197+
.with_context(|| "failed to verify package tarball")?;
198+
}
199+
dsts.push(tarball);
219200
}
220201
}
221202

@@ -231,9 +212,33 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
231212
fn build_ar_list(
232213
ws: &Workspace<'_>,
233214
pkg: &Package,
234-
src_files: Vec<PathBuf>,
235-
vcs_info: Option<VcsInfo>,
215+
opts: &PackageOpts<'_>,
236216
) -> CargoResult<Vec<ArchiveFile>> {
217+
let gctx = ws.gctx();
218+
let mut src = PathSource::new(pkg.root(), pkg.package_id().source_id(), gctx);
219+
src.update()?;
220+
221+
if opts.check_metadata {
222+
check_metadata(pkg, gctx)?;
223+
}
224+
225+
if !pkg.manifest().exclude().is_empty() && !pkg.manifest().include().is_empty() {
226+
gctx.shell().warn(
227+
"both package.include and package.exclude are specified; \
228+
the exclude list will be ignored",
229+
)?;
230+
}
231+
let src_files = src.list_files(pkg)?;
232+
233+
// Check (git) repository state, getting the current commit hash if not
234+
// dirty.
235+
let vcs_info = if !opts.allow_dirty {
236+
// This will error if a dirty repo is found.
237+
check_repo_state(pkg, &src_files, gctx)?
238+
} else {
239+
None
240+
};
241+
237242
let mut result = HashMap::new();
238243
let root = pkg.root();
239244

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)