From 42f8ed47b927551d9b2ba9068ebbb41324bca3db Mon Sep 17 00:00:00 2001 From: Ed Page Date: Wed, 19 Oct 2022 10:36:01 -0500 Subject: [PATCH] feat: Ensure configured owners are set Example use case: Automatically add the clap maintainers group as new crates get published. I went ahead and made it so this always runs and not just after first-publish so its easy to set owners after-the-fact. Maybe we should make a distinction on that between `cargo release` and `cargo release owner` Fixes #529 --- docs/reference.md | 48 +++++++------- src/bin/cargo-release.rs | 2 + src/config.rs | 9 +++ src/ops/cargo.rs | 76 ++++++++++++++++++++++ src/steps/mod.rs | 1 + src/steps/owner.rs | 136 +++++++++++++++++++++++++++++++++++++++ src/steps/release.rs | 1 + 7 files changed, 250 insertions(+), 23 deletions(-) create mode 100644 src/steps/owner.rs diff --git a/docs/reference.md b/docs/reference.md index c5bad8ad5..414530f11 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -83,6 +83,7 @@ Steps: replace Perform pre-release replacements hook Run pre-release hooks publish Publish the specified packages + owner Owner the specified packages tag Tag the released commits push Push tags/commits to remote config Dump workspace configuration @@ -131,33 +132,34 @@ Workspace configuration is read from the following (in precedence order) ### Config Fields -| Field | Argument | Format | Defaults | Description | -|----------------|-----------------|-----------------------------|--------------|-------------| -| | `--prev-tag-name` | string | | Last released tag; used for seeing what changed in the current release (default based on `tag-name` and current version in `Cargo.toml`) | -| `allow-branch` | `--allow-branch` | list of globs | `[*, !HEAD]` | *(workspace)* Which branches are allowed to be released from | -| `sign-commit` | `--sign-commit` | bool | `false` | Use GPG to sign git commits generated by cargo-release. [Further information](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work). In 0.14 `sign-commit` is to control signing for commit only, use `sign-tag` for tag signing. | -| `sign-tag` | `--sign-tag` | bool | `false` | Use GPG to sign git tag generated by cargo-release. | -| `registry` | `--registry` | string | \- | Cargo registry name to publish to (default uses Rust's default, which goes to `crates.io`) | -| `release` | `--package` | bool | `true` | Release this crate (usually disabled for internal crates in a workspace) | -| `push` | `--no-push` | bool | `true` | Don't do git push | -| `push-remote` | `--push-remote` | string | `origin` | Default git remote to push | -| `push-options` | \- | list of strings | `[]` | Flags to send to the server when doing a `git push` | -| `shared-version` | \- | bool or string | `false` | Ensure all crates with `shared-version` are the same version. May also be a string to create named subsets of shared versions | -| `consolidate-commits` | \- | bool | `true` | When releasing a workspace, use a single commit for the pre-release version bump and a single commit for the post-release version bump. Commit settings will be read from the workspace-config. | +| Field | Argument | Format | Defaults | Description | +|----------------|-----------------|-----------------------------|---------------|-------------| +| | `--prev-tag-name` | string | | Last released tag; used for seeing what changed in the current release (default based on `tag-name` and current version in `Cargo.toml`) | +| `allow-branch` | `--allow-branch` | list of globs | `[*, !HEAD]` | *(workspace)* Which branches are allowed to be released from | +| `sign-commit` | `--sign-commit` | bool | `false` | Use GPG to sign git commits generated by cargo-release. [Further information](https://git-scm.com/book/en/v2/Git-Tools-Signing-Your-Work). In 0.14 `sign-commit` is to control signing for commit only, use `sign-tag` for tag signing. | +| `sign-tag` | `--sign-tag` | bool | `false` | Use GPG to sign git tag generated by cargo-release. | +| `registry` | `--registry` | string | \- | Cargo registry name to publish to (default uses Rust's default, which goes to `crates.io`) | +| `release` | `--package` | bool | `true` | Release this crate (usually disabled for internal crates in a workspace) | +| `push` | `--no-push` | bool | `true` | Don't do git push | +| `push-remote` | `--push-remote` | string | `origin` | Default git remote to push | +| `push-options` | \- | list of strings | `[]` | Flags to send to the server when doing a `git push` | +| `shared-version` | \- | bool or string | `false` | Ensure all crates with `shared-version` are the same version. May also be a string to create named subsets of shared versions | +| `consolidate-commits` | \- | bool | `true` | When releasing a workspace, use a single commit for the pre-release version bump and a single commit for the post-release version bump. Commit settings will be read from the workspace-config. | | `pre-release-commit-message` | \- | string | `"(cargo-release) version {{version}}"` | A commit message template for release. For example: `"release {{version}}"`, where `{{version}}` will be replaced by actual version. | | `post-release-commit-message` | \- | string | `"(cargo-release) start next development iteration {{next_version}}"` | A commit message template for bumping version after release. For example: `Released {{version}}, starting {{next_version}}`. The placeholder `{{next_version}}` (the version in git after release) is supported in addition to the global placeholders mentioned below. | -| `tag` | `--no-tag` | bool | `true` | Don't do git tag | +| `tag` | `--no-tag` | bool | `true` | Don't do git tag | | `tag-message` | \- | string | `"(cargo-release) {{crate_name}} version {{version}}"` | A message template for an annotated tag (set to blank for lightweight tags). The placeholder `{{tag_name}}` and `{{prefix}}` (the tag prefix) is supported in addition to the global placeholders mentioned below. | -| `tag-prefix` | `--tag-prefix` | string | *depends* | Prefix of git tag, note that this will override default prefix based on crate name. | +| `tag-prefix` | `--tag-prefix` | string | *depends* | Prefix of git tag, note that this will override default prefix based on crate name. | | `tag-name` | `--tag-name` | string | `"{{prefix}}v{{version}}"` | The name of the git tag. The placeholder `{{prefix}}` (the tag prefix) is supported in addition to the global placeholders mentioned below. | -| `pre-release-replacements` | \- | array of tables (see below) | `[]` | Specify files that cargo-release will search and replace with new version for the release commit | -| `post-release-replacements` | \- | array of tables (see below) | `[]` | Specify files that cargo-release will search and replace with new version for the post-release commit (the one starting development) | -| `pre-release-hook` | \- | list of arguments | \- | Provide a command to run before `cargo-release` commits version change. If the return code of hook command is greater than 0, the release process will be aborted. | -| `publish` | `--no-publish` | bool | `true` | Don't do cargo publish right now, see [manifest `publish` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-publish--field-optional) to permanently disable publish. See `release` for disabling the complete release process. | -| `verify` | `--no-verify` | bool | `true` | Don't verify the contents by building them | -| `enable-features` | `--features` | list of names | `[]` | Provide a set of feature flags that should be passed to `cargo publish` (requires rust 1.33+) | -| `enable-all-features` | `--all-features` | bool | `false` | Signal to `cargo publish`, that all features should be used (requires rust 1.33+) | -| `target` | \- | string | \- | Target triple to use for the verification build | +| `pre-release-replacements` | \- | array of tables (see below) | `[]` | Specify files that cargo-release will search and replace with new version for the release commit | +| `post-release-replacements` | \- | array of tables (see below) | `[]` | Specify files that cargo-release will search and replace with new version for the post-release commit (the one starting development) | +| `pre-release-hook` | \- | list of arguments | \- | Provide a command to run before `cargo-release` commits version change. If the return code of hook command is greater than 0, the release process will be aborted. | +| `publish` | `--no-publish` | bool | `true` | Don't do cargo publish right now, see [manifest `publish` field](https://doc.rust-lang.org/cargo/reference/manifest.html#the-publish--field-optional) to permanently disable publish. See `release` for disabling the complete release process. | +| `verify` | `--no-verify` | bool | `true` | Don't verify the contents by building them | +| `owners` | | list of logins | `[]` | Ensure these logins are marked as owners | +| `enable-features` | `--features` | list of names | `[]` | Provide a set of feature flags that should be passed to `cargo publish` (requires rust 1.33+) | +| `enable-all-features` | `--all-features` | bool | `false` | Signal to `cargo publish`, that all features should be used (requires rust 1.33+) | +| `target` | \- | string | \- | Target triple to use for the verification build | | `dependent-version` | \- | `upgrade`, `fix`, `error`, `warn`, `ignore` | `upgrade` | Policy for upgrading path dependency versions within the workspace | Note: fields are from the package-configuration unless otherwise specified. diff --git a/src/bin/cargo-release.rs b/src/bin/cargo-release.rs index 904a92d5e..62b83a1ae 100644 --- a/src/bin/cargo-release.rs +++ b/src/bin/cargo-release.rs @@ -21,6 +21,7 @@ fn run() -> Result<(), error::CliError> { Some(Step::Replace(config)) => config.run(), Some(Step::Hook(config)) => config.run(), Some(Step::Publish(config)) => config.run(), + Some(Step::Owner(config)) => config.run(), Some(Step::Tag(config)) => config.run(), Some(Step::Push(config)) => config.run(), Some(Step::Config(config)) => config.run(), @@ -89,6 +90,7 @@ pub enum Step { Replace(steps::replace::ReplaceStep), Hook(steps::hook::HookStep), Publish(steps::publish::PublishStep), + Owner(steps::owner::OwnerStep), Tag(steps::tag::TagStep), Push(steps::push::PushStep), Config(steps::config::ConfigStep), diff --git a/src/config.rs b/src/config.rs index 97febae5a..57bdb6ac7 100644 --- a/src/config.rs +++ b/src/config.rs @@ -19,6 +19,7 @@ pub struct Config { pub release: Option, pub publish: Option, pub verify: Option, + pub owners: Option>, pub push: Option, pub push_options: Option>, pub shared_version: Option, @@ -60,6 +61,7 @@ impl Config { release: Some(empty.release()), publish: Some(empty.publish()), verify: Some(empty.verify()), + owners: Some(empty.owners().to_vec()), push: Some(empty.push()), push_options: Some( empty @@ -112,6 +114,9 @@ impl Config { if let Some(verify) = source.verify { self.verify = Some(verify); } + if let Some(owners) = source.owners.as_deref() { + self.owners = Some(owners.to_owned()); + } if let Some(push) = source.push { self.push = Some(push); } @@ -200,6 +205,10 @@ impl Config { self.verify.unwrap_or(true) } + pub fn owners(&self) -> &[String] { + self.owners.as_ref().map(|v| v.as_ref()).unwrap_or(&[]) + } + pub fn push(&self) -> bool { self.push.unwrap_or(true) } diff --git a/src/ops/cargo.rs b/src/ops/cargo.rs index c045a839f..4e159259e 100644 --- a/src/ops/cargo.rs +++ b/src/ops/cargo.rs @@ -185,6 +185,82 @@ pub fn set_workspace_version( Ok(()) } +pub fn ensure_owners( + name: &str, + logins: &[String], + registry: Option<&str>, + dry_run: bool, +) -> CargoResult<()> { + let cargo = cargo(); + + // "Look-before-you-leap" in case the user has permission to publish but not set owners. + let mut cmd = std::process::Command::new(&cargo); + cmd.arg("owner").arg(name).arg("--color=never"); + cmd.arg("--list"); + if let Some(registry) = registry { + cmd.arg("--registry"); + cmd.arg(registry); + } + let output = cmd.output()?; + if !output.status.success() { + anyhow::bail!( + "Failed talking to registry about crate owners: {}", + String::from_utf8_lossy(&output.stderr) + ); + } + let raw = String::from_utf8(output.stdout) + .map_err(|_| anyhow::format_err!("Unrecognized response from registry"))?; + + let mut current = std::collections::BTreeSet::new(); + // HACK: No programmatic CLI access and don't want to link against `cargo` (yet), so parsing + // text output + for line in raw.lines() { + if let Some((owner, _)) = line.split_once(' ') { + if !owner.is_empty() { + current.insert(owner); + } + } + } + + let expected = logins + .iter() + .map(|s| s.as_str()) + .collect::>(); + + let missing = expected.difference(¤t).copied().collect::>(); + if !missing.is_empty() { + log::info!("Adding owners for {}: {}", name, missing.join(", ")); + if !dry_run { + let mut cmd = std::process::Command::new(&cargo); + cmd.arg("owner").arg(name).arg("--color=never"); + for missing in missing { + cmd.arg("--add").arg(missing); + } + if let Some(registry) = registry { + cmd.arg("--registry"); + cmd.arg(registry); + } + let output = cmd.output()?; + if !output.status.success() { + // HACK: Can't error as the user might not have permission to set owners and we can't + // tell what the error was without parsing it + log::warn!( + "Failed to set owners for {}: {}", + name, + String::from_utf8_lossy(&output.stderr) + ); + } + } + } + + let extra = current.difference(&expected).copied().collect::>(); + if !extra.is_empty() { + log::debug!("Extra owners for {}: {}", name, extra.join(", ")); + } + + Ok(()) +} + pub fn set_package_version(manifest_path: &Path, version: &str, dry_run: bool) -> CargoResult<()> { let original_manifest = std::fs::read_to_string(manifest_path)?; let mut manifest: toml_edit::Document = original_manifest.parse()?; diff --git a/src/steps/mod.rs b/src/steps/mod.rs index 74b307af3..11c739458 100644 --- a/src/steps/mod.rs +++ b/src/steps/mod.rs @@ -2,6 +2,7 @@ use std::str::FromStr; pub mod config; pub mod hook; +pub mod owner; pub mod plan; pub mod publish; pub mod push; diff --git a/src/steps/owner.rs b/src/steps/owner.rs new file mode 100644 index 000000000..8f1f10c61 --- /dev/null +++ b/src/steps/owner.rs @@ -0,0 +1,136 @@ +use crate::error::CliError; +use crate::ops::git; +use crate::steps::plan; + +/// Owner the specified packages +/// +/// Will automatically skip published versions +#[derive(Debug, Clone, clap::Args)] +pub struct OwnerStep { + #[command(flatten)] + manifest: clap_cargo::Manifest, + + #[command(flatten)] + workspace: clap_cargo::Workspace, + + /// Custom config file + #[arg(short, long = "config")] + custom_config: Option, + + /// Ignore implicit configuration files. + #[arg(long)] + isolated: bool, + + /// Comma-separated globs of branch names a release can happen from + #[arg(long, value_delimiter = ',')] + allow_branch: Option>, + + /// Actually perform a release. Dry-run mode is the default + #[arg(short = 'x', long)] + execute: bool, + + /// Skip release confirmation and version preview + #[arg(long)] + no_confirm: bool, +} + +impl OwnerStep { + pub fn run(&self) -> Result<(), CliError> { + git::git_version()?; + + let ws_meta = self + .manifest + .metadata() + // When evaluating dependency ordering, we need to consider optional dependencies + .features(cargo_metadata::CargoOpt::AllFeatures) + .exec()?; + let config = self.to_config(); + let ws_config = crate::config::load_workspace_config(&config, &ws_meta)?; + let mut pkgs = plan::load(&config, &ws_meta)?; + + let (_selected_pkgs, excluded_pkgs) = self.workspace.partition_packages(&ws_meta); + for excluded_pkg in excluded_pkgs { + let pkg = if let Some(pkg) = pkgs.get_mut(&excluded_pkg.id) { + pkg + } else { + // Either not in workspace or marked as `release = false`. + continue; + }; + pkg.config.publish = Some(false); + pkg.config.owners = Some(vec![]); + pkg.config.release = Some(false); + + let crate_name = pkg.meta.name.as_str(); + log::debug!("Disabled by user, skipping {}", crate_name,); + } + + let pkgs = plan::plan(pkgs)?; + + let (selected_pkgs, _excluded_pkgs): (Vec<_>, Vec<_>) = pkgs + .into_iter() + .map(|(_, pkg)| pkg) + .partition(|p| p.config.release()); + if selected_pkgs.is_empty() { + log::info!("No packages selected."); + return Err(2.into()); + } + + let dry_run = !self.execute; + let mut failed = false; + + // STEP 0: Help the user make the right decisions. + failed |= !super::verify_git_is_clean( + ws_meta.workspace_root.as_std_path(), + dry_run, + log::Level::Error, + )?; + + failed |= !super::verify_git_branch( + ws_meta.workspace_root.as_std_path(), + &ws_config, + dry_run, + log::Level::Error, + )?; + + failed |= !super::verify_if_behind( + ws_meta.workspace_root.as_std_path(), + &ws_config, + dry_run, + log::Level::Warn, + )?; + + // STEP 1: Release Confirmation + super::confirm("Owner", &selected_pkgs, self.no_confirm, dry_run)?; + + ensure_owners(&selected_pkgs, dry_run)?; + + super::finish(failed, dry_run) + } + + fn to_config(&self) -> crate::config::ConfigArgs { + crate::config::ConfigArgs { + custom_config: self.custom_config.clone(), + isolated: self.isolated, + allow_branch: self.allow_branch.clone(), + ..Default::default() + } + } +} + +pub fn ensure_owners(pkgs: &[plan::PackageRelease], dry_run: bool) -> Result<(), CliError> { + for pkg in pkgs { + if !pkg.config.publish() { + continue; + } + + let crate_name = pkg.meta.name.as_str(); + crate::ops::cargo::ensure_owners( + crate_name, + pkg.config.owners(), + pkg.config.registry(), + dry_run, + )?; + } + + Ok(()) +} diff --git a/src/steps/release.rs b/src/steps/release.rs index 08b1519cb..075eb09b2 100644 --- a/src/steps/release.rs +++ b/src/steps/release.rs @@ -280,6 +280,7 @@ impl ReleaseStep { // STEP 3: cargo publish super::publish::publish(&ws_meta, &selected_pkgs, &mut index, dry_run)?; + super::owner::ensure_owners(&selected_pkgs, dry_run)?; // STEP 5: Tag super::tag::tag(&selected_pkgs, dry_run)?;