Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add '--to-lockfile' flag to cargo-upgrade #337

Merged
merged 3 commits into from
Oct 27, 2019
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
162 changes: 117 additions & 45 deletions src/bin/upgrade/main.rs
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,12 @@ Dev, build, and all target dependencies will also be upgraded. Only dependencies
supported. Git/path dependencies will be ignored.

All packages in the workspace will be upgraded if the `--all` flag is supplied. The `--all` flag may
be supplied in the presence of a virtual manifest."
be supplied in the presence of a virtual manifest.

If the '--to-lockfile' flag is supplied, all dependencies will be upgraded to the currently locked
version as recorded in the Cargo.lock file. This flag requires that the Cargo.lock file is
up-to-date. If the lock file is missing, or it needs to be updated, cargo-upgrade will exit with an
error. If the '--to-lockfile' flag is supplied then the network won't be accessed."
)]
Upgrade(Args),
}
Expand Down Expand Up @@ -84,11 +89,42 @@ struct Args {
/// Run without accessing the network
#[structopt(long = "offline")]
pub offline: bool,

/// Upgrade all packages to the version in the lockfile.
#[structopt(long = "to-lockfile", conflicts_with = "dependency")]
pub to_lockfile: bool,
}

/// A collection of manifests.
struct Manifests(Vec<(LocalManifest, cargo_metadata::Package)>);

/// Helper function to check whether a `cargo_metadata::Dependency` is a version dependency.
fn is_version_dep(dependency: &cargo_metadata::Dependency) -> bool {
match dependency.source {
// This is the criterion cargo uses (in `SourceId::from_url`) to decide whether a
// dependency has the 'registry' kind.
Some(ref s) => s.splitn(2, '+').next() == Some("registry"),
_ => false,
}
}

fn dry_run_message() -> Result<()> {
let bufwtr = BufferWriter::stdout(ColorChoice::Always);
let mut buffer = bufwtr.buffer();
buffer
.set_color(ColorSpec::new().set_fg(Some(Color::Cyan)).set_bold(true))
.chain_err(|| "Failed to set output colour")?;
write!(&mut buffer, "Starting dry run. ").chain_err(|| "Failed to write dry run message")?;
buffer
.set_color(&ColorSpec::new())
.chain_err(|| "Failed to clear output colour")?;
writeln!(&mut buffer, "Changes will not be saved.")
.chain_err(|| "Failed to write dry run message")?;
bufwtr
.print(&buffer)
.chain_err(|| "Failed to print dry run message")
}

