diff --git a/cargo-smart-release/src/changelog/init.rs b/cargo-smart-release/src/changelog/init.rs index 154bb30a7ba..303b0134bc5 100644 --- a/cargo-smart-release/src/changelog/init.rs +++ b/cargo-smart-release/src/changelog/init.rs @@ -6,9 +6,7 @@ use git_repository as git; use crate::{ changelog::{section::segment, Section}, - commit, - utils::package_by_name, - ChangeLog, + commit, ChangeLog, }; #[derive(Clone, Copy)] @@ -88,12 +86,11 @@ impl ChangeLog { } pub fn for_crate_by_name_with_write_lock<'a>( - crate_name: &str, + package: &'a Package, history: &commit::History, ctx: &'a crate::Context, selection: segment::Selection, ) -> anyhow::Result<(Outcome, &'a Package)> { - let package = package_by_name(&ctx.meta, crate_name)?; let out = Self::for_package_with_write_lock(package, history, ctx, selection)?; Ok((out, package)) } diff --git a/cargo-smart-release/src/command/changelog.rs b/cargo-smart-release/src/command/changelog.rs index 5022e8ebb5e..cf6ce78467d 100644 --- a/cargo-smart-release/src/command/changelog.rs +++ b/cargo-smart-release/src/command/changelog.rs @@ -1,5 +1,6 @@ use std::io::Write; +use crate::utils::package_by_name; use crate::{ bat, changelog::write::{Components, Linkables}, @@ -11,10 +12,13 @@ use crate::{ pub fn changelog(opts: Options, crates: Vec) -> anyhow::Result<()> { let ctx = crate::Context::new(crates)?; - let crate_names = if opts.dependencies { + let crates = if opts.dependencies { crate::traverse::dependencies(&ctx, false, true)?.crates_to_be_published } else { - ctx.crate_names.clone() + ctx.crate_names + .iter() + .map(|name| package_by_name(&ctx.meta, name)) + .collect::, _>>()? }; assure_working_tree_is_unchanged(opts)?; let history = match git::history::collect(&ctx.repo)? { @@ -34,13 +38,13 @@ pub fn changelog(opts: Options, crates: Vec) -> anyhow::Result<()> { }) .unwrap_or(Linkables::AsText) }; - for (idx, crate_name) in crate_names.iter().enumerate() { + for (idx, package) in crates.iter().enumerate() { let ( crate::changelog::init::Outcome { log, mut lock, state, .. }, _package, - ) = ChangeLog::for_crate_by_name_with_write_lock(crate_name, &history, &ctx, opts.generator_segments)?; + ) = ChangeLog::for_crate_by_name_with_write_lock(package, &history, &ctx, opts.generator_segments)?; log::info!( "{} write {} sections to {} ({})", will(opts.dry_run), @@ -69,7 +73,7 @@ pub fn changelog(opts: Options, crates: Vec) -> anyhow::Result<()> { bat.display_to_tty( lock.lock_path(), lock.resource_path().strip_prefix(&ctx.root.to_path_buf())?, - format!("PREVIEW {} / {}, press Ctrl+C to cancel", idx + 1, crate_names.len()), + format!("PREVIEW {} / {}, press Ctrl+C to cancel", idx + 1, crates.len()), )?; } if !opts.dry_run { diff --git a/cargo-smart-release/src/command/release/mod.rs b/cargo-smart-release/src/command/release/mod.rs index fff9d7efeff..8a4672189e5 100644 --- a/cargo-smart-release/src/command/release/mod.rs +++ b/cargo-smart-release/src/command/release/mod.rs @@ -112,7 +112,11 @@ pub fn release(opts: Options, crates: Vec, bump: BumpSpec, bump_dependen fn release_depth_first(ctx: Context, options: Options) -> anyhow::Result<()> { let meta = &ctx.base.meta; let changed_crate_names_to_publish = if options.skip_dependencies { - ctx.base.crate_names.clone() + ctx.base + .crate_names + .iter() + .map(|name| package_by_name(&ctx.base.meta, name)) + .collect::, _>>()? } else { crate::traverse::dependencies(&ctx.base, options.verbose, options.allow_auto_publish_of_stable_crates)? .crates_to_be_published @@ -125,12 +129,10 @@ fn release_depth_first(ctx: Context, options: Options) -> anyhow::Result<()> { if options.multi_crate_release && !changed_crate_names_to_publish.is_empty() { perform_multi_version_release(&ctx, options, meta, changed_crate_names_to_publish)?; } else { - for publishee_name in changed_crate_names_to_publish + for publishee in changed_crate_names_to_publish .iter() - .filter(|n| !crates_to_publish_together.contains(n)) + .filter(|package| crates_to_publish_together.iter().find(|p| p.id == package.id).is_none()) { - let publishee = package_by_name(meta, publishee_name)?; - let ( new_version, manifest::Outcome { @@ -144,7 +146,7 @@ fn release_depth_first(ctx: Context, options: Options) -> anyhow::Result<()> { &new_version, commit_id, release_section_by_publishee - .get(publishee_name.as_str()) + .get(publishee.name.as_str()) .and_then(|s| section_to_string(s, WriteMode::Tag)), &ctx.base, options, @@ -154,7 +156,7 @@ fn release_depth_first(ctx: Context, options: Options) -> anyhow::Result<()> { .allow_changelog_github_release .then(|| { release_section_by_publishee - .get(publishee_name.as_str()) + .get(publishee.name.as_str()) .and_then(|s| section_to_string(s, WriteMode::GitHubRelease)) }) .flatten() @@ -188,12 +190,11 @@ fn perform_multi_version_release( ctx: &Context, options: Options, meta: &Metadata, - crates_to_publish_together: Vec, + crates_to_publish_together: Vec<&Package>, ) -> anyhow::Result<()> { let mut crates_to_publish_together = crates_to_publish_together .into_iter() - .map(|name| { - let p = package_by_name(meta, &name)?; + .map(|p| { version::bump(p, version::select_publishee_bump_spec(&p.name, ctx), ctx, &options) .map(|v| (p, v.to_string())) }) @@ -306,15 +307,14 @@ fn perform_single_release<'repo, 'meta>( Ok((new_version, commit_id_and_changelog_sections)) } -fn resolve_cycles_with_publish_group( - meta: &Metadata, - changed_crate_names_to_publish: &[String], +fn resolve_cycles_with_publish_group<'meta>( + meta: &'meta Metadata, + changed_crate_names_to_publish: &[&'meta Package], options: Options, -) -> anyhow::Result> { +) -> anyhow::Result> { let mut crates_to_publish_additionally_to_avoid_instability = Vec::new(); - let mut publish_group = Vec::::new(); - for publishee_name in changed_crate_names_to_publish.iter() { - let publishee = package_by_name(meta, publishee_name)?; + let mut publish_group = Vec::<&Package>::new(); + for publishee in changed_crate_names_to_publish.iter() { let cycles = workspace_members_referring_to_publishee(meta, publishee); if cycles.is_empty() { log::debug!("'{}' is cycle-free", publishee.name); @@ -330,12 +330,16 @@ fn resolve_cycles_with_publish_group( format!("via {} hops", hops) } ); - if !changed_crate_names_to_publish.contains(&from.name) { - crates_to_publish_additionally_to_avoid_instability.push(from.name.clone()); + if changed_crate_names_to_publish + .iter() + .find(|p| p.id == from.id) + .is_none() + { + crates_to_publish_additionally_to_avoid_instability.push(from.name.as_str()); } else { - for name in &[&from.name, &publishee.name] { - if !publish_group.contains(name) { - publish_group.push(name.to_string()) + for package in &[from, publishee] { + if publish_group.iter().find(|p| p.id == package.id).is_none() { + publish_group.push(package) } } } @@ -354,24 +358,27 @@ fn resolve_cycles_with_publish_group( )) } -fn reorder_according_to_existing_order(reference_order: &[String], names_to_order: &[String]) -> Vec { +fn reorder_according_to_existing_order<'meta>( + reference_order: &[&'meta Package], + packages_to_order: &[&'meta Package], +) -> Vec<&'meta Package> { let new_order = reference_order .iter() - .filter(|name| names_to_order.contains(name)) - .fold(Vec::new(), |mut acc, name| { - acc.push(name.clone()); + .filter(|package| packages_to_order.iter().find(|p| p.id == package.id).is_some()) + .fold(Vec::new(), |mut acc, package| { + acc.push(*package); acc }); assert_eq!( new_order.len(), - names_to_order.len(), + packages_to_order.len(), "the reference order must contain all items to be ordered" ); new_order } -struct Cycle<'a> { - from: &'a Package, +struct Cycle<'meta> { + from: &'meta Package, hops: usize, } diff --git a/cargo-smart-release/src/traverse.rs b/cargo-smart-release/src/traverse.rs index d3eddff0055..680a9b9b3fa 100644 --- a/cargo-smart-release/src/traverse.rs +++ b/cargo-smart-release/src/traverse.rs @@ -1,16 +1,18 @@ use std::collections::BTreeSet; -use cargo_metadata::{DependencyKind, Metadata, Package}; +use cargo_metadata::{DependencyKind, Metadata, Package, PackageId}; use crate::{ git, - utils::{is_pre_release_version, is_workspace_member, package_by_name}, + utils::{is_pre_release_version, package_by_name, workspace_package_by_name}, }; pub mod dependencies { - pub struct Outcome { - pub crates_to_be_published: Vec, - pub unchanged_crates_to_skip: Vec, + use cargo_metadata::Package; + + pub struct Outcome<'meta> { + pub crates_to_be_published: Vec<&'meta Package>, + pub unchanged_crates_to_skip: Vec<&'meta Package>, } } @@ -18,21 +20,21 @@ pub fn dependencies( ctx: &crate::Context, verbose: bool, add_production_crates: bool, -) -> anyhow::Result { +) -> anyhow::Result> { let mut seen = BTreeSet::new(); let mut changed_crate_names_to_publish = Vec::new(); let mut skipped = Vec::new(); for crate_name in &ctx.crate_names { - if seen.contains(crate_name) { + let package = package_by_name(&ctx.meta, crate_name)?; + if seen.contains(&&package.id) { continue; } - if dependency_tree_has_link_to_existing_crate_names(&ctx.meta, crate_name, &changed_crate_names_to_publish)? { + if dependency_tree_has_link_to_existing_crate_names(&ctx.meta, package, &changed_crate_names_to_publish)? { // redo all work which includes the previous tree. Could be more efficient but that would be more complicated. seen.clear(); changed_crate_names_to_publish.clear(); } let num_crates_for_publishing_without_dependencies = changed_crate_names_to_publish.len(); - let package = package_by_name(&ctx.meta, crate_name)?; let current_skipped = depth_first_traversal( ctx, add_production_crates, @@ -49,18 +51,17 @@ pub fn dependencies( } skipped.extend(current_skipped); if num_crates_for_publishing_without_dependencies == changed_crate_names_to_publish.len() { - let crate_package = package_by_name(&ctx.meta, crate_name)?; - if !git::has_changed_since_last_release(crate_package, ctx, verbose)? { + if !git::has_changed_since_last_release(package, ctx, verbose)? { log::info!( "Skipping provided {} v{} hasn't changed since last released", - crate_package.name, - crate_package.version + package.name, + package.version ); continue; } } - changed_crate_names_to_publish.push(crate_name.to_owned()); - seen.insert(crate_name.to_owned()); + changed_crate_names_to_publish.push(package); + seen.insert(&package.id); } Ok(dependencies::Outcome { crates_to_be_published: changed_crate_names_to_publish, @@ -68,55 +69,58 @@ pub fn dependencies( }) } -fn depth_first_traversal( - ctx: &crate::Context, +fn depth_first_traversal<'meta>( + ctx: &'meta crate::Context, add_production_crates: bool, - seen: &mut BTreeSet, - changed_crate_names_to_publish: &mut Vec, - package: &Package, + seen: &mut BTreeSet<&'meta PackageId>, + changed_crate_names_to_publish: &mut Vec<&'meta Package>, + root: &Package, verbose: bool, -) -> anyhow::Result> { +) -> anyhow::Result> { let mut skipped = Vec::new(); - for dependency in package.dependencies.iter().filter(|d| d.kind == DependencyKind::Normal) { - if seen.contains(&dependency.name) || !is_workspace_member(&ctx.meta, &dependency.name) { + for dependency in root.dependencies.iter().filter(|d| d.kind == DependencyKind::Normal) { + let workspace_dependency = match workspace_package_by_name(&ctx.meta, &dependency.name) { + Some(p) => p, + None => continue, + }; + if seen.contains(&&workspace_dependency.id) { continue; } - seen.insert(dependency.name.clone()); - let dep_package = package_by_name(&ctx.meta, &dependency.name)?; + seen.insert(&workspace_dependency.id); skipped.extend(depth_first_traversal( ctx, add_production_crates, seen, changed_crate_names_to_publish, - dep_package, + workspace_dependency, verbose, )?); - if git::has_changed_since_last_release(dep_package, ctx, verbose)? { - if is_pre_release_version(&dep_package.version) || add_production_crates { + if git::has_changed_since_last_release(workspace_dependency, ctx, verbose)? { + if is_pre_release_version(&workspace_dependency.version) || add_production_crates { if verbose { log::info!( "Adding '{}' v{} to set of published crates as it changed since last release", - dep_package.name, - dep_package.version + workspace_dependency.name, + workspace_dependency.version ); } - changed_crate_names_to_publish.push(dependency.name.clone()); + changed_crate_names_to_publish.push(workspace_dependency); } else { log::warn!( "'{}' v{} changed since last release - consider releasing it beforehand.", - dep_package.name, - dep_package.version + workspace_dependency.name, + workspace_dependency.version ); } } else { if verbose { log::info!( "'{}' v{} - skipped release as it didn't change", - dep_package.name, - dep_package.version + workspace_dependency.name, + workspace_dependency.version ); } - skipped.push(dep_package.name.clone()); + skipped.push(workspace_dependency); } } Ok(skipped) @@ -124,24 +128,23 @@ fn depth_first_traversal( fn dependency_tree_has_link_to_existing_crate_names( meta: &Metadata, - root_name: &str, - existing_names: &[String], + root: &Package, + existing: &[&Package], ) -> anyhow::Result { - let mut dependency_names = vec![root_name]; + let mut dependencies = vec![root]; let mut seen = BTreeSet::new(); - while let Some(crate_name) = dependency_names.pop() { - if !seen.insert(crate_name) { + while let Some(package) = dependencies.pop() { + if !seen.insert(&package.id) { continue; } - if existing_names.iter().any(|n| n == crate_name) { + if existing.iter().any(|n| n.id == package.id) { return Ok(true); } - dependency_names.extend( - package_by_name(meta, crate_name)? + dependencies.extend( + package .dependencies .iter() - .filter(|dep| is_workspace_member(meta, &dep.name)) - .map(|dep| dep.name.as_str()), + .filter_map(|dep| workspace_package_by_name(meta, &dep.name)), ) } Ok(false) diff --git a/cargo-smart-release/src/utils.rs b/cargo-smart-release/src/utils.rs index f4c6114a413..1243186f2ce 100644 --- a/cargo-smart-release/src/utils.rs +++ b/cargo-smart-release/src/utils.rs @@ -49,10 +49,6 @@ pub fn is_dependency_with_version_requirement(dep: &Dependency) -> bool { !dep.req.comparators.is_empty() } -pub fn is_workspace_member(meta: &Metadata, crate_name: &str) -> bool { - workspace_package_by_name(meta, crate_name).is_some() -} - pub fn package_eq_dependency(package: &Package, dependency: &Dependency) -> bool { package.name == dependency.name }