From 63753f3fc7d56ec01b2e1675773132344e6e9535 Mon Sep 17 00:00:00 2001 From: ysaito1001 Date: Wed, 31 Jul 2024 16:31:37 -0500 Subject: [PATCH] Add new subcommands to `changelogger` for supporting file-per-change changelog (#3771) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit ## Motivation and Context Adds new subcommands `--new` and `--ls` to `changelogger` ## Description This is part 2 in a series of PRs supporting [file-per-change changelog](https://smithy-lang.github.io/smithy-rs/design/rfcs/rfc0042_file_per_change_changelog.html). This PR just adds utilities for humans and does NOT impact the current dev workflow, smithy-rs CI, or our release pipeline. #### A subcommand `--new` We can use this subcommand when creating a new changelog entry markdown file. An example usage: ``` $ changelogger new -t client -t aws-sdk-rust -r smithy-rs#1234 -a someone --bug-fix -m "Some changelog" \ # for long flags, replace -t with --applies-to, -r with --references, -a with --authors, and -m with --message \ # also remember to escape with \ when including backticks in the message at command line, e.g. \`pub\` The following changelog entry has been written to "/Users/ysaito1001/src/smithy-rs/.changelog/5745197.md": --- applies_to: - aws-sdk-rust - client authors: - someone references: - smithy-rs#1234 breaking: false new_feature: false bug_fix: true --- Some changelog ``` The following CLI arguments are "logically" required - `--applies-to` - `--ref` - `--author` - `--message` If any of the above is not passed a value at command line, then an editor is opened for further edit (which editor to open can be configured per the [edit crate](https://docs.rs/edit/0.1.5/edit/)). Note that the YAML syntax above is different from the single line array syntax [proposed in the RFC](https://smithy-lang.github.io/smithy-rs/design/rfcs/rfc0042_file_per_change_changelog.html#the-proposed-developer-experience). This is due to [a limitation](https://github.com/dtolnay/serde-yaml/issues/355) in `serde-yaml` (which has been archived unfortunately), but the multi-line values are still a valid YAML syntax and, most importantly, rendering changelong entries will continue working. For now, I won't post-process from the multi-line values syntax to the single line array syntax. One can work around this by handwriting a changelog entry Markdown file in `smithy-rs/.changelog` without using this subcommand. #### A subcommand `--ls` This subcommand will render a preview of changelog entries stored in `smithy-rs/.changelog` and print it to the standard output. Example usages (using the entry created at 5745197.md above): ``` $ changelogger ls -c smithy-rs \ # for long flag, replace -c with --change-set Next changelog preview for smithy-rs ===================================== **New this release:** - :bug: (client, [smithy-rs#1234](https://github.com/smithy-lang/smithy-rs/issues/1234), @someone) Some changelog **Contributors** Thank you for your contributions! ❤ - @someone ([smithy-rs#1234](https://github.com/smithy-lang/smithy-rs/issues/1234)) ``` ``` $ changelogger ls -c aws-sdk \ # for long flag, replace -c with --change-set Next changelog preview for aws-sdk-rust ======================================== **New this release:** - :bug: ([smithy-rs#1234](https://github.com/smithy-lang/smithy-rs/issues/1234), @someone) Some changelog **Contributors** Thank you for your contributions! ❤ - @someone ([smithy-rs#1234](https://github.com/smithy-lang/smithy-rs/issues/1234)) ``` ## Testing - Existing tests in CI - Basic unit tests for subcommands ---- _By submitting this pull request, I confirm that you can use, modify, copy, and redistribute this contribution, under the terms of your choice._ --- .changelog/.keep | 0 tools/ci-build/changelogger/Cargo.lock | 34 +++- tools/ci-build/changelogger/Cargo.toml | 2 + tools/ci-build/changelogger/src/entry.rs | 14 ++ tools/ci-build/changelogger/src/lib.rs | 2 + tools/ci-build/changelogger/src/ls.rs | 50 ++++++ tools/ci-build/changelogger/src/main.rs | 74 +++++++- tools/ci-build/changelogger/src/new.rs | 159 ++++++++++++++++++ tools/ci-build/changelogger/src/render.rs | 2 +- tools/ci-build/sdk-lints/src/changelog.rs | 5 +- tools/ci-build/sdk-lints/src/lint.rs | 4 +- .../smithy-rs-tool-common/src/changelog.rs | 101 ++++++++++- .../src/changelog/parser.rs | 107 ++++-------- 13 files changed, 462 insertions(+), 92 deletions(-) create mode 100644 .changelog/.keep create mode 100644 tools/ci-build/changelogger/src/ls.rs create mode 100644 tools/ci-build/changelogger/src/new.rs diff --git a/.changelog/.keep b/.changelog/.keep new file mode 100644 index 0000000000..e69de29bb2 diff --git a/tools/ci-build/changelogger/Cargo.lock b/tools/ci-build/changelogger/Cargo.lock index f5b116dea6..bb5d7bff7c 100644 --- a/tools/ci-build/changelogger/Cargo.lock +++ b/tools/ci-build/changelogger/Cargo.lock @@ -133,11 +133,13 @@ version = "0.1.0" dependencies = [ "anyhow", "clap", + "edit", "once_cell", "ordinal", "pretty_assertions", "serde", "serde_json", + "serde_yaml", "smithy-rs-tool-common", "tempfile", "time", @@ -234,6 +236,22 @@ version = "0.1.13" source = "registry+https://github.com/rust-lang/crates.io-index" checksum = "56254986775e3233ffa9c4d7d3faaf6d36a2c09d30b20687e9f88bc8bafc16c8" +[[package]] +name = "edit" +version = "0.1.5" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "f364860e764787163c8c8f58231003839be31276e821e2ad2092ddf496b1aa09" +dependencies = [ + "tempfile", + "which", +] + +[[package]] +name = "either" +version = "1.13.0" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "60b1af1c220855b6ceac025d3f6ecdd2b7c4894bfe9cd9bda4fbb4bc7c0d4cf0" + [[package]] name = "encoding_rs" version = "0.8.34" @@ -261,9 +279,9 @@ dependencies = [ [[package]] name = "fastrand" -version = "2.0.2" +version = "2.1.0" source = "registry+https://github.com/rust-lang/crates.io-index" -checksum = "658bd65b1cf4c852a3cc96f18a8ce7b5640f6b703f905c7d74532294c2a63984" +checksum = "9fc0510504f03c51ada170672ac806f1f105a88aa97a5281117e1ddc3368e51a" [[package]] name = "fnv" @@ -1463,6 +1481,18 @@ dependencies = [ "wasm-bindgen", ] +[[package]] +name = "which" +version = "4.4.2" +source = "registry+https://github.com/rust-lang/crates.io-index" +checksum = "87ba24419a2078cd2b0f2ede2691b6c66d8e47836da3b6db8265ebad47afbfc7" +dependencies = [ + "either", + "home", + "once_cell", + "rustix", +] + [[package]] name = "winapi" version = "0.3.9" diff --git a/tools/ci-build/changelogger/Cargo.toml b/tools/ci-build/changelogger/Cargo.toml index 8dd0936365..7ea48d5f7c 100644 --- a/tools/ci-build/changelogger/Cargo.toml +++ b/tools/ci-build/changelogger/Cargo.toml @@ -17,10 +17,12 @@ opt-level = 0 [dependencies] anyhow = "1.0.57" clap = { version = "~3.2.1", features = ["derive"] } +edit = "0.1" once_cell = "1.15.0" ordinal = "0.3.2" serde = { version = "1", features = ["derive"]} serde_json = "1" +serde_yaml = "0.9" smithy-rs-tool-common = { path = "../smithy-rs-tool-common" } time = { version = "0.3.9", features = ["local-offset"]} toml = "0.5.8" diff --git a/tools/ci-build/changelogger/src/entry.rs b/tools/ci-build/changelogger/src/entry.rs index e97e00019e..8567880636 100644 --- a/tools/ci-build/changelogger/src/entry.rs +++ b/tools/ci-build/changelogger/src/entry.rs @@ -8,6 +8,7 @@ use clap::clap_derive::ArgEnum; use smithy_rs_tool_common::changelog::{Changelog, HandAuthoredEntry, SdkModelEntry}; use smithy_rs_tool_common::git::Git; use smithy_rs_tool_common::versions_manifest::VersionsManifest; +use std::fmt::{Display, Formatter}; use std::path::Path; #[derive(ArgEnum, Copy, Clone, Debug, Eq, PartialEq)] @@ -16,6 +17,19 @@ pub enum ChangeSet { AwsSdk, } +impl Display for ChangeSet { + fn fmt(&self, f: &mut Formatter<'_>) -> std::fmt::Result { + write!( + f, + "{}", + match self { + ChangeSet::SmithyRs => "smithy-rs", + ChangeSet::AwsSdk => "aws-sdk-rust", + } + ) + } +} + pub struct ChangelogEntries { pub aws_sdk_rust: Vec, pub smithy_rs: Vec, diff --git a/tools/ci-build/changelogger/src/lib.rs b/tools/ci-build/changelogger/src/lib.rs index 483dfd2013..f1616c2cff 100644 --- a/tools/ci-build/changelogger/src/lib.rs +++ b/tools/ci-build/changelogger/src/lib.rs @@ -5,5 +5,7 @@ pub mod entry; pub mod init; +pub mod ls; +pub mod new; pub mod render; pub mod split; diff --git a/tools/ci-build/changelogger/src/ls.rs b/tools/ci-build/changelogger/src/ls.rs new file mode 100644 index 0000000000..1bfa57010b --- /dev/null +++ b/tools/ci-build/changelogger/src/ls.rs @@ -0,0 +1,50 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use crate::entry::{ChangeSet, ChangelogEntries}; +use crate::render::render; +use anyhow::Context; +use clap::Parser; +use smithy_rs_tool_common::changelog::{ChangelogLoader, ValidationSet}; +use smithy_rs_tool_common::git::find_git_repository_root; +use smithy_rs_tool_common::here; +use smithy_rs_tool_common::versions_manifest::CrateVersionMetadataMap; + +#[derive(Parser, Debug, Eq, PartialEq)] +pub struct LsArgs { + /// Which set of changes to preview + #[clap(long, short, action)] + pub change_set: ChangeSet, +} + +pub fn subcommand_ls(args: LsArgs) -> anyhow::Result<()> { + let mut dot_changelog = find_git_repository_root("smithy-rs", ".").context(here!())?; + dot_changelog.push(".changelog"); + + let loader = ChangelogLoader::default(); + let changelog = loader.load_from_dir(dot_changelog.clone())?; + + changelog.validate(ValidationSet::Render).map_err(|errs| { + anyhow::Error::msg(format!( + "failed to load {dot_changelog:?}: {errors}", + errors = errs.join("\n") + )) + })?; + + let entries = ChangelogEntries::from(changelog); + + let (release_header, release_notes) = render( + match args.change_set { + ChangeSet::SmithyRs => &entries.smithy_rs, + ChangeSet::AwsSdk => &entries.aws_sdk_rust, + }, + CrateVersionMetadataMap::new(), + &format!("\nNext changelog preview for {}", args.change_set), + ); + + println!("{release_header}{release_notes}"); + + Ok(()) +} diff --git a/tools/ci-build/changelogger/src/main.rs b/tools/ci-build/changelogger/src/main.rs index 5e29945e34..344d83ba53 100644 --- a/tools/ci-build/changelogger/src/main.rs +++ b/tools/ci-build/changelogger/src/main.rs @@ -5,6 +5,8 @@ use anyhow::Result; use changelogger::init::subcommand_init; +use changelogger::ls::subcommand_ls; +use changelogger::new::subcommand_new; use changelogger::render::subcommand_render; use changelogger::split::subcommand_split; use clap::Parser; @@ -12,19 +14,25 @@ use clap::Parser; #[derive(Parser, Debug, Eq, PartialEq)] #[clap(name = "changelogger", author, version, about)] pub enum Args { - /// Split SDK changelog entries into a separate file - Split(changelogger::split::SplitArgs), + /// Print to stdout the empty "next" CHANGELOG template + Init(changelogger::init::InitArgs), + /// Create a new changelog entry Markdown file in the `smithy-rs/.changelog` directory + New(changelogger::new::NewArgs), + /// Render a preview of changelog entries since the last release + Ls(changelogger::ls::LsArgs), /// Render a TOML/JSON changelog into GitHub-flavored Markdown Render(changelogger::render::RenderArgs), - /// Print to stdout the empty "next" CHANGELOG template. - Init(changelogger::init::InitArgs), + /// Split SDK changelog entries into a separate file + Split(changelogger::split::SplitArgs), } fn main() -> Result<()> { match Args::parse() { - Args::Split(split) => subcommand_split(&split), - Args::Render(render) => subcommand_render(&render), Args::Init(init) => subcommand_init(&init), + Args::New(new) => subcommand_new(new), + Args::Ls(ls) => subcommand_ls(ls), + Args::Render(render) => subcommand_render(&render), + Args::Split(split) => subcommand_split(&split), } } @@ -32,10 +40,14 @@ fn main() -> Result<()> { mod tests { use super::Args; use changelogger::entry::ChangeSet; + use changelogger::ls::LsArgs; + use changelogger::new::NewArgs; use changelogger::render::RenderArgs; use changelogger::split::SplitArgs; use clap::Parser; + use smithy_rs_tool_common::changelog::{Reference, Target}; use std::path::PathBuf; + use std::str::FromStr; #[test] fn args_parsing() { @@ -188,5 +200,55 @@ mod tests { ]) .unwrap() ); + + assert_eq!( + Args::New(NewArgs { + applies_to: Some(vec![Target::Client, Target::AwsSdk]), + authors: Some(vec!["external-contrib".to_owned(), "ysaito1001".to_owned()]), + references: Some(vec![ + Reference::from_str("smithy-rs#1234").unwrap(), + Reference::from_str("aws-sdk-rust#5678").unwrap() + ]), + breaking: false, + new_feature: true, + bug_fix: false, + message: Some("Implement a long-awaited feature for S3".to_owned()), + basename: None, + }), + Args::try_parse_from([ + "./changelogger", + "new", + "--applies-to", + "client", + "--applies-to", + "aws-sdk-rust", + "--authors", + "external-contrib", + "--authors", + "ysaito1001", + "--references", + "smithy-rs#1234", + "--references", + "aws-sdk-rust#5678", + "--new-feature", + "--message", + "Implement a long-awaited feature for S3", + ]) + .unwrap() + ); + + assert_eq!( + Args::Ls(LsArgs { + change_set: ChangeSet::SmithyRs + }), + Args::try_parse_from(["./changelogger", "ls", "--change-set", "smithy-rs",]).unwrap() + ); + + assert_eq!( + Args::Ls(LsArgs { + change_set: ChangeSet::AwsSdk + }), + Args::try_parse_from(["./changelogger", "ls", "--change-set", "aws-sdk",]).unwrap() + ); } } diff --git a/tools/ci-build/changelogger/src/new.rs b/tools/ci-build/changelogger/src/new.rs new file mode 100644 index 0000000000..2f9983156c --- /dev/null +++ b/tools/ci-build/changelogger/src/new.rs @@ -0,0 +1,159 @@ +/* + * Copyright Amazon.com, Inc. or its affiliates. All Rights Reserved. + * SPDX-License-Identifier: Apache-2.0 + */ + +use anyhow::Context; +use clap::Parser; +use smithy_rs_tool_common::changelog::{FrontMatter, Markdown, Reference, Target}; +use smithy_rs_tool_common::git::find_git_repository_root; +use smithy_rs_tool_common::here; +use std::path::PathBuf; +use std::time::{SystemTime, UNIX_EPOCH}; + +#[derive(Parser, Debug, Eq, PartialEq)] +pub struct NewArgs { + /// Target audience for the change (if not provided, user's editor will open for authoring one) + #[clap(long, short = 't')] + pub applies_to: Option>, + /// List of git usernames for the authors of the change (if not provided, user's editor will open for authoring one) + #[clap(long, short)] + pub authors: Option>, + /// List of relevant issues and PRs (if not provided, user's editor will open for authoring one) + #[clap(long, short)] + pub references: Option>, + /// Whether or not the change contains a breaking change (defaults to false) + #[clap(long, action)] + pub breaking: bool, + /// Whether or not the change implements a new feature (defaults to false) + #[clap(long, action)] + pub new_feature: bool, + /// Whether or not the change fixes a bug (defaults to false) + #[clap(long, short, action)] + pub bug_fix: bool, + /// The changelog entry message (if not provided, user's editor will open for authoring one) + #[clap(long, short)] + pub message: Option, + /// Basename of a changelog markdown file (defaults to a random 6-digit basename) + #[clap(long)] + pub basename: Option, +} + +impl From for Markdown { + fn from(value: NewArgs) -> Self { + Markdown { + front_matter: FrontMatter { + applies_to: value.applies_to.unwrap_or_default().into_iter().collect(), + authors: value.authors.unwrap_or_default(), + references: value.references.unwrap_or_default(), + breaking: value.breaking, + new_feature: value.new_feature, + bug_fix: value.bug_fix, + }, + message: value.message.unwrap_or_default(), + } + } +} + +pub fn subcommand_new(args: NewArgs) -> anyhow::Result<()> { + let mut md_full_filename = find_git_repository_root("smithy-rs", ".").context(here!())?; + md_full_filename.push(".changelog"); + md_full_filename.push(args.basename.clone().unwrap_or(PathBuf::from(format!( + "{:?}.md", + SystemTime::now() + .duration_since(UNIX_EPOCH) + .expect("should get the current Unix epoch time") + .as_secs(), + )))); + + let changelog_entry = new_entry(Markdown::from(args))?; + std::fs::write(&md_full_filename, &changelog_entry).with_context(|| { + format!( + "failed to write the following changelog entry to {:?}:\n{}", + md_full_filename.as_path(), + changelog_entry + ) + })?; + + println!( + "\nThe following changelog entry has been written to {:?}:\n{}", + md_full_filename.as_path(), + changelog_entry + ); + + Ok(()) +} + +fn new_entry(markdown: Markdown) -> anyhow::Result { + // Due to the inability for `serde_yaml` to output single line array syntax, an array of values + // will be serialized as follows: + // + // key: + // - value1 + // - value2 + // + // as opposed to: + // + // key: [value1, value2] + // + // This doesn't present practical issues when rendering changelogs. See + // https://github.com/dtolnay/serde-yaml/issues/355 + let front_matter = serde_yaml::to_string(&markdown.front_matter)?; + let changelog_entry = format!("---\n{}---\n{}", front_matter, markdown.message); + let changelog_entry = if any_required_field_needs_to_be_filled(&markdown) { + edit::edit(changelog_entry).context("failed while editing changelog entry)")? + } else { + changelog_entry + }; + + Ok(changelog_entry) +} + +fn any_required_field_needs_to_be_filled(markdown: &Markdown) -> bool { + macro_rules! any_empty { + () => { false }; + ($head:expr $(, $tail:expr)*) => { + $head.is_empty() || any_empty!($($tail),*) + }; + } + any_empty!( + &markdown.front_matter.applies_to, + &markdown.front_matter.authors, + &markdown.front_matter.references, + &markdown.message + ) +} + +#[cfg(test)] +mod tests { + use crate::new::{any_required_field_needs_to_be_filled, new_entry, NewArgs}; + use smithy_rs_tool_common::changelog::{Reference, Target}; + use std::str::FromStr; + + #[test] + fn test_new_entry_from_args() { + // make sure `args` populates required fields (so the function + // `any_required_field_needs_to_be_filled` should return false), otherwise an editor would + // be opened during the test execution for human input, causing the test to get struck + let args = NewArgs { + applies_to: Some(vec![Target::Client]), + authors: Some(vec!["ysaito1001".to_owned()]), + references: Some(vec![Reference::from_str("smithy-rs#1234").unwrap()]), + breaking: false, + new_feature: true, + bug_fix: false, + message: Some("Implement a long-awaited feature for S3".to_owned()), + basename: None, + }; + let markdown = args.into(); + assert!( + !any_required_field_needs_to_be_filled(&markdown), + "one or more required fields were not populated" + ); + + let expected = "---\napplies_to:\n- client\nauthors:\n- ysaito1001\nreferences:\n- smithy-rs#1234\nbreaking: false\nnew_feature: true\nbug_fix: false\n---\nImplement a long-awaited feature for S3"; + let actual = new_entry(markdown).unwrap(); + + assert_eq!(expected, &actual); + } +} diff --git a/tools/ci-build/changelogger/src/render.rs b/tools/ci-build/changelogger/src/render.rs index ef41109bba..c3ec125779 100644 --- a/tools/ci-build/changelogger/src/render.rs +++ b/tools/ci-build/changelogger/src/render.rs @@ -475,7 +475,7 @@ fn render_crate_versions(crate_version_metadata_map: CrateVersionMetadataMap, ou /// Convert a list of changelog entries and crate versions into markdown. /// Returns (header, body) -fn render( +pub(crate) fn render( entries: &[ChangelogEntry], crate_version_metadata_map: CrateVersionMetadataMap, release_header: &str, diff --git a/tools/ci-build/sdk-lints/src/changelog.rs b/tools/ci-build/sdk-lints/src/changelog.rs index e3144948bb..d14845af8c 100644 --- a/tools/ci-build/sdk-lints/src/changelog.rs +++ b/tools/ci-build/sdk-lints/src/changelog.rs @@ -7,6 +7,7 @@ use crate::lint::LintError; use crate::{repo_root, Check, Lint}; use anyhow::Result; use smithy_rs_tool_common::changelog::{ChangelogLoader, ValidationSet}; +use std::fmt::Debug; use std::path::{Path, PathBuf}; pub(crate) struct ChangelogNext; @@ -22,7 +23,7 @@ impl Lint for ChangelogNext { } impl Check for ChangelogNext { - fn check(&self, path: impl AsRef) -> Result> { + fn check(&self, path: impl AsRef + Debug) -> Result> { match check_changelog_next(path) { Ok(_) => Ok(vec![]), Err(errs) => Ok(errs), @@ -34,7 +35,7 @@ impl Check for ChangelogNext { // and run the validation only when the directory has at least one changelog entry file, otherwise // a default constructed `ChangeLog` won't pass the validation. /// Validate that `CHANGELOG.next.toml` follows best practices -fn check_changelog_next(path: impl AsRef) -> std::result::Result<(), Vec> { +fn check_changelog_next(path: impl AsRef + Debug) -> std::result::Result<(), Vec> { let parsed = ChangelogLoader::default() .load_from_file(path) .map_err(|e| vec![LintError::via_display(e)])?; diff --git a/tools/ci-build/sdk-lints/src/lint.rs b/tools/ci-build/sdk-lints/src/lint.rs index 82592e5e12..8c773c12c6 100644 --- a/tools/ci-build/sdk-lints/src/lint.rs +++ b/tools/ci-build/sdk-lints/src/lint.rs @@ -6,7 +6,7 @@ use anyhow::Context; use std::borrow::Cow; -use std::fmt::{Display, Formatter}; +use std::fmt::{Debug, Display, Formatter}; use std::fs::read_to_string; use std::path::{Path, PathBuf}; @@ -119,7 +119,7 @@ pub(crate) trait Lint { } pub(crate) trait Check: Lint { - fn check(&self, path: impl AsRef) -> anyhow::Result>; + fn check(&self, path: impl AsRef + Debug) -> anyhow::Result>; } #[derive(Debug, Eq, PartialEq, Copy, Clone)] diff --git a/tools/ci-build/smithy-rs-tool-common/src/changelog.rs b/tools/ci-build/smithy-rs-tool-common/src/changelog.rs index 0be4cfd183..78484962a4 100644 --- a/tools/ci-build/smithy-rs-tool-common/src/changelog.rs +++ b/tools/ci-build/smithy-rs-tool-common/src/changelog.rs @@ -10,7 +10,9 @@ pub mod parser; use crate::changelog::parser::{ParseIntoChangelog, ParserChain}; use anyhow::{bail, Context, Result}; use serde::{de, Deserialize, Deserializer, Serialize}; +use std::collections::HashSet; use std::fmt; +use std::fmt::Debug; use std::path::Path; use std::str::FromStr; @@ -69,7 +71,7 @@ pub struct Meta { pub target: Option, } -#[derive(Clone, Debug)] +#[derive(Clone, Debug, Eq, PartialEq)] pub struct Reference { pub repo: String, pub number: usize, @@ -130,10 +132,19 @@ enum AuthorsInner { Multiple(Vec), } -#[derive(Clone, Debug, Default, PartialEq, Serialize, Deserialize)] +#[derive(Clone, Debug, Default, Serialize, Deserialize)] #[serde(from = "AuthorsInner", into = "AuthorsInner")] pub struct Authors(pub(super) Vec); +impl PartialEq for Authors { + fn eq(&self, other: &Self) -> bool { + // `true` if two `Authors` contain the same set of authors, regardless of their order + self.0.iter().collect::>() == other.0.iter().collect::>() + } +} + +impl Eq for Authors {} + impl From for Authors { fn from(value: AuthorsInner) -> Self { match value { @@ -317,6 +328,87 @@ impl Changelog { } } +#[derive(Copy, Clone, Debug, Deserialize, Eq, Hash, PartialEq, Serialize)] +pub enum Target { + #[serde(rename = "client")] + Client, + #[serde(rename = "server")] + Server, + #[serde(rename = "aws-sdk-rust")] + AwsSdk, +} + +impl FromStr for Target { + type Err = anyhow::Error; + + fn from_str(sdk: &str) -> std::result::Result { + match sdk.to_lowercase().as_str() { + "client" => Ok(Target::Client), + "server" => Ok(Target::Server), + "aws-sdk-rust" => Ok(Target::AwsSdk), + _ => bail!("An invalid type of `Target` {sdk} has been specified"), + } + } +} + +#[derive(Clone, Debug, Default, Deserialize, Serialize)] +pub struct FrontMatter { + pub applies_to: HashSet, + pub authors: Vec, + pub references: Vec, + pub breaking: bool, + pub new_feature: bool, + pub bug_fix: bool, +} + +#[derive(Clone, Debug, Default)] +pub struct Markdown { + pub front_matter: FrontMatter, + pub message: String, +} + +impl From for Changelog { + fn from(value: Markdown) -> Self { + let front_matter = value.front_matter; + let entry = HandAuthoredEntry { + message: value.message.trim_start().to_owned(), + meta: Meta { + bug: front_matter.bug_fix, + breaking: front_matter.breaking, + tada: front_matter.new_feature, + target: None, + }, + authors: AuthorsInner::Multiple(front_matter.authors).into(), + references: front_matter.references, + since_commit: None, + age: None, + }; + + let mut changelog = Changelog::new(); + + // Bin `entry` into the appropriate `Vec` based on the `applies_to` field in `front_matter` + if front_matter.applies_to.contains(&Target::AwsSdk) { + changelog.aws_sdk_rust.push(entry.clone()) + } + if front_matter.applies_to.contains(&Target::Client) + && front_matter.applies_to.contains(&Target::Server) + { + let mut entry = entry.clone(); + entry.meta.target = Some(SdkAffected::All); + changelog.smithy_rs.push(entry); + } else if front_matter.applies_to.contains(&Target::Client) { + let mut entry = entry.clone(); + entry.meta.target = Some(SdkAffected::Client); + changelog.smithy_rs.push(entry); + } else if front_matter.applies_to.contains(&Target::Server) { + let mut entry = entry.clone(); + entry.meta.target = Some(SdkAffected::Server); + changelog.smithy_rs.push(entry); + } + + changelog + } +} /// Parses changelog entries into [`Changelog`] using a series of parsers. /// /// Each parser will attempt to parse an input string in order: @@ -336,10 +428,11 @@ impl ChangelogLoader { } /// Parses the contents of a file located at `path` into `Changelog` - pub fn load_from_file(&self, path: impl AsRef) -> Result { + pub fn load_from_file(&self, path: impl AsRef + Debug) -> Result { let contents = std::fs::read_to_string(path.as_ref()) .with_context(|| format!("failed to read {:?}", path.as_ref()))?; self.parse_str(contents) + .with_context(|| format!("failed to parse the contents in {path:?}")) } /// Parses the contents of files stored in a directory `dir_path` into `Changelog` @@ -347,7 +440,7 @@ impl ChangelogLoader { /// It opens each file in the directory, parses the file contents into `Changelog`, /// and merges it with accumulated `Changelog`. It currently does not support loading /// from recursive directory structures. - pub fn load_from_dir(&self, dir_path: impl AsRef) -> Result { + pub fn load_from_dir(&self, dir_path: impl AsRef + Debug) -> Result { let entries = std::fs::read_dir(dir_path.as_ref())?; let result = entries .into_iter() diff --git a/tools/ci-build/smithy-rs-tool-common/src/changelog/parser.rs b/tools/ci-build/smithy-rs-tool-common/src/changelog/parser.rs index 27af7ba63e..7b6827ef6c 100644 --- a/tools/ci-build/smithy-rs-tool-common/src/changelog/parser.rs +++ b/tools/ci-build/smithy-rs-tool-common/src/changelog/parser.rs @@ -3,10 +3,8 @@ * SPDX-License-Identifier: Apache-2.0 */ -use crate::changelog::{Authors, Changelog, HandAuthoredEntry, Meta, Reference, SdkAffected}; +use crate::changelog::{Changelog, Markdown, SdkAffected}; use anyhow::{bail, Context, Result}; -use serde::{Deserialize, Serialize}; -use std::collections::HashSet; use std::fmt::Debug; pub(crate) trait ParseIntoChangelog: Debug { @@ -45,75 +43,6 @@ impl ParseIntoChangelog for Json { } } -#[derive(Copy, Clone, Debug, Deserialize, Hash, Serialize, PartialEq, Eq)] -enum Target { - #[serde(rename = "client")] - Client, - #[serde(rename = "server")] - Server, - #[serde(rename = "aws-sdk-rust")] - AwsSdk, -} - -#[derive(Clone, Debug, Default, Deserialize)] -struct FrontMatter { - applies_to: HashSet, - authors: Authors, - references: Vec, - breaking: bool, - new_feature: bool, - bug_fix: bool, -} - -#[derive(Clone, Debug, Default)] -struct Markdown { - front_matter: FrontMatter, - message: String, -} - -impl From for Changelog { - fn from(value: Markdown) -> Self { - let front_matter = value.front_matter; - let entry = HandAuthoredEntry { - message: value.message.trim_start().to_owned(), - meta: Meta { - bug: front_matter.bug_fix, - breaking: front_matter.breaking, - tada: front_matter.new_feature, - target: None, - }, - authors: front_matter.authors, - references: front_matter.references, - since_commit: None, - age: None, - }; - - let mut changelog = Changelog::new(); - - // Bin `entry` into the appropriate `Vec` based on the `applies_to` field in `front_matter` - if front_matter.applies_to.contains(&Target::AwsSdk) { - changelog.aws_sdk_rust.push(entry.clone()) - } - if front_matter.applies_to.contains(&Target::Client) - && front_matter.applies_to.contains(&Target::Server) - { - let mut entry = entry.clone(); - entry.meta.target = Some(SdkAffected::All); - changelog.smithy_rs.push(entry); - } else if front_matter.applies_to.contains(&Target::Client) { - let mut entry = entry.clone(); - entry.meta.target = Some(SdkAffected::Client); - changelog.smithy_rs.push(entry); - } else if front_matter.applies_to.contains(&Target::Server) { - let mut entry = entry.clone(); - entry.meta.target = Some(SdkAffected::Server); - changelog.smithy_rs.push(entry); - } - - changelog - } -} - impl ParseIntoChangelog for Markdown { fn parse(&self, value: &str) -> Result { let mut parts = value.splitn(3, "---"); @@ -153,17 +82,18 @@ impl Default for ParserChain { impl ParseIntoChangelog for ParserChain { fn parse(&self, value: &str) -> Result { - for (name, parser) in &self.parsers { + let mut errs = Vec::new(); + for (_name, parser) in &self.parsers { match parser.parse(value) { Ok(parsed) => { return Ok(parsed); } Err(err) => { - tracing::debug!(parser = %name, err = %err, "failed to parse the input string"); + errs.push(err.to_string()); } } } - bail!("no parsers in chain parsed ${value} into `Changelog`") + bail!("no parsers in chain parsed the following into `Changelog`\n{value}\nwith respective errors: \n{errs:?}") } } @@ -336,5 +266,32 @@ This is some **Markdown** content. ); assert!(changelog.aws_sdk_rust.is_empty()); } + { + let markdown = r#"--- +applies_to: +- client +- aws-sdk-rust +authors: +- ysaito1001 +references: +- smithy-rs#1234 +breaking: false +new_feature: false +bug_fix: true +--- +Fix a long-standing bug. +"#; + let changelog = Markdown::default().parse(markdown).unwrap(); + assert_eq!(1, changelog.smithy_rs.len()); + assert_eq!( + Some(SdkAffected::Client), + changelog.smithy_rs[0].meta.target + ); + assert_eq!( + "Fix a long-standing bug.\n", + &changelog.smithy_rs[0].message + ); + assert_eq!(1, changelog.aws_sdk_rust.len()); + } } }