diff --git a/tools/changelogger/src/split.rs b/tools/changelogger/src/split.rs index 1a020a7a8..cd76496c9 100644 --- a/tools/changelogger/src/split.rs +++ b/tools/changelogger/src/split.rs @@ -8,8 +8,12 @@ use clap::Parser; use smithy_rs_tool_common::changelog::Changelog; use smithy_rs_tool_common::git::{find_git_repository_root, Git, GitCLI}; use std::path::{Path, PathBuf}; -use std::{env, fs}; +use std::{env, fs, mem}; +// Value chosen arbitrarily. It is large enough that we're unlikely to lose +// SDK changelog entries, but small enough that the SDK changelog file +// doesn't get too long. +const MAX_ENTRY_AGE: usize = 5; const INTERMEDIATE_SOURCE_HEADER: &str = "# This is an intermediate file that will be replaced after automation is complete.\n\ # It will be used to generate a changelog entry for smithy-rs.\n\ @@ -38,20 +42,32 @@ pub struct SplitArgs { } pub fn subcommand_split(args: &SplitArgs) -> Result<()> { - let changelog = Changelog::load_from_file(&args.source).map_err(|errs| { + let combined_changelog = Changelog::load_from_file(&args.source).map_err(|errs| { anyhow::Error::msg(format!( "cannot split changelogs with changelog errors: {:#?}", errs )) })?; + let current_sdk_changelog = if args.destination.exists() { + Changelog::load_from_file(&args.destination).map_err(|errs| { + anyhow::Error::msg(format!( + "failed to load existing SDK changelog entries: {:#?}", + errs + )) + })? + } else { + Changelog::new() + }; - let (source_log, dest_log) = ( - smithy_rs_entries(changelog.clone()), - sdk_entries(args, changelog).context("failed to filter SDK entries")?, + let (smithy_rs_entries, new_sdk_entries) = ( + smithy_rs_entries(combined_changelog.clone()), + sdk_entries(args, combined_changelog).context("failed to filter SDK entries")?, ); - write_entries(&args.source, INTERMEDIATE_SOURCE_HEADER, &source_log) + let sdk_changelog = merge_sdk_entries(current_sdk_changelog, new_sdk_entries); + + write_entries(&args.source, INTERMEDIATE_SOURCE_HEADER, &smithy_rs_entries) .context("failed to write source")?; - write_entries(&args.destination, DEST_HEADER, &dest_log) + write_entries(&args.destination, DEST_HEADER, &sdk_changelog) .context("failed to write destination")?; Ok(()) } @@ -86,8 +102,72 @@ fn smithy_rs_entries(mut changelog: Changelog) -> Changelog { changelog } +fn merge_sdk_entries(old_changelog: Changelog, new_changelog: Changelog) -> Changelog { + let mut merged = old_changelog; + merged.merge(new_changelog); + + for entry in &mut merged.aws_sdk_rust { + *entry.age.get_or_insert(0) += 1; + } + + let mut to_filter = Vec::new(); + mem::swap(&mut merged.aws_sdk_rust, &mut to_filter); + merged.aws_sdk_rust = to_filter + .into_iter() + .filter(|entry| entry.age.expect("set above") <= MAX_ENTRY_AGE) + .collect(); + + merged +} + fn write_entries(into_path: &Path, header: &str, changelog: &Changelog) -> Result<()> { let json_changelog = changelog.to_json_string()?; fs::write(into_path, format!("{header}\n{json_changelog}"))?; Ok(()) } + +#[cfg(test)] +mod tests { + use super::{merge_sdk_entries, MAX_ENTRY_AGE}; + use smithy_rs_tool_common::changelog::{Changelog, HandAuthoredEntry}; + + #[test] + fn test_merge_sdk_entries() { + let mut old_entries = Changelog::new(); + old_entries.aws_sdk_rust.push(HandAuthoredEntry { + message: "old-1".into(), + age: None, + ..Default::default() + }); + old_entries.aws_sdk_rust.push(HandAuthoredEntry { + message: "old-2".into(), + age: Some(MAX_ENTRY_AGE), + ..Default::default() + }); + old_entries.aws_sdk_rust.push(HandAuthoredEntry { + message: "old-3".into(), + age: Some(1), + ..Default::default() + }); + + let mut new_entries = Changelog::new(); + new_entries.aws_sdk_rust.push(HandAuthoredEntry { + message: "new-1".into(), + ..Default::default() + }); + new_entries.aws_sdk_rust.push(HandAuthoredEntry { + message: "new-2".into(), + ..Default::default() + }); + + let combined = merge_sdk_entries(old_entries, new_entries); + assert_eq!("old-1", combined.aws_sdk_rust[0].message); + assert_eq!(Some(1), combined.aws_sdk_rust[0].age); + assert_eq!("old-3", combined.aws_sdk_rust[1].message); + assert_eq!(Some(2), combined.aws_sdk_rust[1].age); + assert_eq!("new-1", combined.aws_sdk_rust[2].message); + assert_eq!(Some(1), combined.aws_sdk_rust[2].age); + assert_eq!("new-2", combined.aws_sdk_rust[3].message); + assert_eq!(Some(1), combined.aws_sdk_rust[3].age); + } +} diff --git a/tools/changelogger/tests/e2e_test.rs b/tools/changelogger/tests/e2e_test.rs index 13ecb13c7..ee07e7445 100644 --- a/tools/changelogger/tests/e2e_test.rs +++ b/tools/changelogger/tests/e2e_test.rs @@ -6,6 +6,7 @@ use changelogger::entry::ChangeSet; use changelogger::render::{subcommand_render, RenderArgs, EXAMPLE_ENTRY, USE_UPDATE_CHANGELOGS}; use changelogger::split::{subcommand_split, SplitArgs}; +use smithy_rs_tool_common::changelog::{Changelog, HandAuthoredEntry}; use smithy_rs_tool_common::git::{CommitHash, Git, GitCLI}; use smithy_rs_tool_common::shell::handle_failure; use std::fs; @@ -105,7 +106,29 @@ fn split_aws_sdk_test() { create_fake_repo_root(tmp_dir.path(), "0.42.0", "0.12.0"); fs::write(&source_path, SOURCE_TOML).unwrap(); - fs::write(&dest_path, "overwrite-me").unwrap(); + + let mut original_dest_changelog = Changelog::new(); + original_dest_changelog + .aws_sdk_rust + .push(HandAuthoredEntry { + message: "old-existing-entry-1".into(), + // this entry should get filtered out since it's too old + age: Some(5), + ..Default::default() + }); + original_dest_changelog + .aws_sdk_rust + .push(HandAuthoredEntry { + message: "old-existing-entry-2".into(), + age: Some(2), + since_commit: Some("commit-old-existing-entry-2".into()), + ..Default::default() + }); + fs::write( + &dest_path, + original_dest_changelog.to_json_string().unwrap(), + ) + .unwrap(); subcommand_split(&SplitArgs { source: source_path.clone(), @@ -153,6 +176,18 @@ fn split_aws_sdk_test() { { "smithy-rs": [], "aws-sdk-rust": [ + { + "message": "old-existing-entry-2", + "meta": { + "bug": false, + "breaking": false, + "tada": false + }, + "author": "", + "references": [], + "since-commit": "commit-old-existing-entry-2", + "age": 3 + }, { "message": "Some change", "meta": { @@ -165,7 +200,8 @@ fn split_aws_sdk_test() { "aws-sdk-rust#123", "smithy-rs#456" ], - "since-commit": "test-commit-hash" + "since-commit": "test-commit-hash", + "age": 1 }, { "message": "Some other change", @@ -179,7 +215,8 @@ fn split_aws_sdk_test() { "aws-sdk-rust#234", "smithy-rs#567" ], - "since-commit": "test-commit-hash" + "since-commit": "test-commit-hash", + "age": 1 } ], "aws-sdk-model": [] diff --git a/tools/smithy-rs-tool-common/src/changelog.rs b/tools/smithy-rs-tool-common/src/changelog.rs index 893828955..e6524dc9b 100644 --- a/tools/smithy-rs-tool-common/src/changelog.rs +++ b/tools/smithy-rs-tool-common/src/changelog.rs @@ -61,7 +61,7 @@ impl<'de> Deserialize<'de> for SdkAffected { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] #[serde(deny_unknown_fields)] pub struct Meta { pub bug: bool, @@ -125,7 +125,7 @@ impl FromStr for Reference { } } -#[derive(Clone, Debug, Deserialize, Serialize)] +#[derive(Clone, Debug, Default, Deserialize, Serialize)] pub struct HandAuthoredEntry { pub message: String, pub meta: Meta, @@ -135,6 +135,11 @@ pub struct HandAuthoredEntry { /// Optional commit hash to indicate "since when" these changes were made #[serde(rename = "since-commit")] pub since_commit: Option, + /// Optional age of this entry, for the SDK use-case where entries must be + /// preserved across multiple smithy-rs releases. This allows the changelogger + /// to eventually cull older entries. + #[serde(skip_serializing_if = "Option::is_none")] + pub age: Option, } impl HandAuthoredEntry {