diff --git a/docs/cli-schema.json b/docs/cli-schema.json index 44e750515d..211628c240 100644 --- a/docs/cli-schema.json +++ b/docs/cli-schema.json @@ -2861,7 +2861,7 @@ "changelog" ], "name": "bundle-amend", - "summary": "Append additional changelog entries to a published bundle without modifying it.", + "summary": "Append or exclude changelog entries in a published bundle without modifying it.", "notes": "Creates an immutable .amend-N.yaml sidecar file alongside the original bundle.", "usage": "docs-builder changelog bundle-amend \u003Cbundle-path\u003E [options]", "examples": [], @@ -2893,7 +2893,16 @@ "name": "add", "type": "array", "required": false, - "summary": "Required: Path(s) to changelog YAML file(s) to add as comma-separated values (e.g., --add \u0022file1.yaml,file2.yaml\u0022). Supports tilde (~) expansion and relative paths.", + "summary": "Optional: Changelog YAML paths to add. Repeat --add or pass a comma-separated list in one value (for example, --add \u0022file1.yaml,file2.yaml\u0022). Supports tilde (~) expansion and relative paths.", + "repeatable": true, + "elementType": "string" + }, + { + "role": "flag", + "name": "remove", + "type": "array", + "required": false, + "summary": "Optional: Changelog YAML paths to exclude from the effective bundle. Repeat --remove or pass a comma-separated list in one value. Supports tilde (~) expansion and relative paths.", "repeatable": true, "elementType": "string" }, @@ -2902,9 +2911,25 @@ "name": "resolve", "type": "boolean", "required": false, - "summary": "Optional: Copy the contents of each changelog file into the entries array. Use --no-resolve to explicitly turn off resolve (overrides inference from original bundle).", + "summary": "Optional: When using --add, inline each added changelog\u0027s content in the amend file. Use --no-resolve to record file references only. When omitted, inferred from the parent bundle. Does not apply to --remove.", "defaultValue": "default" }, + { + "role": "flag", + "name": "force", + "type": "boolean", + "required": false, + "summary": "Optional: When removing, match by file name even if the bundle checksum differs from the file on disk.", + "defaultValue": "false" + }, + { + "role": "flag", + "name": "dry-run", + "type": "boolean", + "required": false, + "summary": "Optional: Preview changes without writing an amend file.", + "defaultValue": "false" + }, { "role": "flag", "name": "log-level", diff --git a/docs/cli/changelog/cmd-bundle-amend.md b/docs/cli/changelog/cmd-bundle-amend.md index 2ec88dbb02..b13d4cd7ff 100644 --- a/docs/cli/changelog/cmd-bundle-amend.md +++ b/docs/cli/changelog/cmd-bundle-amend.md @@ -1,14 +1,16 @@ ## Description -Amend a bundle with additional changelog entries. +Amend a bundle with additional or excluded changelog entries without modifying the parent bundle file. Amend bundles follow a specific naming convention: `{parent-bundle-name}.amend-{N}.yaml` where `{N}` is a sequence number. +Specify at least one of `--add` or `--remove`. + To create a bundle, use [](/cli/changelog/bundle.md). For details and examples, go to [](/contribute/bundle-changelogs.md). ## Resolve behaviour -By default, the `bundle-amend` command **infers** whether to resolve entries from the original bundle. +By default, the `bundle-amend` command **infers** whether to resolve entries from the original bundle when you use `--add`. If the original bundle contains resolved entries (with inline `title`, `type`, and so on), the amend file will also be resolved. If the original bundle contains only file references, the amend file will also contain only file references. @@ -19,9 +21,13 @@ You can override this behaviour: - `--resolve`: Force entries to be resolved (inline content), regardless of the original bundle. - `--no-resolve`: Force entries to contain only file references, regardless of the original bundle. +`--resolve` and `--no-resolve` apply only to `--add`. Removals always record `exclude-entries` with file name and checksum. + ## Output -Amend bundles contain only the additional entries, not a full repetition of the original bundle. For example: +Amend bundles contain only the changes for that amend file, not a full repetition of the original bundle. + +Additions: ```yaml # 9.3.0.amend-1.yaml @@ -31,8 +37,20 @@ entries: checksum: abc123def456 ``` -When bundles are loaded (either via the `changelog render` command or the `{changelog}` directive), amend files are **automatically merged** with their parent bundles. -The entries from all matching amend files are combined with the parent bundle's entries, and the result is rendered as a single release. +Removals: + +```yaml +# 9.3.0.amend-2.yaml +exclude-entries: +- file: + name: 138723.yaml + checksum: def456abc123 +``` + +An amend file can contain both `exclude-entries` and `entries`. Within each amend file, exclusions are applied before additions. + +When bundles are loaded (either via the `changelog render` command or the `{changelog}` directive), amend files are **automatically merged** with their parent bundles in sequence (`amend-1`, `amend-2`, …). +The result is rendered as a single release. :::{note} Amend bundles do not need to include `products` or `hide-features` fields — they inherit these from their parent bundle. If an amend bundle is found without a matching parent bundle, it remains standalone. @@ -52,14 +70,53 @@ docs-builder changelog bundle-amend \ The new bundle automatically matches the resolve style of the original bundle. +### Remove a changelog from a bundle + +```sh +docs-builder changelog bundle-amend \ + ./docs/changelog/bundles/9.3.0.yaml \ + --remove ./docs/changelog/138723.yaml +``` + +The CLI computes the file checksum automatically and matches it against the effective bundle (parent plus any existing amend files). +If the bundle contains the file with a different checksum, the command fails unless you pass `--force` to remove by file name only. + ### Add multiple changelogs to a bundle +Comma-separated list: + ```sh docs-builder changelog bundle-amend \ ./docs/changelog/bundles/9.3.0.yaml \ --add "./docs/changelog/138723.yaml,./docs/changelog/1770424335.yaml" ``` +Or repeat `--add`: + +```sh +docs-builder changelog bundle-amend \ + ./docs/changelog/bundles/9.3.0.yaml \ + --add ./docs/changelog/138723.yaml \ + --add ./docs/changelog/1770424335.yaml +``` + +### Remove multiple changelogs from a bundle + +```sh +docs-builder changelog bundle-amend \ + ./docs/changelog/bundles/9.3.0.yaml \ + --remove "./docs/changelog/old-a.yaml,./docs/changelog/old-b.yaml" +``` + +### Replace an entry in one amend file + +```sh +docs-builder changelog bundle-amend \ + ./docs/changelog/bundles/9.3.0.yaml \ + --remove ./docs/changelog/old-entry.yaml \ + --add ./docs/changelog/new-entry.yaml +``` + ### Force resolve or file-reference style ```sh @@ -74,3 +131,12 @@ docs-builder changelog bundle-amend 9.3.0.yaml \ --add ./docs/changelog/late-addition.yaml \ --no-resolve ``` + +### Preview without writing an amend file + +```sh +docs-builder changelog bundle-amend \ + ./docs/changelog/bundles/9.3.0.yaml \ + --remove ./docs/changelog/138723.yaml \ + --dry-run +``` diff --git a/docs/cli/changelog/cmd-remove.md b/docs/cli/changelog/cmd-remove.md index f10fd4a130..6486aa3995 100644 --- a/docs/cli/changelog/cmd-remove.md +++ b/docs/cli/changelog/cmd-remove.md @@ -11,6 +11,16 @@ Before deleting anything, the command checks whether any matching files are refe For more context, go to [](/contribute/bundle-changelogs.md#changelog-remove). +## Bundle dependency check + +The dependency scan considers the **effective** entry list for each parent bundle: parent `entries` merged with amend sidecars (`{bundle}.amend-N.yaml`) in numeric order. Within each amend file, `exclude-entries` are applied before additions, matching how bundles are loaded for [`changelog render`](/cli/changelog/render.md) and the `{changelog}` directive. + +A changelog file blocks deletion only when it still appears in that effective list as an **unresolved** entry (the bundle references the file by name and checksum rather than embedding inline title and type). Resolved entries do not require the source file on disk. + +If you removed an entry from a bundle with [`changelog bundle-amend --remove`](/cli/changelog/bundle-amend.md), the corresponding `exclude-entries` record drops it from the effective list, so `changelog remove` can delete the source file even when the parent bundle still lists it. + +Amend sidecar files are not scanned as parent bundles themselves—only the parent bundle file plus its amend chain is evaluated. If an amend file cannot be parsed, the command logs a warning and uses the parent bundle entries only for that dependency check. + ## Directory resolution Both modes use the same ordered fallback to locate changelog YAML files and existing bundles. diff --git a/docs/contribute/bundle-changelogs.md b/docs/contribute/bundle-changelogs.md index 5fbb1aed59..e10eeff989 100644 --- a/docs/contribute/bundle-changelogs.md +++ b/docs/contribute/bundle-changelogs.md @@ -249,9 +249,16 @@ docs-builder changelog bundle-amend \ Amend bundles follow a specific naming convention: `{parent-bundle-name}.amend-{N}.yaml` where `{N}` is a sequence number. -:::{note} -There is currently no command to **remove** changelogs from a bundle. You must edit the bundle file manually or else re-generate the bundle with an updated source of truth or a new rule that excludes the changelog. -::: +To remove entries from an existing bundle without editing the parent file, use `--remove` on the same command: + +```sh +docs-builder changelog bundle-amend \ + ./docs/releases/9.3.0.yaml \ + --remove "./docs/changelog/138723.yaml" +``` + +This creates an amend file with `exclude-entries` that is merged when the bundle is rendered. +After excluding an entry from unresolved bundles, you can use `changelog remove` to delete the source changelog file. When bundles are turned into docs (either via the `changelog render` command or the `{changelog}` directive), amend files are **automatically merged** with their parent bundles. The changelogs from all matching amend files are combined with the parent bundle's changelogs and the result is rendered as a single release. diff --git a/docs/syntax/changelog.md b/docs/syntax/changelog.md index 1c28a02844..29ba749f90 100644 --- a/docs/syntax/changelog.md +++ b/docs/syntax/changelog.md @@ -220,7 +220,7 @@ Bundles with the same target version/date are automatically merged into a single Bundles can have associated **amend files** that follow the naming pattern `{bundle-name}.amend-{N}.yaml` (e.g., `9.3.0.amend-1.yaml`). When loading bundles, the directive automatically discovers and merges amend files with their parent bundles. -This allows you to add late additions to a release without modifying the original bundle file: +This allows you to add or remove late changes to a release without modifying the original bundle file: ``` bundles/ @@ -229,6 +229,8 @@ bundles/ └── 9.3.0.amend-2.yaml # Second amend (auto-merged with parent) ``` +Amend files may contain `entries` (additions) and `exclude-entries` (removals). Within each amend file, exclusions are applied before additions. Amend files are processed in numeric order. + All entries from the parent and amend bundles are rendered together as a single release section. The parent bundle's metadata (products, hide-features, repo) is preserved. ## Default folder structure @@ -320,8 +322,9 @@ This prevents silent data loss where changelog entries would be quietly omitted To fix this, either: - Restore the missing changelog files, or -- Re-create the bundle with `--resolve` to embed entry content directly (making the bundle self-contained), or -- Remove the unresolvable entry from the bundle file. +- Re-create the bundle with `--resolve` to embed entry content directly (making the bundle self-contained). + +`bundle-amend --remove` only applies when the source changelog file is still available (for example, to drop an entry from the effective bundle before you delete the file with `changelog remove`). :::{tip} In general, if you want to be able to remove changelog files after your releases, create your bundles with the `--resolve` option or set `bundle.resolve` to `true` in the changelog configuration file. diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs index 11e72a87fa..8e4406319b 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/Bundle.cs @@ -30,6 +30,11 @@ public sealed record BundleDto [YamlMember(Alias = "hide-features", ApplyNamingConventions = false)] public List? HideFeatures { get; set; } public List? Entries { get; set; } + /// + /// Entries to exclude when this amend file is merged with its parent bundle. + /// + [YamlMember(Alias = "exclude-entries", ApplyNamingConventions = false)] + public List? ExcludeEntries { get; set; } } /// diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleAmendMerger.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleAmendMerger.cs new file mode 100644 index 0000000000..aef091d154 --- /dev/null +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleAmendMerger.cs @@ -0,0 +1,113 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using System.Globalization; +using System.Text.RegularExpressions; +using Elastic.Documentation.ReleaseNotes; + +namespace Elastic.Documentation.Configuration.ReleaseNotes; + +/// +/// Merges parent bundle entries with amend files (exclusions first, then additions per amend). +/// +public static partial class BundleAmendMerger +{ + [GeneratedRegex(@"\.amend-(\d+)\.ya?ml$", RegexOptions.IgnoreCase)] + private static partial Regex AmendFileRegex(); + + /// Whether a path is an amend sidecar ({name}.amend-{N}.yaml). + public static bool IsAmendFile(string filePath) => AmendFileRegex().IsMatch(filePath); + + /// Numeric suffix from an amend file path; 0 when not an amend file. + public static int GetAmendFileNumber(string filePath) + { + var match = AmendFileRegex().Match(filePath); + return match.Success && int.TryParse(match.Groups[1].Value, NumberStyles.Integer, CultureInfo.InvariantCulture, out var number) + ? number + : 0; + } + + /// + /// Applies amend bundles in order to parent entries and returns the effective entry list. + /// + public static List MergeEntries( + IReadOnlyList parentEntries, + IReadOnlyList amendBundlesInOrder) + { + var current = parentEntries.ToList(); + foreach (var amend in amendBundlesInOrder) + current = ApplySingleAmend(current, amend); + return current; + } + + /// + /// Collects all exclusion keys already applied by prior amend files. + /// + public static HashSet CollectAppliedExclusionKeys( + IReadOnlyList amendBundlesInOrder) + { + var keys = new HashSet(StringComparer.OrdinalIgnoreCase); + foreach (var amend in amendBundlesInOrder) + { + foreach (var exclusion in amend.ExcludeEntries) + _ = keys.Add(BuildExclusionKey(exclusion)); + } + return keys; + } + + /// Whether a bundled entry matches an exclusion record. + public static bool EntryMatchesExclusion(BundledEntry entry, BundledEntry exclusion) + { + var entryFileName = NormalizeFileName(entry.File?.Name); + var exclusionFileName = NormalizeFileName(exclusion.File?.Name); + if (string.IsNullOrEmpty(entryFileName) || string.IsNullOrEmpty(exclusionFileName)) + return false; + + if (!string.Equals(entryFileName, exclusionFileName, StringComparison.OrdinalIgnoreCase)) + return false; + + if (string.IsNullOrWhiteSpace(exclusion.File?.Checksum)) + return true; + + return string.Equals(entry.File?.Checksum, exclusion.File.Checksum, StringComparison.OrdinalIgnoreCase); + } + + /// Builds a stable key for an exclusion record (file name + checksum). + public static string BuildExclusionKey(BundledEntry exclusion) + { + var name = NormalizeFileName(exclusion.File?.Name) ?? string.Empty; + var checksum = exclusion.File?.Checksum ?? string.Empty; + return $"{name}|{checksum}"; + } + + private static List ApplySingleAmend(IReadOnlyList entries, Bundle amend) + { + var result = ApplyExclusions(entries, amend.ExcludeEntries); + if (amend.Entries.Count > 0) + result.AddRange(amend.Entries); + return result; + } + + private static List ApplyExclusions( + IReadOnlyList entries, + IReadOnlyList exclusions) + { + if (exclusions.Count == 0) + return entries.ToList(); + + return entries + .Where(entry => !exclusions.Any(exclusion => EntryMatchesExclusion(entry, exclusion))) + .ToList(); + } + + private static string? NormalizeFileName(string? fileName) + { + if (string.IsNullOrWhiteSpace(fileName)) + return null; + + var normalized = fileName.Replace('\\', '/'); + var lastSlash = normalized.LastIndexOf('/'); + return lastSlash >= 0 ? normalized[(lastSlash + 1)..] : normalized; + } +} diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cs index 9dabf2d9ed..e320cbd6ec 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/BundleLoader.cs @@ -53,7 +53,7 @@ public IReadOnlyList LoadBundles( } // Merge amend files with their parent bundles - loadedBundles = MergeAmendFiles(loadedBundles); + loadedBundles = MergeAmendFiles(loadedBundles, emitWarning); return loadedBundles; } @@ -262,63 +262,63 @@ private static LoadedBundle MergeBundleGroup(IGrouping gro /// Amend files follow the naming pattern: {baseName}.amend-{N}.yaml /// /// The list of loaded bundles including amend files. + /// Callback to emit warnings during entry resolution. /// A list of bundles with amend file entries merged into their parent bundles. - private List MergeAmendFiles(List bundles) + private List MergeAmendFiles(List bundles, Action emitWarning) { if (bundles.Count <= 1) return bundles; - // Build a lookup of bundles by their file path for quick access var bundlesByPath = bundles.ToDictionary(b => b.FilePath, StringComparer.OrdinalIgnoreCase); - - // Identify amend files and their parent bundles - var amendBundles = bundles.Where(b => IsAmendFile(b.FilePath)).ToList(); + var amendBundles = bundles.Where(b => BundleAmendMerger.IsAmendFile(b.FilePath)).ToList(); if (amendBundles.Count == 0) return bundles; - // Track which bundles to remove (amend files that were merged) var mergedAmendPaths = new HashSet(StringComparer.OrdinalIgnoreCase); - - // Track parent bundles with their merged entries var mergedParents = new Dictionary(StringComparer.OrdinalIgnoreCase); - foreach (var amendBundle in amendBundles) + var amendsByParent = amendBundles + .GroupBy(a => GetParentBundlePath(a.FilePath)) + .Where(group => group.Key != null); + + foreach (var group in amendsByParent) { - var parentPath = GetParentBundlePath(amendBundle.FilePath); + var parentPath = group.Key!; + if (!bundlesByPath.TryGetValue(parentPath, out var parentBundle)) + continue; - if (parentPath == null || !bundlesByPath.TryGetValue(parentPath, out var parentBundle)) - continue; // No parent found, amend file will remain standalone + var orderedAmendData = group + .OrderBy(a => BundleAmendMerger.GetAmendFileNumber(a.FilePath)) + .Select(a => a.Data) + .ToList(); - // Get or create the merged parent entry - if (!mergedParents.TryGetValue(parentPath, out var mergedParent)) - mergedParent = parentBundle; + var mergedEntryList = BundleAmendMerger.MergeEntries(parentBundle.Data.Entries, orderedAmendData); + var mergedBundleData = parentBundle.Data with { Entries = mergedEntryList }; - // Merge the amend entries into the parent - var combinedEntries = mergedParent.Entries.Concat(amendBundle.Entries).ToList(); + var bundleDirectory = fileSystem.Path.GetDirectoryName(parentPath) ?? string.Empty; + var changelogDirectory = fileSystem.Path.GetDirectoryName(bundleDirectory) ?? bundleDirectory; + var resolvedEntries = ResolveEntries(mergedBundleData, changelogDirectory, emitWarning); mergedParents[parentPath] = new LoadedBundle( - mergedParent.Version, - mergedParent.Repo, - mergedParent.Owner, - mergedParent.Data, - mergedParent.FilePath, - combinedEntries - ); - - _ = mergedAmendPaths.Add(amendBundle.FilePath); + parentBundle.Version, + parentBundle.Repo, + parentBundle.Owner, + mergedBundleData, + parentPath, + resolvedEntries); + + foreach (var amend in group) + _ = mergedAmendPaths.Add(amend.FilePath); } - // Build the final result: replace parent bundles with merged versions, exclude merged amend files - var result = bundles + return bundles .Where(bundle => !mergedAmendPaths.Contains(bundle.FilePath)) .Select(bundle => mergedParents.TryGetValue(bundle.FilePath, out var mergedBundle) ? mergedBundle : bundle) .ToList(); - - return result; } /// @@ -327,7 +327,7 @@ private List MergeAmendFiles(List bundles) /// The file path to check. /// True if the file is an amend file. private static bool IsAmendFile(string filePath) => - AmendFileRegex().IsMatch(filePath); + BundleAmendMerger.IsAmendFile(filePath); /// /// Gets the parent bundle path from an amend file path. @@ -336,16 +336,12 @@ private static bool IsAmendFile(string filePath) => /// The parent bundle path, or null if not an amend file. private string? GetParentBundlePath(string amendFilePath) { - var match = AmendFileRegex().Match(amendFilePath); - if (!match.Success) + if (!BundleAmendMerger.IsAmendFile(amendFilePath)) return null; - // Replace the ".amend-N" part with just the extension var directory = fileSystem.Path.GetDirectoryName(amendFilePath) ?? string.Empty; var fileName = fileSystem.Path.GetFileName(amendFilePath); var extension = fileSystem.Path.GetExtension(amendFilePath); - - // Remove the .amend-N part from the filename var parentFileName = AmendFileRegex().Replace(fileName, extension); return fileSystem.Path.Join(directory, parentFileName); diff --git a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs index 6cb7263878..7a17e33a12 100644 --- a/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs +++ b/src/Elastic.Documentation.Configuration/ReleaseNotes/ReleaseNotesSerialization.cs @@ -197,7 +197,8 @@ private static string ToYamlDoubleQuotedString(string s) Description = dto.Description, ReleaseDate = ParseReleaseDate(dto.ReleaseDate), HideFeatures = dto.HideFeatures ?? [], - Entries = dto.Entries?.Select(ToBundledEntry).ToList() ?? [] + Entries = dto.Entries?.Select(ToBundledEntry).ToList() ?? [], + ExcludeEntries = dto.ExcludeEntries?.Select(ToBundledEntry).ToList() ?? [] }; private static BundledProduct ToBundledProduct(BundledProductDto dto) => new() @@ -308,7 +309,8 @@ private static ChangelogEntryType ParseEntryType(string? value) Description = bundle.Description, ReleaseDate = bundle.ReleaseDate?.ToString("yyyy-MM-dd", CultureInfo.InvariantCulture), HideFeatures = bundle.HideFeatures.Count > 0 ? bundle.HideFeatures.ToList() : null, - Entries = bundle.Entries.Select(ToDto).ToList() + Entries = bundle.Entries.Count > 0 ? bundle.Entries.Select(ToDto).ToList() : null, + ExcludeEntries = bundle.ExcludeEntries.Count > 0 ? bundle.ExcludeEntries.Select(ToDto).ToList() : null }; private static BundledProductDto ToDto(BundledProduct product) => new() diff --git a/src/Elastic.Documentation/ReleaseNotes/Bundle.cs b/src/Elastic.Documentation/ReleaseNotes/Bundle.cs index 70c470eb7f..4a5f0d88bc 100644 --- a/src/Elastic.Documentation/ReleaseNotes/Bundle.cs +++ b/src/Elastic.Documentation/ReleaseNotes/Bundle.cs @@ -36,6 +36,12 @@ public record Bundle /// Changelog entries in this bundle. public IReadOnlyList Entries { get; init; } = []; + /// + /// Entries to exclude when this amend file is merged with its parent bundle. + /// Matched by file name and optional checksum. + /// + public IReadOnlyList ExcludeEntries { get; init; } = []; + /// Whether entries in this bundle have their contents resolved (inlined). public bool IsResolved => Entries.Any(e => e.Title != null); } diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs index cf171799a8..f7cfea8483 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogBundleAmendService.cs @@ -33,13 +33,28 @@ public record AmendBundleArguments /// /// Paths to changelog YAML files to add to the bundle /// - public required IReadOnlyList AddFiles { get; init; } + public IReadOnlyList AddFiles { get; init; } = []; + + /// + /// Paths to changelog YAML files to remove from the effective bundle + /// + public IReadOnlyList RemoveFiles { get; init; } = []; /// /// Whether to resolve (copy contents) the added entries. /// When null, inferred from the original bundle. /// public bool? Resolve { get; init; } + + /// + /// Remove by file name when the bundle checksum does not match the file on disk. + /// + public bool Force { get; init; } + + /// + /// Preview changes without writing an amend file. + /// + public bool DryRun { get; init; } } /// @@ -65,13 +80,12 @@ public partial class ChangelogBundleAmendService( private static partial Regex AmendFileRegex(); /// - /// Amends a bundle with additional changelog entries, creating a new immutable amend file. + /// Amends a bundle with additional or excluded changelog entries, creating a new immutable amend file. /// public async Task AmendBundle(IDiagnosticsCollector collector, AmendBundleArguments input, Cancel ctx) { try { - // Validate bundle file exists if (!_fileSystem.File.Exists(input.BundlePath)) { var currentDir = _fileSystem.Directory.GetCurrentDirectory(); @@ -82,86 +96,77 @@ public async Task AmendBundle(IDiagnosticsCollector collector, AmendBundle return false; } - // Validate add files - if (input.AddFiles.Count == 0) + if (input.AddFiles.Count == 0 && input.RemoveFiles.Count == 0) { - collector.EmitError(string.Empty, "At least one file must be specified with --add"); + collector.EmitError(string.Empty, "At least one file must be specified with --add or --remove"); return false; } - // Validate all add files exist - var addFilePaths = new List(); - foreach (var addFile in input.AddFiles) + var addFilePaths = ValidateInputFiles(collector, input.AddFiles, "--add"); + if (addFilePaths == null) + return false; + + var removeFilePaths = ValidateInputFiles(collector, input.RemoveFiles, "--remove"); + if (removeFilePaths == null) + return false; + + var (parentOk, parentBundle) = await TryDeserializeParentBundleAsync( + input.BundlePath, + collector, + emitParseErrorToCollector: true, + ctx); + if (!parentOk || parentBundle == null) + return false; + + var (amendsOk, existingAmendBundles) = await LoadExistingAmendBundlesAsync( + input.BundlePath, + collector, + ctx); + if (!amendsOk) + return false; + + var effectiveEntries = BundleAmendMerger.MergeEntries(parentBundle.Entries, existingAmendBundles); + var appliedExclusionKeys = BundleAmendMerger.CollectAppliedExclusionKeys(existingAmendBundles); + + var excludeEntries = new List(); + foreach (var removeFilePath in removeFilePaths!) { - if (!_fileSystem.File.Exists(addFile)) - { - var currentDir = _fileSystem.Directory.GetCurrentDirectory(); - collector.EmitError( - addFile, - $"File does not exist. Current directory: {currentDir}. " + - "Tip: Specify multiple files as comma-separated values (e.g., --add \"file1.yaml,file2.yaml\"). " + - "Paths support tilde (~) expansion and can be relative or absolute." - ); + var exclusion = await BuildExclusionEntryAsync( + collector, + removeFilePath, + effectiveEntries, + appliedExclusionKeys, + input.Force, + ctx); + if (exclusion == null) return false; - } - addFilePaths.Add(addFile); + if (exclusion is RemoveExclusionResult.Skip) + continue; + + var entry = ((RemoveExclusionResult.Add)exclusion).Entry; + excludeEntries.Add(entry); + _ = appliedExclusionKeys.Add(BundleAmendMerger.BuildExclusionKey(entry)); } - // Resolve flag: explicit CLI wins; otherwise infer from parent bundle (single read + deserialize). - Bundle? parentBundleFromInfer = null; bool shouldResolve; if (input.Resolve.HasValue) shouldResolve = input.Resolve.Value; else { - var (ok, inferredBundle) = await TryDeserializeParentBundleAsync( - input.BundlePath, - collector, - emitParseErrorToCollector: false, - ctx); - if (!ok) - shouldResolve = false; - else - { - parentBundleFromInfer = inferredBundle; - shouldResolve = inferredBundle!.IsResolved; - _logger.LogInformation("Inferred resolve={Resolve} from original bundle", shouldResolve); - } + shouldResolve = parentBundle.IsResolved; + _logger.LogInformation("Inferred resolve={Resolve} from original bundle", shouldResolve); } - - // Determine the next amend file number - var nextAmendNumber = GetNextAmendNumber(input.BundlePath); - var amendFilePath = GenerateAmendFilePath(input.BundlePath, nextAmendNumber); - - _logger.LogInformation("Creating amend file: {AmendFilePath} (resolve={Resolve})", amendFilePath, shouldResolve); - - ChangelogConfiguration? changelogConfig = null; - if (_configLoader != null) - changelogConfig = await _configLoader.LoadChangelogConfiguration(collector, null, ctx); - - var linkAllowRepos = changelogConfig?.Bundle?.LinkAllowRepos; - var linkAllowlistActive = linkAllowRepos != null; - Bundle? parentBundleForAllowlist = null; - - if (linkAllowlistActive) + var entries = new List(); + if (addFilePaths!.Count > 0) { - if (parentBundleFromInfer != null) - parentBundleForAllowlist = parentBundleFromInfer; - else - { - var (ok, loaded) = await TryDeserializeParentBundleAsync( - input.BundlePath, - collector, - emitParseErrorToCollector: true, - ctx); - if (!ok) - return false; - ArgumentNullException.ThrowIfNull(loaded); - parentBundleForAllowlist = loaded; - } + ChangelogConfiguration? changelogConfig = null; + if (_configLoader != null) + changelogConfig = await _configLoader.LoadChangelogConfiguration(collector, null, ctx); + + var linkAllowRepos = changelogConfig?.Bundle?.LinkAllowRepos; + var linkAllowlistActive = linkAllowRepos != null; - ArgumentNullException.ThrowIfNull(parentBundleForAllowlist); - if (!parentBundleForAllowlist.IsResolved) + if (linkAllowlistActive && !parentBundle.IsResolved) { collector.EmitError( string.Empty, @@ -170,101 +175,132 @@ public async Task AmendBundle(IDiagnosticsCollector collector, AmendBundle return false; } - var owner = parentBundleForAllowlist.Products.Count > 0 ? parentBundleForAllowlist.Products[0].Owner ?? "elastic" : "elastic"; - var repo = parentBundleForAllowlist.Products.Count > 0 ? parentBundleForAllowlist.Products[0].Repo : null; - if (!LinkAllowlistSanitizer.TryApplyBundle( - collector, - parentBundleForAllowlist, - linkAllowRepos!, - owner, - repo, - out _, - out var parentHadAllowlistChanges)) - return false; + if (linkAllowlistActive) + { + var owner = parentBundle.Products.Count > 0 ? parentBundle.Products[0].Owner ?? "elastic" : "elastic"; + var repo = parentBundle.Products.Count > 0 ? parentBundle.Products[0].Repo : null; + if (!LinkAllowlistSanitizer.TryApplyBundle( + collector, + parentBundle, + linkAllowRepos!, + owner, + repo, + out _, + out var parentHadAllowlistChanges)) + return false; - if (parentHadAllowlistChanges) + if (parentHadAllowlistChanges) + { + collector.EmitError( + string.Empty, + "bundle.link_allow_repos requires the parent bundle to already reflect filtered PR/issue references. " + + "Re-create the parent bundle with the same bundle.link_allow_repos and resolve enabled, " + + "or remove bundle.link_allow_repos for amend."); + return false; + } + } + + if (linkAllowlistActive && !shouldResolve) { collector.EmitError( string.Empty, - "bundle.link_allow_repos requires the parent bundle to already reflect filtered PR/issue references. " + - "Re-create the parent bundle with the same bundle.link_allow_repos and resolve enabled, " + - "or remove bundle.link_allow_repos for amend."); + "bundle.link_allow_repos requires resolved amend content. Use --resolve or ensure the original bundle is resolved, or remove bundle.link_allow_repos."); return false; } + + foreach (var filePath in addFilePaths) + { + var entry = await LoadChangelogFileAsync(collector, filePath, shouldResolve, ctx); + if (entry == null) + return false; + entries.Add(entry); + } } - if (linkAllowlistActive && !shouldResolve) + if (excludeEntries.Count == 0 && entries.Count == 0) { - collector.EmitError( - string.Empty, - "bundle.link_allow_repos requires resolved amend content. Use --resolve or ensure the original bundle is resolved, or remove bundle.link_allow_repos."); - return false; + collector.EmitWarning(string.Empty, "No changes to apply; amend file was not created."); + return true; } - // Load and process the files to add - var entries = new List(); - foreach (var filePath in addFilePaths) + if (input.DryRun) { - var entry = await LoadChangelogFileAsync(collector, filePath, shouldResolve, ctx); - if (entry == null) - return false; - entries.Add(entry); + _logger.LogInformation( + "Dry run: would exclude {ExcludeCount} and add {AddCount} entries", + excludeEntries.Count, + entries.Count); + return true; } - // Create the amend bundle + var nextAmendNumber = GetNextAmendNumber(input.BundlePath); + var amendFilePath = GenerateAmendFilePath(input.BundlePath, nextAmendNumber); + + _logger.LogInformation( + "Creating amend file: {AmendFilePath} (exclude={ExcludeCount}, add={AddCount}, resolve={Resolve})", + amendFilePath, + excludeEntries.Count, + entries.Count, + shouldResolve); + var amendBundle = new Bundle { - Products = [], // Amend files don't have products, they inherit from the original bundle + Products = [], + ExcludeEntries = excludeEntries, Entries = entries }; var bundleForWrite = amendBundle; - if (linkAllowlistActive && shouldResolve) + if (entries.Count > 0 && shouldResolve && _configLoader != null) { - ArgumentNullException.ThrowIfNull(parentBundleForAllowlist); - var owner = parentBundleForAllowlist.Products.Count > 0 ? parentBundleForAllowlist.Products[0].Owner ?? "elastic" : "elastic"; - var repo = parentBundleForAllowlist.Products.Count > 0 ? parentBundleForAllowlist.Products[0].Repo : null; + var changelogConfig = await _configLoader.LoadChangelogConfiguration(collector, null, ctx); + var linkAllowRepos = changelogConfig?.Bundle?.LinkAllowRepos; + if (linkAllowRepos != null) + { + var owner = parentBundle.Products.Count > 0 ? parentBundle.Products[0].Owner ?? "elastic" : "elastic"; + var repo = parentBundle.Products.Count > 0 ? parentBundle.Products[0].Repo : null; - if (!LinkAllowlistSanitizer.TryApplyBundle( - collector, - amendBundle, - linkAllowRepos!, - owner, - repo, - out var sanitized, - out _)) - return false; - bundleForWrite = sanitized; + if (!LinkAllowlistSanitizer.TryApplyBundle( + collector, + amendBundle, + linkAllowRepos, + owner, + repo, + out var sanitized, + out _)) + return false; + bundleForWrite = sanitized; - if (configurationContext != null && linkAllowRepos!.Count > 0) - { - try + if (configurationContext != null && linkAllowRepos.Count > 0) { - var assemblyYaml = configurationContext.ConfigurationFileProvider.AssemblerFile.ReadToEnd(); - var assembly = AssemblyConfiguration.Deserialize(assemblyYaml, skipPrivateRepositories: false); - LinkAllowlistSanitizer.EmitAssemblerDiagnostics(collector, linkAllowRepos!, assembly); - } - catch (Exception ex) when (ex is not (OutOfMemoryException or StackOverflowException)) - { - collector.EmitWarning( - string.Empty, - $"Could not load assembler.yml for bundle.link_allow_repos diagnostics: {ex.Message}"); + try + { + var assemblyYaml = configurationContext.ConfigurationFileProvider.AssemblerFile.ReadToEnd(); + var assembly = AssemblyConfiguration.Deserialize(assemblyYaml, skipPrivateRepositories: false); + LinkAllowlistSanitizer.EmitAssemblerDiagnostics(collector, linkAllowRepos, assembly); + } + catch (Exception ex) when (ex is not (OutOfMemoryException or StackOverflowException)) + { + collector.EmitWarning( + string.Empty, + $"Could not load assembler.yml for bundle.link_allow_repos diagnostics: {ex.Message}"); + } } } } - // Serialize and write the amend file var yaml = ReleaseNotesSerialization.SerializeBundle(bundleForWrite); - // Ensure output directory exists var outputDir = _fileSystem.Path.GetDirectoryName(amendFilePath); if (!string.IsNullOrWhiteSpace(outputDir) && !_fileSystem.Directory.Exists(outputDir)) _ = _fileSystem.Directory.CreateDirectory(outputDir); - // Strip any leading BOM to ensure clean UTF-8 output for tooling compatibility var normalizedYaml = ChangelogUtf8Normalization.StripLeadingUtf8BomChar(yaml); await _fileSystem.File.WriteAllTextAsync(amendFilePath, normalizedYaml, Utf8NoBom, ctx); - _logger.LogInformation("Created amend file: {AmendFilePath} with {Count} entries", amendFilePath, entries.Count); + _logger.LogInformation( + "Created amend file: {AmendFilePath} with {ExcludeCount} exclusions and {AddCount} additions", + amendFilePath, + excludeEntries.Count, + entries.Count); return true; } @@ -280,6 +316,152 @@ public async Task AmendBundle(IDiagnosticsCollector collector, AmendBundle } } + private List? ValidateInputFiles( + IDiagnosticsCollector collector, + IReadOnlyList files, + string optionName) + { + if (files.Count == 0) + return []; + + var validatedPaths = new List(); + foreach (var file in files) + { + if (!_fileSystem.File.Exists(file)) + { + var currentDir = _fileSystem.Directory.GetCurrentDirectory(); + collector.EmitError( + file, + $"File does not exist. Current directory: {currentDir}. " + + $"Tip: Repeat {optionName} for each file, or use comma-separated values (e.g., {optionName} \"file1.yaml,file2.yaml\"). " + + "Paths support tilde (~) expansion and can be relative or absolute." + ); + return null; + } + validatedPaths.Add(file); + } + + return validatedPaths; + } + + private async Task<(bool Ok, List Bundles)> LoadExistingAmendBundlesAsync( + string bundlePath, + IDiagnosticsCollector collector, + Cancel ctx) + { + var amendPaths = DiscoverAmendFiles(_fileSystem, bundlePath); + var amendBundles = new List(); + foreach (var amendPath in amendPaths) + { + try + { + var content = await _fileSystem.File.ReadAllTextAsync(amendPath, ctx); + amendBundles.Add(ReleaseNotesSerialization.DeserializeBundle(content)); + } + catch (Exception ex) when (ex is not (OutOfMemoryException or StackOverflowException or ThreadAbortException)) + { + collector.EmitError( + amendPath, + $"Failed to deserialize amend file: {ex.Message}", + ex); + return (false, []); + } + } + return (true, amendBundles); + } + + private async Task BuildExclusionEntryAsync( + IDiagnosticsCollector collector, + string removeFilePath, + IReadOnlyList effectiveEntries, + HashSet appliedExclusionKeys, + bool force, + Cancel ctx) + { + var fileName = _fileSystem.Path.GetFileName(removeFilePath); + var content = await _fileSystem.File.ReadAllTextAsync(removeFilePath, ctx); + var fileChecksum = ChangelogBundlingService.ComputeSha1(content); + + var strictExclusion = new BundledEntry + { + File = new BundledFile + { + Name = fileName, + Checksum = fileChecksum + } + }; + + var exclusionKey = BundleAmendMerger.BuildExclusionKey(strictExclusion); + if (appliedExclusionKeys.Contains(exclusionKey)) + { + collector.EmitWarning( + removeFilePath, + $"Changelog '{fileName}' is already excluded by a prior amend file; skipping."); + return RemoveExclusionResult.Skip.Instance; + } + + var strictMatches = effectiveEntries + .Where(entry => BundleAmendMerger.EntryMatchesExclusion(entry, strictExclusion)) + .ToList(); + + var matchedEntry = strictMatches.Count > 0 ? strictMatches[0] : null; + + if (matchedEntry == null) + { + var nameOnlyExclusion = new BundledEntry + { + File = new BundledFile + { + Name = fileName, + Checksum = string.Empty + } + }; + + var nameMatches = effectiveEntries + .Where(entry => BundleAmendMerger.EntryMatchesExclusion(entry, nameOnlyExclusion)) + .ToList(); + + if (nameMatches.Count == 0) + { + collector.EmitError( + removeFilePath, + $"Changelog '{fileName}' was not found in the effective bundle (parent plus existing amend files)."); + return null; + } + + if (!force) + { + collector.EmitError( + removeFilePath, + $"Bundle contains '{fileName}' but with a different checksum than the file on disk. " + + "Re-create the bundle or use --force to remove by file name only."); + return null; + } + + matchedEntry = nameMatches[0]; + } + + var exclusionChecksum = matchedEntry.File?.Checksum ?? fileChecksum; + return new RemoveExclusionResult.Add(new BundledEntry + { + File = new BundledFile + { + Name = fileName, + Checksum = exclusionChecksum + } + }); + } + + private abstract record RemoveExclusionResult + { + public sealed record Add(BundledEntry Entry) : RemoveExclusionResult; + public sealed record Skip : RemoveExclusionResult + { + public static readonly Skip Instance = new(); + private Skip() { } + } + } + private async Task<(bool Ok, Bundle? Bundle)> TryDeserializeParentBundleAsync( string bundlePath, IDiagnosticsCollector collector, @@ -313,7 +495,6 @@ private int GetNextAmendNumber(string bundlePath) var directory = _fileSystem.Path.GetDirectoryName(bundlePath) ?? string.Empty; var baseName = _fileSystem.Path.GetFileNameWithoutExtension(bundlePath); - // Find existing amend files var existingAmendFiles = _fileSystem.Directory.GetFiles(directory, $"{baseName}.amend-*.y*ml"); var maxNumber = existingAmendFiles @@ -346,12 +527,10 @@ private string GenerateAmendFilePath(string bundlePath, int amendNumber) var fileName = _fileSystem.Path.GetFileName(filePath); var content = await _fileSystem.File.ReadAllTextAsync(filePath, ctx); - // Compute checksum var checksum = ChangelogBundlingService.ComputeSha1(content); if (!resolve) { - // Just return file reference return new BundledEntry { File = new BundledFile @@ -362,7 +541,6 @@ private string GenerateAmendFilePath(string bundlePath, int amendNumber) }; } - // Parse the changelog file and include full entry data var normalizedYaml = ReleaseNotesSerialization.NormalizeYaml(content); var entry = ReleaseNotesSerialization.DeserializeEntry(normalizedYaml); @@ -406,7 +584,7 @@ public static IReadOnlyList DiscoverAmendFiles(IFileSystem fileSystem, s return []; var amendFiles = fileSystem.Directory.GetFiles(directory, $"{baseName}.amend-*.y*ml") - .OrderBy(f => f, StringComparer.OrdinalIgnoreCase) + .OrderBy(BundleAmendMerger.GetAmendFileNumber) .ToList(); return amendFiles; diff --git a/src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs b/src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs index 2f867e3f1a..8d311920d6 100644 --- a/src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs +++ b/src/services/Elastic.Changelog/Bundling/ChangelogRemoveService.cs @@ -9,6 +9,7 @@ using Elastic.Documentation.Configuration.Changelog; using Elastic.Documentation.Configuration.ReleaseNotes; using Elastic.Documentation.Diagnostics; +using Elastic.Documentation.ReleaseNotes; using Elastic.Documentation.Services; using Microsoft.Extensions.Logging; using Nullean.ScopedFileSystem; @@ -349,16 +350,35 @@ private async Task> FindBundleDependenciesAsync( var dependencies = new List(); - foreach (var bundleFile in bundleFiles) + foreach (var bundleFile in bundleFiles.Where(file => !BundleAmendMerger.IsAmendFile(file))) { try { var content = await _fileSystem.File.ReadAllTextAsync(bundleFile, ctx); var bundle = ReleaseNotesSerialization.DeserializeBundle(content); + var amendPaths = ChangelogBundleAmendService.DiscoverAmendFiles(_fileSystem, bundleFile); + var amendBundles = new List(); + foreach (var amendPath in amendPaths) + { + try + { + var amendContent = await _fileSystem.File.ReadAllTextAsync(amendPath, ctx); + amendBundles.Add(ReleaseNotesSerialization.DeserializeBundle(amendContent)); + } + catch (Exception ex) when (ex is not (OutOfMemoryException or StackOverflowException or ThreadAbortException)) + { + _logger.LogWarning(ex, + "Could not parse amend file {AmendFile} for dependency check; using parent bundle entries only", + amendPath); + } + } + + var effectiveEntries = BundleAmendMerger.MergeEntries(bundle.Entries, amendBundles); + // Only treat as unresolved when the entry would need to load from file. // Resolved entries have inline data (Title+Type) and don't need the file even if they have a File block. - var entryFileNames = bundle.Entries + var entryFileNames = effectiveEntries .Where(entry => !string.IsNullOrWhiteSpace(entry.File?.Name) && (string.IsNullOrWhiteSpace(entry.Title) || entry.Type == null)) diff --git a/src/services/Elastic.Changelog/Rendering/BundleValidationService.cs b/src/services/Elastic.Changelog/Rendering/BundleValidationService.cs index 8d83e6e35f..8b3cfe1009 100644 --- a/src/services/Elastic.Changelog/Rendering/BundleValidationService.cs +++ b/src/services/Elastic.Changelog/Rendering/BundleValidationService.cs @@ -95,7 +95,7 @@ public async Task ValidateBundlesAsync( IReadOnlyList amendFiles, Cancel ctx) { - var mergedEntries = new List(mainBundle.Entries); + var amendBundles = new List(); foreach (var amendFile in amendFiles) { @@ -103,9 +103,12 @@ public async Task ValidateBundlesAsync( { var amendContent = await fileSystem.File.ReadAllTextAsync(amendFile, ctx); var amendBundle = ReleaseNotesSerialization.DeserializeBundle(amendContent); - - _logger.LogInformation("Merging {Count} entries from amend file {AmendFile}", amendBundle.Entries.Count, amendFile); - mergedEntries.AddRange(amendBundle.Entries); + amendBundles.Add(amendBundle); + _logger.LogInformation( + "Merging amend file {AmendFile} ({AddCount} additions, {ExcludeCount} exclusions)", + amendFile, + amendBundle.Entries.Count, + amendBundle.ExcludeEntries.Count); } catch (YamlException yamlEx) { @@ -114,9 +117,14 @@ public async Task ValidateBundlesAsync( } } + var mergedEntries = BundleAmendMerger.MergeEntries(mainBundle.Entries, amendBundles); + return new Bundle { Products = mainBundle.Products, + Description = mainBundle.Description, + ReleaseDate = mainBundle.ReleaseDate, + HideFeatures = mainBundle.HideFeatures, Entries = mergedEntries }; } diff --git a/src/tooling/docs-builder/Commands/ChangelogCommand.cs b/src/tooling/docs-builder/Commands/ChangelogCommand.cs index 860bd26074..9b222f5f4b 100644 --- a/src/tooling/docs-builder/Commands/ChangelogCommand.cs +++ b/src/tooling/docs-builder/Commands/ChangelogCommand.cs @@ -1275,16 +1275,22 @@ async static (s, collector, state, ctx) => await s.CreateChangelogsFromRelease(c return await serviceInvoker.InvokeAsync(ctx); } - /// Append additional changelog entries to a published bundle without modifying it. + /// Append or exclude changelog entries in a published bundle without modifying it. /// Creates an immutable .amend-N.yaml sidecar file alongside the original bundle. /// Required: Path to the original bundle file to amend - /// Required: Path(s) to changelog YAML file(s) to add as comma-separated values (e.g., --add "file1.yaml,file2.yaml"). Supports tilde (~) expansion and relative paths. - /// Optional: Copy the contents of each changelog file into the entries array. Use --no-resolve to explicitly turn off resolve (overrides inference from original bundle). + /// Optional: Changelog YAML paths to add. Repeat --add or pass a comma-separated list in one value (for example, --add "file1.yaml,file2.yaml"). Supports tilde (~) expansion and relative paths. + /// Optional: Changelog YAML paths to exclude from the effective bundle. Repeat --remove or pass a comma-separated list in one value. Supports tilde (~) expansion and relative paths. + /// Optional: When using --add, inline each added changelog's content in the amend file. Use --no-resolve to record file references only. When omitted, inferred from the parent bundle. Does not apply to --remove. + /// Optional: When removing, match by file name even if the bundle checksum differs from the file on disk. + /// Optional: Preview changes without writing an amend file. [NoOptionsInjection] public async Task BundleAmend( [Argument, Existing, ExpandUserProfile, RejectSymbolicLinks, FileExtensions(Extensions = "yml,yaml")] FileInfo bundlePath, string[]? add = null, + string[]? remove = null, bool? resolve = null, + bool force = false, + bool dryRun = false, CancellationToken ct = default ) { @@ -1293,30 +1299,32 @@ public async Task BundleAmend( var service = new ChangelogBundleAmendService(logFactory, configurationContext: configurationContext); - if (add == null || add.Length == 0) + var normalizedAddFiles = add != null + ? ExpandCommaSeparated(add).Select(NormalizePath).ToList() + : []; + var normalizedRemoveFiles = remove != null + ? ExpandCommaSeparated(remove).Select(NormalizePath).ToList() + : []; + + if (normalizedAddFiles.Count == 0 && normalizedRemoveFiles.Count == 0) { - collector.EmitError(string.Empty, "At least one file must be specified with --add"); + collector.EmitError(string.Empty, "At least one file must be specified with --add or --remove"); _ = collector.StartAsync(ctx); await collector.WaitForDrain(); await collector.StopAsync(ctx); return 1; } - // Normalize the bundle path var normalizedBundlePath = bundlePath.FullName; - var normalizedAddFiles = ExpandCommaSeparated(add) - .Select(NormalizePath) - .ToList(); - - // Determine resolve: CLI --no-resolve takes precedence, then CLI --resolve, then infer from bundle - var shouldResolve = resolve; - var input = new AmendBundleArguments { BundlePath = normalizedBundlePath, - AddFiles = normalizedAddFiles.ToArray(), - Resolve = shouldResolve + AddFiles = normalizedAddFiles, + RemoveFiles = normalizedRemoveFiles, + Resolve = resolve, + Force = force, + DryRun = dryRun }; serviceInvoker.AddCommand(service, input, diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendMergerTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendMergerTests.cs new file mode 100644 index 0000000000..fadc603b3d --- /dev/null +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendMergerTests.cs @@ -0,0 +1,64 @@ +// Licensed to Elasticsearch B.V under one or more agreements. +// Elasticsearch B.V licenses this file to you under the Apache 2.0 License. +// See the LICENSE file in the project root for more information + +using AwesomeAssertions; +using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.ReleaseNotes; + +namespace Elastic.Changelog.Tests.Changelogs; + +public class BundleAmendMergerTests +{ + [Fact] + public void MergeEntries_AppliesExclusionsBeforeAdditionsWithinAmend() + { + var parent = new List + { + CreateFileEntry("keep.yaml", "aaa"), + CreateFileEntry("remove.yaml", "bbb") + }; + + var amend = new Bundle + { + ExcludeEntries = [CreateFileEntry("remove.yaml", "bbb")], + Entries = [CreateFileEntry("add.yaml", "ccc")] + }; + + var merged = BundleAmendMerger.MergeEntries(parent, [amend]); + + merged.Should().HaveCount(2); + merged.Should().Contain(e => e.File!.Name == "keep.yaml"); + merged.Should().Contain(e => e.File!.Name == "add.yaml"); + merged.Should().NotContain(e => e.File!.Name == "remove.yaml"); + } + + [Fact] + public void MergeEntries_AppliesAmendsInOrder() + { + var parent = new List { CreateFileEntry("one.yaml", "1") }; + + var amend1 = new Bundle + { + Entries = [CreateFileEntry("two.yaml", "2")] + }; + var amend2 = new Bundle + { + ExcludeEntries = [CreateFileEntry("one.yaml", "1")] + }; + + var merged = BundleAmendMerger.MergeEntries(parent, [amend1, amend2]); + + merged.Should().HaveCount(1); + merged[0].File!.Name.Should().Be("two.yaml"); + } + + private static BundledEntry CreateFileEntry(string name, string checksum) => new() + { + File = new BundledFile + { + Name = name, + Checksum = checksum + } + }; +} diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendTests.cs index 13f6cbb465..814951a14d 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleAmendTests.cs @@ -6,6 +6,7 @@ using Elastic.Changelog.Bundling; using Elastic.Documentation.Configuration; using Elastic.Documentation.Configuration.ReleaseNotes; +using Elastic.Documentation.Diagnostics; namespace Elastic.Changelog.Tests.Changelogs; @@ -266,4 +267,163 @@ public async Task AmendBundle_WithExplicitResolveFalse_OverridesResolvedBundle() amendContent.Should().NotContain("title: New amended feature"); amendContent.Should().NotContain("type: enhancement"); } + + [Fact] + public async Task AmendBundle_RemoveFromParent_CreatesExcludeEntries() + { + var ct = TestContext.Current.CancellationToken; + var bundlePath = await CreateUnresolvedBundle(ct); + + var changelogFile = FileSystem.Path.Join(_changelogDir, "1755268130-existing.yaml"); + var amendCollector = new TestDiagnosticsCollector(Output); + + var input = new AmendBundleArguments + { + BundlePath = bundlePath, + RemoveFiles = [changelogFile] + }; + + var result = await Service.AmendBundle(amendCollector, input, ct); + + result.Should().BeTrue(); + amendCollector.Errors.Should().Be(0); + + var amendFiles = ChangelogBundleAmendService.DiscoverAmendFiles(FileSystem, bundlePath); + amendFiles.Should().HaveCount(1); + + var amendContent = await FileSystem.File.ReadAllTextAsync(amendFiles[0], ct); + amendContent.Should().Contain("exclude-entries:"); + amendContent.Should().Contain("name: 1755268130-existing.yaml"); + amendContent.TrimStart().Should().StartWith("exclude-entries:"); + } + + [Fact] + public async Task AmendBundle_RemoveAfterAdd_ExcludesAmendedEntry() + { + var ct = TestContext.Current.CancellationToken; + var bundlePath = await CreateUnresolvedBundle(ct); + var newFile = await CreateNewChangelogFile(ct); + + var addCollector = new TestDiagnosticsCollector(Output); + var addInput = new AmendBundleArguments + { + BundlePath = bundlePath, + AddFiles = [newFile] + }; + (await Service.AmendBundle(addCollector, addInput, ct)).Should().BeTrue(); + + var removeCollector = new TestDiagnosticsCollector(Output); + var removeInput = new AmendBundleArguments + { + BundlePath = bundlePath, + RemoveFiles = [newFile] + }; + + var result = await Service.AmendBundle(removeCollector, removeInput, ct); + + result.Should().BeTrue(); + removeCollector.Errors.Should().Be(0); + + var amendFiles = ChangelogBundleAmendService.DiscoverAmendFiles(FileSystem, bundlePath); + amendFiles.Should().HaveCount(2); + + var removeAmendContent = await FileSystem.File.ReadAllTextAsync(amendFiles[1], ct); + removeAmendContent.Should().Contain("exclude-entries:"); + removeAmendContent.Should().Contain("name: 1755268200-new-feature.yaml"); + } + + [Fact] + public async Task AmendBundle_RemoveWithChecksumMismatch_WithoutForce_Fails() + { + var ct = TestContext.Current.CancellationToken; + var bundlePath = await CreateUnresolvedBundle(ct); + var changelogFile = FileSystem.Path.Join(_changelogDir, "1755268130-existing.yaml"); + + await FileSystem.File.WriteAllTextAsync( + changelogFile, + """ + title: Changed title + type: feature + products: + - product: elasticsearch + target: 9.2.0 + prs: + - "100" + """, + ct); + + var amendCollector = new TestDiagnosticsCollector(Output); + var input = new AmendBundleArguments + { + BundlePath = bundlePath, + RemoveFiles = [changelogFile] + }; + + var result = await Service.AmendBundle(amendCollector, input, ct); + + result.Should().BeFalse(); + amendCollector.Diagnostics.Should().ContainSingle(d => + d.Message.Contains("different checksum")); + } + + [Fact] + public async Task AmendBundle_RemoveAndAdd_InSingleAmendFile() + { + var ct = TestContext.Current.CancellationToken; + var bundlePath = await CreateUnresolvedBundle(ct); + var removeFile = FileSystem.Path.Join(_changelogDir, "1755268130-existing.yaml"); + var addFile = await CreateNewChangelogFile(ct); + + var amendCollector = new TestDiagnosticsCollector(Output); + var input = new AmendBundleArguments + { + BundlePath = bundlePath, + RemoveFiles = [removeFile], + AddFiles = [addFile] + }; + + var result = await Service.AmendBundle(amendCollector, input, ct); + + result.Should().BeTrue(); + amendCollector.Errors.Should().Be(0); + + var amendFiles = ChangelogBundleAmendService.DiscoverAmendFiles(FileSystem, bundlePath); + amendFiles.Should().HaveCount(1); + + var amendContent = await FileSystem.File.ReadAllTextAsync(amendFiles[0], ct); + amendContent.Should().Contain("exclude-entries:"); + amendContent.Should().Contain("name: 1755268130-existing.yaml"); + amendContent.Should().Contain("entries:"); + amendContent.Should().Contain("name: 1755268200-new-feature.yaml"); + } + + [Fact] + public async Task AmendBundle_CorruptExistingAmend_FailsWithoutWritingNewAmend() + { + var ct = TestContext.Current.CancellationToken; + var bundlePath = await CreateUnresolvedBundle(ct); + var changelogFile = FileSystem.Path.Join(_changelogDir, "1755268130-existing.yaml"); + + await FileSystem.File.WriteAllTextAsync( + FileSystem.Path.ChangeExtension(bundlePath, ".amend-1.yaml"), + "exclude-entries:\n - file: [invalid yaml", + ct); + + var amendCollector = new TestDiagnosticsCollector(Output); + var input = new AmendBundleArguments + { + BundlePath = bundlePath, + RemoveFiles = [changelogFile] + }; + + var result = await Service.AmendBundle(amendCollector, input, ct); + + result.Should().BeFalse(); + amendCollector.Diagnostics.Should().ContainSingle(d => + d.Severity == Severity.Error && + d.Message.Contains("Failed to deserialize amend file")); + + var amendFiles = ChangelogBundleAmendService.DiscoverAmendFiles(FileSystem, bundlePath); + amendFiles.Should().HaveCount(1, "corrupt amend should not produce a second amend file"); + } } diff --git a/tests/Elastic.Changelog.Tests/Changelogs/BundleLoading/BundleLoaderTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/BundleLoading/BundleLoaderTests.cs index d272a31520..b7410c0bd1 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/BundleLoading/BundleLoaderTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/BundleLoading/BundleLoaderTests.cs @@ -592,6 +592,49 @@ public void LoadBundles_WithMultipleAmendFiles_MergesAllIntoParent() bundles[0].Entries.Select(e => e.Title).Should().Contain(["Original feature", "First amendment", "Second amendment"]); } + [Fact] + public void LoadBundles_WithExcludeAmendFile_RemovesEntryFromParent() + { + var bundlesFolder = "/docs/changelog/bundles"; + _fileSystem.Directory.CreateDirectory(bundlesFolder); + + // language=yaml + var parentBundle = + """ + products: + - product: elasticsearch + target: 9.3.0 + entries: + - title: Original feature + type: feature + file: + name: original.yaml + checksum: abc + - title: Removed feature + type: bug-fix + file: + name: removed.yaml + checksum: def + """; + // language=yaml + var excludeAmend = + """ + exclude-entries: + - file: + name: removed.yaml + checksum: def + """; + _fileSystem.File.WriteAllText($"{bundlesFolder}/9.3.0.yaml", parentBundle); + _fileSystem.File.WriteAllText($"{bundlesFolder}/9.3.0.amend-1.yaml", excludeAmend); + + var service = CreateService(); + var bundles = service.LoadBundles(bundlesFolder, EmitWarning); + + bundles.Should().HaveCount(1); + bundles[0].Entries.Should().HaveCount(1); + bundles[0].Entries[0].Title.Should().Be("Original feature"); + } + [Fact] public void LoadBundles_AmendFileWithoutParent_RemainsStandalone() { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cs index 3f9e016501..cf64966594 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/ChangelogRemoveTests.cs @@ -290,6 +290,93 @@ await FileSystem.File.WriteAllTextAsync( FileExists("1001-es-feature.yaml").Should().BeTrue("File must not be deleted when blocked"); } + [Fact] + public async Task Remove_WhenExcludedByAmendBundle_Proceeds() + { + await WriteFile("1001-es-feature.yaml", ElasticsearchFeatureYaml); + + var bundlesDir = FileSystem.Path.Join(_changelogDir, "bundles"); + FileSystem.Directory.CreateDirectory(bundlesDir); + var checksum = ComputeSha1(ElasticsearchFeatureYaml); + var bundlePath = FileSystem.Path.Join(bundlesDir, "9.3.0.yaml"); + await FileSystem.File.WriteAllTextAsync( + bundlePath, + // language=yaml + $""" + products: + - product: elasticsearch + target: 9.3.0 + entries: + - file: + name: 1001-es-feature.yaml + checksum: {checksum} + """, + TestContext.Current.CancellationToken + ); + + await FileSystem.File.WriteAllTextAsync( + FileSystem.Path.Join(bundlesDir, "9.3.0.amend-1.yaml"), + // language=yaml + $""" + exclude-entries: + - file: + name: 1001-es-feature.yaml + checksum: {checksum} + """, + TestContext.Current.CancellationToken + ); + + var input = new ChangelogRemoveArguments { Directory = _changelogDir, All = true }; + + var result = await Service.RemoveChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeTrue("Amend exclusion should remove bundle dependency"); + Collector.Errors.Should().Be(0); + FileExists("1001-es-feature.yaml").Should().BeFalse(); + } + + [Fact] + public async Task Remove_WhenCorruptAmendBundle_StillBlocksFromParentReference() + { + await WriteFile("1001-es-feature.yaml", ElasticsearchFeatureYaml); + + var bundlesDir = FileSystem.Path.Join(_changelogDir, "bundles"); + FileSystem.Directory.CreateDirectory(bundlesDir); + var checksum = ComputeSha1(ElasticsearchFeatureYaml); + var bundlePath = FileSystem.Path.Join(bundlesDir, "9.3.0.yaml"); + await FileSystem.File.WriteAllTextAsync( + bundlePath, + // language=yaml + $""" + products: + - product: elasticsearch + target: 9.3.0 + entries: + - file: + name: 1001-es-feature.yaml + checksum: {checksum} + """, + TestContext.Current.CancellationToken + ); + + await FileSystem.File.WriteAllTextAsync( + FileSystem.Path.Join(bundlesDir, "9.3.0.amend-1.yaml"), + "exclude-entries:\n - file: [invalid yaml", + TestContext.Current.CancellationToken + ); + + var input = new ChangelogRemoveArguments { Directory = _changelogDir, All = true }; + + var result = await Service.RemoveChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeFalse("Parent bundle reference should block deletion when amend file is corrupt"); + Collector.Diagnostics.Should().ContainSingle(d => + d.Severity == Severity.Error && + d.Message.Contains("1001-es-feature.yaml") && + d.Message.Contains("unresolved bundle")); + FileExists("1001-es-feature.yaml").Should().BeTrue("File must not be deleted when parent still references it"); + } + [Fact] public async Task Remove_WhenReferencedByUnresolvedBundle_WithForce_Proceeds() { diff --git a/tests/Elastic.Changelog.Tests/Changelogs/Render/BundleValidationTests.cs b/tests/Elastic.Changelog.Tests/Changelogs/Render/BundleValidationTests.cs index c3d0c6d41f..8ec2082185 100644 --- a/tests/Elastic.Changelog.Tests/Changelogs/Render/BundleValidationTests.cs +++ b/tests/Elastic.Changelog.Tests/Changelogs/Render/BundleValidationTests.cs @@ -251,6 +251,48 @@ public async Task AmendFileEntry_CommentedFile_NormalizedChecksum_NoWarning() Collector.Warnings.Should().Be(0, "normalized checksum matches regardless of comments"); } + [Fact] + public async Task ExcludeAmendFile_OmitsEntryFromRenderedOutput() + { + var (bundleDir, changelogDir) = CreateTestDirs(); + + var file1 = "1000000001-feature.yaml"; + var file2 = "1000000002-enhancement.yaml"; + await WriteFileAsync(changelogDir, file1, ChangelogFeature1); + await WriteFileAsync(changelogDir, file2, ChangelogFeature2); + + var bundleFile = FileSystem.Path.Join(bundleDir, "bundle.yaml"); + await WriteBundleWithEntriesAsync( + bundleFile, + (file1, ComputeSha1(ChangelogFeature1)), + (file2, ComputeSha1(ChangelogFeature2))); + + var amend1 = FileSystem.Path.Join(bundleDir, "bundle.amend-1.yaml"); + await FileSystem.File.WriteAllTextAsync( + amend1, + // language=yaml + $""" + exclude-entries: + - file: + name: {file2} + checksum: {ComputeSha1(ChangelogFeature2)} + """, + TestContext.Current.CancellationToken); + + var input = CreateRenderInput(bundleFile, changelogDir); + + var result = await Service.RenderChangelogs(Collector, input, TestContext.Current.CancellationToken); + + result.Should().BeTrue(); + Collector.Errors.Should().Be(0); + + var outputDir = input.Output ?? throw new InvalidOperationException("Output must be set"); + var indexFile = FileSystem.Path.Join(outputDir, "9.2.0", "index.md"); + var content = await FileSystem.File.ReadAllTextAsync(indexFile, TestContext.Current.CancellationToken); + content.Should().Contain("Feature one"); + content.Should().NotContain("Feature two"); + } + private (string BundleDir, string ChangelogDir) CreateTestDirs() { var bundleDir = FileSystem.Path.Join(Paths.WorkingDirectoryRoot.FullName, Guid.NewGuid().ToString()); @@ -280,6 +322,31 @@ private async Task WriteBundleAsync(string bundlePath, string entryFileName, str await FileSystem.File.WriteAllTextAsync(bundlePath, content, TestContext.Current.CancellationToken); } + private async Task WriteBundleWithEntriesAsync( + string bundlePath, + params (string FileName, string Checksum)[] entries) + { + var entryLines = entries + .Select(entry => + $""" + - file: + name: {entry.FileName} + checksum: {entry.Checksum} + """) + .ToList(); + + // language=yaml + var content = + $""" + products: + - product: elasticsearch + target: 9.2.0 + entries: + {string.Join('\n', entryLines)} + """; + await FileSystem.File.WriteAllTextAsync(bundlePath, content, TestContext.Current.CancellationToken); + } + private RenderChangelogsArguments CreateRenderInput(string bundleFile, string changelogDir) => new() { diff --git a/tests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cs b/tests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cs index 4307d815f6..779e0cae76 100644 --- a/tests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cs +++ b/tests/Elastic.Markdown.Tests/Directives/ChangelogBasicTests.cs @@ -72,6 +72,70 @@ public void RendersMarkdownContent() } } +public class ChangelogExcludeAmendTests : DirectiveTest +{ + public ChangelogExcludeAmendTests(ITestOutputHelper output) : base(output, + // language=markdown + """ + :::{changelog} + ::: + """) + { + FileSystem.AddFile("docs/changelog/bundles/9.3.0.yaml", new MockFileData( + // language=yaml + """ + products: + - product: elasticsearch + target: 9.3.0 + lifecycle: ga + entries: + - title: Keep this feature + type: feature + products: + - product: elasticsearch + target: 9.3.0 + file: + name: keep.yaml + checksum: keep-checksum + prs: + - "123456" + - title: Remove this feature + type: feature + products: + - product: elasticsearch + target: 9.3.0 + file: + name: removed.yaml + checksum: excluded + prs: + - "123457" + """)); + FileSystem.AddFile("docs/changelog/bundles/9.3.0.amend-1.yaml", new MockFileData( + // language=yaml + """ + exclude-entries: + - file: + name: removed.yaml + checksum: excluded + """)); + } + + [Fact] + public void RendersWithoutExcludedEntry() + { + Html.Should().Contain("Keep this feature"); + Html.Should().NotContain("Remove this feature"); + } + + [Fact] + public void LoadsMergedEntryCount() + { + Block!.LoadedBundles.Should().HaveCount(1); + Block.LoadedBundles[0].Entries.Should().HaveCount(1); + Block.LoadedBundles[0].Entries[0].Title.Should().Be("Keep this feature"); + } +} + public class ChangelogMultipleBundlesTests : DirectiveTest { public ChangelogMultipleBundlesTests(ITestOutputHelper output) : base(output,