impl Manifests {
/// Get all manifests in the workspace.
fn get_all(manifest_path: &Option<PathBuf>) -> Result<Self> {
Expand Down Expand Up @@ -145,16 +181,6 @@ impl Manifests {
/// Get the the combined set of dependencies to upgrade. If the user has specified
/// per-dependency desired versions, extract those here.
fn get_dependencies(&self, only_update: Vec<String>) -> Result<DesiredUpgrades> {
/// Helper function to check whether a `cargo_metadata::Dependency` is a version dependency.
fn is_version_dep(dependency: &cargo_metadata::Dependency) -> bool {
match dependency.source {
// This is the criterion cargo uses (in `SourceId::from_url`) to decide whether a
// dependency has the 'registry' kind.
Some(ref s) => s.splitn(2, '+').next() == Some("registry"),
_ => false,
}
}

// Map the names of user-specified dependencies to the (optionally) requested version.
let selected_dependencies = only_update
.into_iter()
Expand Down Expand Up @@ -203,21 +229,7 @@ impl Manifests {
/// Upgrade the manifests on disk following the previously-determined upgrade schema.
fn upgrade(self, upgraded_deps: &ActualUpgrades, dry_run: bool) -> Result<()> {
if dry_run {
let bufwtr = BufferWriter::stdout(ColorChoice::Always);
let mut buffer = bufwtr.buffer();
buffer
.set_color(ColorSpec::new().set_fg(Some(Color::Cyan)).set_bold(true))
.chain_err(|| "Failed to set output colour")?;
write!(&mut buffer, "Starting dry run. ")
.chain_err(|| "Failed to write dry run message")?;
buffer
.set_color(&ColorSpec::new())
.chain_err(|| "Failed to clear output colour")?;
writeln!(&mut buffer, "Changes will not be saved.")
.chain_err(|| "Failed to write dry run message")?;
bufwtr
.print(&buffer)
.chain_err(|| "Failed to print dry run message")?;
dry_run_message()?;
}

for (mut manifest, package) in self.0 {
Expand All @@ -234,6 +246,61 @@ impl Manifests {

Ok(())
}

/// Update dependencies in Cargo.toml file(s) to match the corresponding
/// version in Cargo.lock.
fn sync_to_lockfile(self, dry_run: bool) -> Result<()> {
// Get locked dependencies. For workspaces with multiple Cargo.toml
// files, there is only a single lockfile, so it suffices to get
// metadata for any one of Cargo.toml files.
let (manifest, _package) =
self.0.iter().next().ok_or_else(|| {
ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
})?;
let mut cmd = cargo_metadata::MetadataCommand::new();
cmd.manifest_path(manifest.path.clone());
cmd.other_options(vec!["--locked".to_string()]);

let result = cmd
.exec()
.map_err(|e| Error::from(e.compat()).chain_err(|| "Invalid manifest"))?;

let locked = result
.packages
.into_iter()
.filter(|p| p.source.is_some()) // Source is none for local packages
.collect::<Vec<_>>();

if dry_run {
dry_run_message()?;
}

for (mut manifest, package) in self.0 {
println!("{}:", package.name);

// Upgrade the manifests one at a time, as multiple manifests may
// request the same dependency at differing versions.
for (name, version) in package
.dependencies
.clone()
.into_iter()
.filter(is_version_dep)
.filter_map(|d| {
for p in &locked {
// The requested dependency may be present in the lock file with different versions,
// but only one will be semver-compatible with the requested version.
if d.name == p.name && d.req.matches(&p.version) {
return Some((d.name, p.version.to_string()));
}
}
None
})
{
manifest.upgrade(&Dependency::new(&name).set_version(&version), dry_run)?;
}
}
Ok(())
}
}

/// The set of dependencies to be upgraded, alongside the registries returned from cargo metadata, and
Expand Down Expand Up @@ -287,10 +354,11 @@ fn process(args: Args) -> Result<()> {
all,
allow_prerelease,
dry_run,
to_lockfile,
..
} = args;

if !args.offline {
if !args.offline && !to_lockfile {
let url = registry_url(&find(&manifest_path)?, None)?;
update_registry_index(&url)?;
}
Expand All @@ -301,27 +369,31 @@ fn process(args: Args) -> Result<()> {
Manifests::get_local_one(&manifest_path)
}?;

let existing_dependencies = manifests.get_dependencies(dependency)?;

// Update indices for any alternative registries, unless
// we're offline.
if !args.offline {
for registry_url in existing_dependencies
.0
.values()
.filter_map(|(registry, _)| registry.as_ref())
.collect::<HashSet<_>>()
{
update_registry_index(&Url::parse(registry_url).map_err(|_| {
ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
})?)?;
if to_lockfile {
manifests.sync_to_lockfile(dry_run)
} else {
let existing_dependencies = manifests.get_dependencies(dependency)?;

// Update indices for any alternative registries, unless
// we're offline.
if !args.offline {
for registry_url in existing_dependencies
.0
.values()
.filter_map(|(registry, _)| registry.as_ref())
.collect::<HashSet<_>>()
{
update_registry_index(&Url::parse(registry_url).map_err(|_| {
ErrorKind::CargoEditLib(::cargo_edit::ErrorKind::InvalidCargoConfig)
})?)?;
}
}
}

let upgraded_dependencies =
existing_dependencies.get_upgraded(allow_prerelease, &find(&manifest_path)?)?;
let upgraded_dependencies =
existing_dependencies.get_upgraded(allow_prerelease, &find(&manifest_path)?)?;

manifests.upgrade(&upgraded_dependencies, dry_run)
manifests.upgrade(&upgraded_dependencies, dry_run)
}
}

fn main() {
Expand Down
2 changes: 1 addition & 1 deletion src/manifest.rs
Original file line number Diff line number Diff line change
Expand Up @@ -366,7 +366,7 @@ impl str::FromStr for Manifest {
#[derive(Debug)]
pub struct LocalManifest {
/// Path to the manifest
path: PathBuf,
pub path: PathBuf,
/// Manifest contents
manifest: Manifest,
}
Expand Down
37 changes: 36 additions & 1 deletion tests/cargo-upgrade.rs
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
#[macro_use]
extern crate pretty_assertions;

use std::fs;
use std::{fs, path::Path};

mod utils;
use crate::utils::{
Expand Down Expand Up @@ -41,6 +41,7 @@ pub fn copy_workspace_test() -> (tempdir::TempDir, String, Vec<String>) {

let root_manifest_path = copy_in(".", "Cargo.toml");
copy_in(".", "dummy.rs");
copy_in(".", "Cargo.lock");

let workspace_manifest_paths = ["one", "two", "implicit/three", "explicit/four"]
.iter()
Expand Down Expand Up @@ -415,6 +416,40 @@ For more information try --help ",
.unwrap();
}

// Verify that an upgraded Cargo.toml matches what we expect.
#[test]
fn upgrade_to_lockfile() {
let (tmpdir, manifest) = clone_out_test("tests/fixtures/upgrade/Cargo.toml.lockfile_source");
fs::copy(
Path::new("tests/fixtures/upgrade/Cargo.lock"),
tmpdir.path().join("Cargo.lock"),
)
.unwrap_or_else(|err| panic!("could not copy test lock file: {}", err));;
execute_command(&["upgrade", "--to-lockfile"], &manifest);

let upgraded = get_toml(&manifest);
let target = get_toml("tests/fixtures/upgrade/Cargo.toml.lockfile_target");

assert_eq!(target.to_string(), upgraded.to_string());
}

#[test]
fn upgrade_workspace_to_lockfile() {
let (tmpdir, root_manifest, _workspace_manifests) = copy_workspace_test();

execute_command(&["upgrade", "--all", "--to-lockfile"], &root_manifest);

// The members one and two both request different, semver incompatible
// versions of rand. Test that both were upgraded correctly.
let one_upgraded = get_toml(tmpdir.path().join("one/Cargo.toml").to_str().unwrap());
let one_target = get_toml("tests/fixtures/workspace/one/Cargo.toml.lockfile_target");
assert_eq!(one_target.to_string(), one_upgraded.to_string());

let two_upgraded = get_toml(tmpdir.path().join("two/Cargo.toml").to_str().unwrap());
let two_target = get_toml("tests/fixtures/workspace/two/Cargo.toml.lockfile_target");
assert_eq!(two_target.to_string(), two_upgraded.to_string());
}

#[test]
#[cfg(feature = "test-external-apis")]
fn upgrade_prints_messages() {
Expand Down
78 changes: 78 additions & 0 deletions tests/fixtures/upgrade/Cargo.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 10 additions & 0 deletions tests/fixtures/upgrade/Cargo.toml.lockfile_source
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "one"
version = "0.1.0"

[lib]
path = "../dummy.rs"

[dependencies]
libc = "0.2.28"
rand = "0.3"
10 changes: 10 additions & 0 deletions tests/fixtures/upgrade/Cargo.toml.lockfile_target
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
[package]
name = "one"
version = "0.1.0"

[lib]
path = "../dummy.rs"

[dependencies]
libc = "0.2.65"
rand = "0.3.10"
Loading