Skip to content

Commit

Permalink
Auto merge of #8175 - ehuss:allow-package-list, r=alexcrichton
Browse files Browse the repository at this point in the history
Allow `cargo package --list` even for things that don't package.

`cargo package --list` was changed in #7905 to generate `Cargo.lock` earlier. If there is a problem, then it would fail where previously it would succeed. This changes it so that file generation is deferred until after `--list`.

This also changes it so that the "dependencies must have a version" check is deferred until after `--list` as well.

Closes #8151
  • Loading branch information
bors committed Apr 28, 2020
2 parents 90931d9 + 156c651 commit ade4c7b
Show file tree
Hide file tree
Showing 4 changed files with 86 additions and 20 deletions.
32 changes: 22 additions & 10 deletions src/cargo/ops/cargo_package.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,8 +50,17 @@ struct ArchiveFile {
enum FileContents {
/// Absolute path to the file on disk to add to the archive.
OnDisk(PathBuf),
/// Contents of a file generated in memory.
Generated(String),
/// Generates a file.
Generated(GeneratedFile),
}

enum GeneratedFile {
/// Generates `Cargo.toml` by rewriting the original.
Manifest,
/// Generates `Cargo.lock` in some cases (like if there is a binary).
Lockfile,
/// Adds a `.cargo-vcs_info.json` file if in a (clean) git repo.
VcsInfo(String),
}

pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option<FileLock>> {
Expand All @@ -71,8 +80,6 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
check_metadata(pkg, config)?;
}

verify_dependencies(pkg)?;

if !pkg.manifest().exclude().is_empty() && !pkg.manifest().include().is_empty() {
config.shell().warn(
"both package.include and package.exclude are specified; \
Expand Down Expand Up @@ -100,6 +107,8 @@ pub fn package(ws: &Workspace<'_>, opts: &PackageOpts<'_>) -> CargoResult<Option
return Ok(None);
}

verify_dependencies(pkg)?;

let filename = format!("{}-{}.crate", pkg.name(), pkg.version());
let dir = ws.target_dir().join("package");
let mut dst = {
Expand Down Expand Up @@ -156,11 +165,10 @@ fn build_ar_list(
rel_str: "Cargo.toml.orig".to_string(),
contents: FileContents::OnDisk(src_file),
});
let generated = pkg.to_registry_toml(ws)?;
result.push(ArchiveFile {
rel_path,
rel_str,
contents: FileContents::Generated(generated),
contents: FileContents::Generated(GeneratedFile::Manifest),
});
}
"Cargo.lock" => continue,
Expand All @@ -179,18 +187,17 @@ fn build_ar_list(
}
}
if pkg.include_lockfile() {
let new_lock = build_lock(ws)?;
result.push(ArchiveFile {
rel_path: PathBuf::from("Cargo.lock"),
rel_str: "Cargo.lock".to_string(),
contents: FileContents::Generated(new_lock),
contents: FileContents::Generated(GeneratedFile::Lockfile),
});
}
if let Some(vcs_info) = vcs_info {
result.push(ArchiveFile {
rel_path: PathBuf::from(VCS_INFO_FILE),
rel_str: VCS_INFO_FILE.to_string(),
contents: FileContents::Generated(vcs_info),
contents: FileContents::Generated(GeneratedFile::VcsInfo(vcs_info)),
});
}
if let Some(license_file) = &pkg.manifest().metadata().license_file {
Expand Down Expand Up @@ -530,7 +537,12 @@ fn tar(
format!("could not archive source file `{}`", disk_path.display())
})?;
}
FileContents::Generated(contents) => {
FileContents::Generated(generated_kind) => {
let contents = match generated_kind {
GeneratedFile::Manifest => pkg.to_registry_toml(ws)?,
GeneratedFile::Lockfile => build_lock(ws)?,
GeneratedFile::VcsInfo(s) => s,
};
header.set_entry_type(EntryType::file());
header.set_mode(0o644);
header.set_mtime(
Expand Down
55 changes: 52 additions & 3 deletions tests/testsuite/package.rs
Original file line number Diff line number Diff line change
@@ -1,10 +1,10 @@
//! Tests for the `cargo package` command.
use cargo_test_support::paths::CargoPathExt;
use cargo_test_support::registry::Package;
use cargo_test_support::publish::validate_crate_contents;
use cargo_test_support::registry::{self, Package};
use cargo_test_support::{
basic_manifest, cargo_process, git, path2url, paths, project, publish::validate_crate_contents,
registry, symlink_supported, t,
basic_manifest, cargo_process, git, path2url, paths, project, symlink_supported, t,
};
use std::fs::{self, read_to_string, File};
use std::path::Path;
Expand Down Expand Up @@ -1796,3 +1796,52 @@ Caused by:
)
.run();
}

#[cargo_test]
fn list_with_path_and_lock() {
// Allow --list even for something that isn't packageable.

// Init an empty registry because a versionless path dep will search for
// the package on crates.io.
registry::init();
let p = project()
.file(
"Cargo.toml",
r#"
[package]
name = "foo"
version = "0.1.0"
license = "MIT"
description = "foo"
homepage = "foo"
[dependencies]
bar = {path="bar"}
"#,
)
.file("src/main.rs", "fn main() {}")
.file("bar/Cargo.toml", &basic_manifest("bar", "0.1.0"))
.file("bar/src/lib.rs", "")
.build();

p.cargo("package --list")
.with_stdout(
"\
Cargo.lock
Cargo.toml
Cargo.toml.orig
src/main.rs
",
)
.run();

p.cargo("package")
.with_status(101)
.with_stderr(
"\
error: all path dependencies must have a version specified when packaging.
dependency `bar` does not specify a version.
",
)
.run();
}
4 changes: 2 additions & 2 deletions tests/testsuite/publish_lockfile.rs
Original file line number Diff line number Diff line change
Expand Up @@ -297,8 +297,8 @@ fn no_warn_workspace_extras() {
.cwd("a")
.with_stderr(
"\
[UPDATING] `[..]` index
[PACKAGING] a v0.1.0 ([..])
[UPDATING] `[..]` index
",
)
.run();
Expand Down Expand Up @@ -328,10 +328,10 @@ fn warn_package_with_yanked() {
p.cargo("package --no-verify")
.with_stderr(
"\
[PACKAGING] foo v0.0.1 ([..])
[UPDATING] `[..]` index
[WARNING] package `bar v0.1.0` in Cargo.lock is yanked in registry \
`crates.io`, consider updating to a version that is not yanked
[PACKAGING] foo v0.0.1 ([..])
",
)
.run();
Expand Down
15 changes: 10 additions & 5 deletions tests/testsuite/registry.rs
Original file line number Diff line number Diff line change
Expand Up @@ -359,13 +359,18 @@ fn package_with_path_deps() {
.file("notyet/src/lib.rs", "")
.build();

p.cargo("package -v")
p.cargo("package")
.with_status(101)
.with_stderr_contains(
"\
[ERROR] no matching package named `notyet` found
location searched: registry [..]
required by package `foo v0.0.1 ([..])`
[PACKAGING] foo [..]
[UPDATING] [..]
[ERROR] failed to prepare local package for uploading
Caused by:
no matching package named `notyet` found
location searched: registry `https://github.com/rust-lang/crates.io-index`
required by package `foo v0.0.1 [..]`
",
)
.run();
Expand All @@ -375,8 +380,8 @@ required by package `foo v0.0.1 ([..])`
p.cargo("package")
.with_stderr(
"\
[UPDATING] `[..]` index
[PACKAGING] foo v0.0.1 ([CWD])
[UPDATING] `[..]` index
[VERIFYING] foo v0.0.1 ([CWD])
[DOWNLOADING] crates ...
[DOWNLOADED] notyet v0.0.1 (registry `[ROOT][..]`)
Expand Down

0 comments on commit ade4c7b

Please sign in to comment.