From 5a808c54de8b21d010a17e6f81b405b18286df82 Mon Sep 17 00:00:00 2001 From: Jeff Parsons Date: Sun, 29 Oct 2023 09:42:11 +1100 Subject: [PATCH] Add `upgrade` command to upgrade self and bindings Installs the latest version of cargo-component and upgrade to the corresponding version of cargo-component-bindings. --- src/bin/cargo-component.rs | 11 ++- src/commands.rs | 2 + src/commands/add.rs | 2 +- src/commands/publish.rs | 2 +- src/commands/update.rs | 2 +- src/commands/upgrade.rs | 196 +++++++++++++++++++++++++++++++++++++ src/lib.rs | 13 ++- tests/support/mod.rs | 10 +- tests/upgrade.rs | 142 +++++++++++++++++++++++++++ 9 files changed, 369 insertions(+), 11 deletions(-) create mode 100644 src/commands/upgrade.rs create mode 100644 tests/upgrade.rs diff --git a/src/bin/cargo-component.rs b/src/bin/cargo-component.rs index 61f42e77..beddcf5b 100644 --- a/src/bin/cargo-component.rs +++ b/src/bin/cargo-component.rs @@ -1,6 +1,6 @@ use anyhow::{bail, Result}; use cargo_component::{ - commands::{AddCommand, KeyCommand, NewCommand, PublishCommand, UpdateCommand}, + commands::{AddCommand, KeyCommand, NewCommand, PublishCommand, UpdateCommand, UpgradeCommand}, config::{CargoArguments, Config}, load_component_metadata, load_metadata, run_cargo_command, }; @@ -23,6 +23,7 @@ const BUILTIN_COMMANDS: &[&str] = &[ "remove", "rm", "update", + "upgrade", "vendor", "yank", ]; @@ -72,6 +73,7 @@ enum Command { New(NewCommand), // TODO: Remove(RemoveCommand), Update(UpdateCommand), + Upgrade(UpgradeCommand), Publish(PublishCommand), // TODO: Yank(YankCommand), // TODO: Vendor(VendorCommand), @@ -115,6 +117,7 @@ async fn main() -> Result<()> { Command::Key(cmd) => cmd.exec().await, Command::New(cmd) => cmd.exec().await, Command::Update(cmd) => cmd.exec().await, + Command::Upgrade(cmd) => cmd.exec().await, Command::Publish(cmd) => cmd.exec().await, }, } { @@ -159,7 +162,11 @@ async fn main() -> Result<()> { cargo_args.color.unwrap_or_default(), ))?; - let metadata = load_metadata(config.terminal(), cargo_args.manifest_path.as_deref())?; + let metadata = load_metadata( + config.terminal(), + cargo_args.manifest_path.as_deref(), + false, + )?; let packages = load_component_metadata( &metadata, cargo_args.packages.iter(), diff --git a/src/commands.rs b/src/commands.rs index 65370770..71979679 100644 --- a/src/commands.rs +++ b/src/commands.rs @@ -5,9 +5,11 @@ mod key; mod new; mod publish; mod update; +mod upgrade; pub use self::add::*; pub use self::key::*; pub use self::new::*; pub use self::publish::*; pub use self::update::*; +pub use self::upgrade::*; diff --git a/src/commands/add.rs b/src/commands/add.rs index 40236539..96a329ba 100644 --- a/src/commands/add.rs +++ b/src/commands/add.rs @@ -65,7 +65,7 @@ impl AddCommand { /// Executes the command pub async fn exec(self) -> Result<()> { let config = Config::new(self.common.new_terminal())?; - let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref())?; + let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref(), false)?; let PackageComponentMetadata { package, metadata }: PackageComponentMetadata<'_> = match &self.spec { diff --git a/src/commands/publish.rs b/src/commands/publish.rs index 69efe88c..bf0a05f7 100644 --- a/src/commands/publish.rs +++ b/src/commands/publish.rs @@ -92,7 +92,7 @@ impl PublishCommand { } } - let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref())?; + let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref(), false)?; let packages = [PackageComponentMetadata::new( if let Some(spec) = &self.cargo_package { metadata diff --git a/src/commands/update.rs b/src/commands/update.rs index 5b1095c1..e8a75327 100644 --- a/src/commands/update.rs +++ b/src/commands/update.rs @@ -38,7 +38,7 @@ impl UpdateCommand { pub async fn exec(self) -> Result<()> { log::debug!("executing update command"); let config = Config::new(self.common.new_terminal())?; - let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref())?; + let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref(), false)?; let packages = load_component_metadata(&metadata, [].iter(), true)?; let network_allowed = !self.frozen && !self.offline; diff --git a/src/commands/upgrade.rs b/src/commands/upgrade.rs new file mode 100644 index 00000000..0f21c683 --- /dev/null +++ b/src/commands/upgrade.rs @@ -0,0 +1,196 @@ +use crate::{load_metadata, Config, BINDINGS_CRATE_NAME}; +use anyhow::{Context, Result}; +use cargo_component_core::command::CommonOptions; +use cargo_metadata::Metadata; +use clap::Args; +use std::{fs, path::PathBuf}; +use toml_edit::{value, Document}; + +/// Install the latest version of cargo-component and upgrade to the +/// corresponding version of cargo-component-bindings. +#[derive(Args)] +#[clap(disable_version_flag = true)] +pub struct UpgradeCommand { + /// The common command options + #[clap(flatten)] + pub common: CommonOptions, + + /// Don't actually write the Cargo.toml changes. + /// + /// Note that this will not prevent installing a new version of cargo-component itself; + /// if you want to do that, you must also specify the '--no-install' flag. + #[clap(long = "dry-run")] + pub dry_run: bool, + + /// Path to Cargo.toml + #[clap(long = "manifest-path", value_name = "PATH")] + pub manifest_path: Option, + + /// Skip installing the latest version of cargo-component; + /// instead just upgrade cargo-component-bindings to match + /// the version currently running. + #[clap(long = "no-install")] + pub no_install: bool, +} + +impl UpgradeCommand { + /// Executes the command. + pub async fn exec(self) -> Result<()> { + log::debug!("executing upgrade command"); + + if !self.no_install { + // Do the self-upgrade first, and then _unconditionally_ delegate + // to whatever version of `cargo-component` is now at the same path as the + // current executable. + // + // This avoids needing to query crates.io ourselves, scrape the version + // from `cargo-component --version` etc. + // + // (We can't tell whether or not cargo-install actually installed anything + // without scraping its output; it considers "already installed" as success.) + // + // Skip this in tests, but still delegate to a new instance of `cargo-component` + // so that we can exercise as much of the flow as practicable. + #[cfg(not(test))] + upgrade_self()?; + run_cargo_component_and_exit(); + } + + let config = Config::new(self.common.new_terminal())?; + let metadata = load_metadata(config.terminal(), self.manifest_path.as_deref(), true)?; + + upgrade_bindings(&config, &metadata, self.dry_run).await?; + + Ok(()) + } +} + +#[cfg(not(test))] +fn upgrade_self() -> Result<()> { + log::debug!("running self-upgrade using cargo-install"); + + let mut command = std::process::Command::new("cargo"); + command.args(["install", "cargo-component"]); + + match command.status() { + Ok(status) => { + if !status.success() { + std::process::exit(status.code().unwrap_or(1)); + } + Ok(()) + } + Err(e) => { + anyhow::bail!("failed to execute `cargo install` command: {e}") + } + } +} + +fn run_cargo_component_and_exit() -> ! { + log::debug!("running cargo-component from same path as this process"); + + let mut args = std::env::args(); + + // argv[0] cannot be relied up on as a path to the executable; + // skip it and use `current_exe` instead. + let _ = args.next(); + + let mut command = std::process::Command::new( + std::env::current_exe().expect("Failed to get path to current executable"), + ); + command.args(args); + + // Unconditionally specify '--no-install' to prevent infinite recursion. + command.arg("--no-install"); + + match command.status() { + Ok(status) => { + std::process::exit(status.code().unwrap_or(1)); + } + Err(e) => { + log::error!("failed to delegate to `cargo-component` command: {e}"); + std::process::exit(1); + } + } +} + +async fn upgrade_bindings(config: &Config, metadata: &Metadata, dry_run: bool) -> Result<()> { + let self_version = semver::VersionReq::parse(env!("CARGO_PKG_VERSION")) + .context("Failed to parse current cargo-component version")?; + + for package in metadata.workspace_packages() { + let Some(bindings_dep) = package + .dependencies + .iter() + .find(|dep| dep.name == "cargo-component-bindings") + else { + log::debug!( + "Workspace package {} doesn't depend on cargo-component-bindings", + package.name + ); + continue; + }; + + if bindings_dep.req == self_version { + config.terminal().status( + "Skipping", + format!( + "package `{}` as it already uses the current bindings crate version", + package.name + ), + )?; + continue; + } + + let manifest_path = package.manifest_path.as_std_path(); + let manifest = fs::read_to_string(manifest_path).with_context(|| { + format!( + "failed to read manifest file `{path}`", + path = manifest_path.display() + ) + })?; + + let mut doc: Document = manifest.parse().with_context(|| { + format!( + "failed to parse manifest file `{path}`", + path = manifest_path.display() + ) + })?; + + doc["dependencies"][BINDINGS_CRATE_NAME] = value(env!("CARGO_PKG_VERSION")); + + // Do this fairly late, so we exercise as much of the real code as possible + // (encounter explosions that would happen if doing it for real) + // without actually writing back the file. + if dry_run { + config.terminal().status( + "Would update", + format!( + "{path} from {from} to {to}", + path = manifest_path.display(), + from = bindings_dep.req, + to = env!("CARGO_PKG_VERSION") + ), + )?; + continue; + } + + fs::write(manifest_path, doc.to_string()).with_context(|| { + format!( + "failed to write manifest file `{path}`", + path = manifest_path.display() + ) + })?; + + config.terminal().status( + "Updated", + format!( + "{path} from {from} to {to}", + path = manifest_path.display(), + from = bindings_dep.req, + to = env!("CARGO_PKG_VERSION") + ), + )?; + } + + Ok(()) +} diff --git a/src/lib.rs b/src/lib.rs index 96138636..84c13d11 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -200,7 +200,11 @@ fn last_modified_time(path: &Path) -> Result { } /// Loads the workspace metadata based on the given manifest path. -pub fn load_metadata(terminal: &Terminal, manifest_path: Option<&Path>) -> Result { +pub fn load_metadata( + terminal: &Terminal, + manifest_path: Option<&Path>, + ignore_version_mismatch: bool, +) -> Result { let mut command = MetadataCommand::new(); command.no_deps(); @@ -222,9 +226,10 @@ pub fn load_metadata(terminal: &Terminal, manifest_path: Option<&Path>) -> Resul continue; } - if dep - .req - .matches(&Version::parse(env!("CARGO_PKG_VERSION")).unwrap()) + if ignore_version_mismatch + || dep + .req + .matches(&Version::parse(env!("CARGO_PKG_VERSION")).unwrap()) { continue; } diff --git a/tests/support/mod.rs b/tests/support/mod.rs index 6999d89d..cf09f8b3 100644 --- a/tests/support/mod.rs +++ b/tests/support/mod.rs @@ -319,10 +319,16 @@ impl Project { Ok(self) } + pub fn read_manifest(&self) -> Result { + let manifest_path = self.root.join("Cargo.toml"); + let manifest_text = fs::read_to_string(manifest_path)?; + Ok(manifest_text.parse()?) + } + pub fn update_manifest(&self, f: impl FnOnce(Document) -> Result) -> Result<()> { + let manifest = self.read_manifest()?; let manifest_path = self.root.join("Cargo.toml"); - let manifest = fs::read_to_string(&manifest_path)?; - fs::write(manifest_path, f(manifest.parse()?)?.to_string())?; + fs::write(manifest_path, f(manifest)?.to_string())?; Ok(()) } diff --git a/tests/upgrade.rs b/tests/upgrade.rs new file mode 100644 index 00000000..39e7aeaa --- /dev/null +++ b/tests/upgrade.rs @@ -0,0 +1,142 @@ +use crate::support::*; +use anyhow::Result; +use assert_cmd::prelude::*; +use cargo_component::BINDINGS_CRATE_NAME; +use predicates::{prelude::PredicateBooleanExt, str::contains}; +use toml_edit::value; + +mod support; + +#[test] +fn help() { + for arg in ["help upgrade", "upgrade -h", "upgrade --help"] { + cargo_component(arg) + .assert() + .stdout(contains( + "Install the latest version of cargo-component and upgrade to the corresponding version of cargo-component-bindings", + )) + .success(); + } +} + +#[test] +fn upgrade_single_crate_already_current_is_no_op() -> Result<()> { + let root = create_root()?; + let project = Project::with_root(&root, "component", "")?; + + project + .cargo_component("upgrade") + .assert() + .success() + .stderr(contains( + "Skipping package `component` as it already uses the current bindings crate version", + )); + + Ok(()) +} + +#[test] +fn upgrade_single_crate_upgrades_bindings_dep() -> Result<()> { + let root = create_root()?; + let project = Project::with_root(&root, "component", "")?; + project.update_manifest(|mut doc| { + // Set arbitrary old version of bindings crate. + doc["dependencies"][BINDINGS_CRATE_NAME] = value("0.1"); + Ok(doc) + })?; + + // Check that the change actually stuck, and the old version + // we set isn't the same as the current version. + // (For symmetry with the assertion below that we actually + // end up upgrading the bindings dep.) + let manifest = project.read_manifest()?; + assert_eq!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some("0.1") + ); + assert_ne!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some(env!("CARGO_PKG_VERSION")) + ); + + project + .cargo_component("upgrade") + .assert() + .success() + .stderr(contains("Updated ")) + .stderr(contains(format!( + "from ^0.1 to {}", + env!("CARGO_PKG_VERSION") + ))); + + // It should have actually written the upgrade. + let manifest = project.read_manifest()?; + assert_ne!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some("0.1") + ); + assert_eq!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some(env!("CARGO_PKG_VERSION")) + ); + + // A repeated upgrade should recognize that there is no change required. + project + .cargo_component("upgrade") + .assert() + .success() + .stderr(contains( + "Skipping package `component` as it already uses the current bindings crate version", + )); + + Ok(()) +} + +#[test] +fn upgrade_dry_run_does_not_alter_manifest() -> Result<()> { + let root = create_root()?; + let project = Project::with_root(&root, "component", "")?; + project.update_manifest(|mut doc| { + // Set arbitrary old version of bindings crate. + doc["dependencies"][BINDINGS_CRATE_NAME] = value("0.1"); + Ok(doc) + })?; + + // Check that the change actually stuck, and the old version + // we set isn't the same as the current version. + // (For symmetry with the assertion below that we actually + // end up upgrading the bindings dep.) + let manifest = project.read_manifest()?; + assert_eq!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some("0.1") + ); + assert_ne!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some(env!("CARGO_PKG_VERSION")) + ); + + project + .cargo_component("upgrade --dry-run") + .assert() + .success() + .stderr(contains("Would update ")) + .stderr(contains("Updated ").not()) + .stderr(contains(format!( + "from ^0.1 to {}", + env!("CARGO_PKG_VERSION") + ))); + + // It should NOT have written the upgrade. + let manifest = project.read_manifest()?; + assert_eq!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some("0.1") + ); + assert_ne!( + manifest["dependencies"][BINDINGS_CRATE_NAME].as_str(), + Some(env!("CARGO_PKG_VERSION")) + ); + + Ok(()) +}