From a4c6f65496199d036fb2fbf1c5ec13aa7d26e1cf Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Thu, 20 Aug 2026 15:17:06 -0400 Subject: [PATCH 01/10] tools(releaser): Add version computation and native SDK pinning to releaser. Computes package version bumps from conventional commits (including BREAKING/refs footers) and resolves iOS/Android/C++ native SDK pins er trigger context (mainline/patch/pre-release), wiring both into release_plan.dart's ReleasePlan output. Consolidates duplicated regex/parsing logic that previously lived separately in cocoapod_util.dart, gradle_util.dart, and generate_changelog.dart into shared native_sdk.dart and conventional_commits.dart utilities, and splits pure git-history queries (findLastReleaseTag, commitMessagesSince) into their own git_history.dart module. refs: RUM-17812 RUM-18022 # Conflicts: # tools/releaser/test/package_discovery_test.dart # tools/releaser/test/release_plan_test.dart # tools/releaser/test/support/fixture_repo.dart --- .gitlab-ci.yml | 1 + tools/releaser/bin/releaser.dart | 22 +- tools/releaser/lib/cmake_util.dart | 50 ++ tools/releaser/lib/cocoapod_util.dart | 21 +- tools/releaser/lib/conventional_commits.dart | 105 ++++ tools/releaser/lib/generate_changelog.dart | 177 +++---- tools/releaser/lib/git_history.dart | 61 +++ tools/releaser/lib/github_cmd_wrapper.dart | 43 +- tools/releaser/lib/gradle_util.dart | 12 +- tools/releaser/lib/native_sdk.dart | 205 ++++++++ tools/releaser/lib/release_plan.dart | 375 +++++++++++++- tools/releaser/lib/trigger_context.dart | 14 + tools/releaser/lib/version_updater.dart | 115 ++++- tools/releaser/pubspec.lock | 2 +- tools/releaser/pubspec.yaml | 1 + tools/releaser/test/cmake_util_test.dart | 107 ++++ .../test/conventional_commits_test.dart | 103 ++++ tools/releaser/test/git_history_test.dart | 107 ++++ tools/releaser/test/native_sdk_test.dart | 241 +++++++++ tools/releaser/test/release_plan_test.dart | 470 ++++++++++++++---- tools/releaser/test/support/fixture_repo.dart | 53 +- 21 files changed, 1994 insertions(+), 291 deletions(-) create mode 100644 tools/releaser/lib/cmake_util.dart create mode 100644 tools/releaser/lib/conventional_commits.dart create mode 100644 tools/releaser/lib/git_history.dart create mode 100644 tools/releaser/lib/native_sdk.dart create mode 100644 tools/releaser/lib/trigger_context.dart create mode 100644 tools/releaser/test/cmake_util_test.dart create mode 100644 tools/releaser/test/conventional_commits_test.dart create mode 100644 tools/releaser/test/git_history_test.dart create mode 100644 tools/releaser/test/native_sdk_test.dart diff --git a/.gitlab-ci.yml b/.gitlab-ci.yml index 2e317fac1..46f608e3e 100644 --- a/.gitlab-ci.yml +++ b/.gitlab-ci.yml @@ -121,6 +121,7 @@ build-flutter: - specific:true script: - !reference [.pre, script] + - pushd tools/releaser && dart pub get && dart test && popd - melos dartfmt_check - melos run analyze:dart - melos run unit_test:flutter diff --git a/tools/releaser/bin/releaser.dart b/tools/releaser/bin/releaser.dart index 53a28d183..2cb595f52 100644 --- a/tools/releaser/bin/releaser.dart +++ b/tools/releaser/bin/releaser.dart @@ -23,8 +23,10 @@ void main(List arguments) async { }); final argParser = ArgParser() - ..addOption('packages', - help: 'A comma separated list of package:version pairs to release.') + ..addOption( + 'packages', + help: 'A comma separated list of package:version pairs to release.', + ) ..addOption('version', abbr: 'v') ..addOption('repo-root', help: 'The root of the repo to release from') ..addFlag( @@ -108,7 +110,7 @@ void main(List arguments) async { var versionBumpType = VersionBumpType.minor; // If we're on a release branch, bump by a revision if (currentBranch.branchName.contains('release')) { - versionBumpType = VersionBumpType.rev; + versionBumpType = VersionBumpType.patch; } // If we're releasing a pre-release, bump by pre-release // if (commandArgs.version.contains('-')) { @@ -116,8 +118,9 @@ void main(List arguments) async { // } // If there are any initial releases, having no changes on the chore branch is okay (though unlikely) - final isInitialRelease = - commandArgs.packages.where((e) => e.version == '1.0.0').isNotEmpty; + final isInitialRelease = commandArgs.packages + .where((e) => e.version == '1.0.0') + .isNotEmpty; final commitPackageName = commandArgs.packages.length == 1 ? '${commandArgs.packages.first.name} ${commandArgs.packages.first.version}' @@ -128,7 +131,7 @@ void main(List arguments) async { commitBody = 'Releasing the following packages:\n'; commitBody += [ for (final package in commandArgs.packages) - ' - ${package.name} ${package.version}' + ' - ${package.name} ${package.version}', ].join('\n'); } @@ -173,7 +176,9 @@ void main(List arguments) async { } Future _validateArguments( - ArgResults argResults, Logger logger) async { + ArgResults argResults, + Logger logger, +) async { var packages = _parsePackages(argResults['packages'], logger); if (packages == null) { if (argResults.rest.isEmpty) { @@ -228,7 +233,8 @@ List? _parsePackages(String? packages, Logger logger) { final colonIndex = e.indexOf(':'); if (colonIndex < 0) { logger.shout( - '❌ Invalid package specification $e. Missing : to specify version.'); + '❌ Invalid package specification $e. Missing : to specify version.', + ); throw Error(); } diff --git a/tools/releaser/lib/cmake_util.dart b/tools/releaser/lib/cmake_util.dart new file mode 100644 index 000000000..a8fad384c --- /dev/null +++ b/tools/releaser/lib/cmake_util.dart @@ -0,0 +1,50 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'dart:io'; + +import 'package:logging/logging.dart'; + +import 'helpers.dart'; + +final _gitTagLinePattern = RegExp( + r'^(?\s*GIT_TAG\s+)(?[\w./-]+)(?[^#]*)(?:#.*)?$', +); + +/// Rewrites a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line to pin at +/// [targetSha] -- the resolved commit SHA for [targetTag] (e.g. `v1.4.0`), +/// kept as a trailing `# ` comment since CMake's `FetchContent_Declare` +/// has no field of its own for pairing a tag with a verified commit. A +/// full commit SHA is used as the actual pin (not the tag) because it's +/// immutable, unlike a tag, which can be moved to point elsewhere later. +/// +/// Preserves the line's existing whitespace and anything between the ref +/// and end of line -- a trailing `)` closing the `FetchContent_Declare(...` +/// call is common, and the new comment is appended *after* it, since a `#` +/// placed before would comment out the paren too and break the call. +/// +/// Only ever called against a release-prep/patch/pre-release branch's copy +/// of the file -- `develop`'s own floating `GIT_TAG develop` is never +/// touched by release tooling. +Future pinCppVersion( + File cmakeListsFile, + String targetTag, + String targetSha, + Logger logger, + bool dryRun, +) async { + logger.info( + 'ℹ️ Pinning dd-sdk-cpp GIT_TAG to $targetSha ($targetTag) in ' + '${cmakeListsFile.path}', + ); + + await transformFile(cmakeListsFile, logger, dryRun, (line) { + final match = _gitTagLinePattern.firstMatch(line); + if (match == null) return line; + + final prefix = match.namedGroup('prefix')!; + final trailing = (match.namedGroup('trailing') ?? '').trimRight(); + return '$prefix$targetSha$trailing # $targetTag'; + }); +} diff --git a/tools/releaser/lib/cocoapod_util.dart b/tools/releaser/lib/cocoapod_util.dart index 573bb5731..5c15cff68 100644 --- a/tools/releaser/lib/cocoapod_util.dart +++ b/tools/releaser/lib/cocoapod_util.dart @@ -6,13 +6,11 @@ import 'package:path/path.dart' as path; import 'command.dart'; import 'helpers.dart'; +import 'native_sdk.dart'; import 'package_list.dart'; final overridesStartPattern = RegExp(r'\s+# Datadog Pod Overrides'); final overridesEndPattern = RegExp(r'\s+# End Datadog Pod Overrides'); -final specDependencyPattern = RegExp( - r"\s+s\.dependency\s+'(?Datadog.+)', '.+", -); class PinCocoapodsVersionCommand extends Command { @override @@ -22,8 +20,9 @@ class PinCocoapodsVersionCommand extends Command { } // Other packages can keep looser version constraints - final pinedPackage = args.packages - .firstWhereOrNull((e) => e.name == 'datadog_flutter_plugin'); + final pinedPackage = args.packages.firstWhereOrNull( + (e) => e.name == 'datadog_flutter_plugin', + ); if (pinedPackage != null) { if (!await _pinPodspecVersion(args, pinedPackage, logger)) { return false; @@ -64,14 +63,14 @@ class PinCocoapodsVersionCommand extends Command { } Future _pinPodspecVersion( - CommandArguments args, PackageRelease package, Logger logger) async { + CommandArguments args, + PackageRelease package, + Logger logger, + ) async { final podspecLocation = 'ios/${package.name}.podspec'; final file = File( - path.join( - getPackageRoot(args, package), - podspecLocation, - ), + path.join(getPackageRoot(args, package), podspecLocation), ); if (!file.existsSync()) { @@ -83,7 +82,7 @@ class PinCocoapodsVersionCommand extends Command { logger.info('ℹ️ Setting the iOS Pod Dependency to ${args.iOSRelease}'); await transformFile(file, logger, args.dryRun, (element) { - final match = specDependencyPattern.firstMatch(element); + final match = iosPodspecDependencyPattern.firstMatch(element); if (match != null) { element = " s.dependency '${match.namedGroup('dependency')}', '${args.iOSRelease}'"; diff --git a/tools/releaser/lib/conventional_commits.dart b/tools/releaser/lib/conventional_commits.dart new file mode 100644 index 000000000..506d8b0a5 --- /dev/null +++ b/tools/releaser/lib/conventional_commits.dart @@ -0,0 +1,105 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'version_updater.dart'; + +/// A parsed conventional commit: header (`type(scope)!: description`) plus +/// the footers other tooling in this package cares about (a `BREAKING +/// CHANGE:` footer, and `refs:` lines referencing GitHub issues). +class ConventionalCommit { + final String type; + final String? scope; + + /// Whether the header itself carries a `!` breaking marker. + final bool hasBreakingMarker; + final String description; + + /// Whether a `BREAKING CHANGE:`/`BREAKING-CHANGE:` footer is present, + /// independent of [hasBreakingMarker] -- either one marks the commit + /// breaking, see [isBreaking]. + final bool hasBreakingFooter; + + /// Raw reference tokens from `refs:` footer lines (e.g. `refs: #123, + /// RUM-456` -> `['#123', 'RUM-456']`) -- a GitHub issue number, a JIRA + /// ticket, or anything else a `refs:` footer might carry. Not filtered + /// or interpreted here; a consumer that only wants GitHub issue links + /// (like `generate_changelog.dart`) picks those out itself. + final List refs; + + ConventionalCommit({ + required this.type, + required this.scope, + required this.hasBreakingMarker, + required this.description, + required this.hasBreakingFooter, + required this.refs, + }); + + bool get isBreaking => hasBreakingMarker || hasBreakingFooter; + + /// The semver bump this commit implies on its own: major if breaking + /// (marker or footer), minor for `feat`, patch for `fix`/`perf`, or null + /// for a type that doesn't carry semver weight by itself (`chore:`, + /// `docs:`, `test:`, etc.) and isn't marked breaking. + VersionBumpType? get bumpType { + if (isBreaking) return VersionBumpType.major; + + switch (type) { + case 'feat': + return VersionBumpType.minor; + case 'fix': + case 'perf': + return VersionBumpType.patch; + default: + return null; + } + } + + static final _headerPattern = RegExp( + r'^(?\w+)(\((?[^)]*)\))?(?!)?:\s*(?.*)', + ); + static final _breakingFooterPattern = RegExp( + r'^BREAKING[ -]CHANGE:', + multiLine: true, + ); + + /// Parses a full commit message (subject line plus body/footers) into + /// its conventional-commit parts, or returns null if the subject line + /// doesn't match the convention at all. + static ConventionalCommit? parse(String commitMessage) { + final lines = commitMessage.split('\n'); + final match = _headerPattern.firstMatch(lines.first); + if (match == null) return null; + + final refs = [ + for (final refLine in lines.where((l) => l.startsWith('refs:'))) + for (final token + in refLine.substring('refs:'.length).split(RegExp(r'[,\s]+'))) + if (token.isNotEmpty) token, + ]; + + return ConventionalCommit( + type: match.namedGroup('type')!, + scope: match.namedGroup('scope'), + hasBreakingMarker: match.namedGroup('breaking') == '!', + description: match.namedGroup('rest')!, + hasBreakingFooter: _breakingFooterPattern.hasMatch(commitMessage), + refs: refs, + ); + } +} + +/// The highest-severity bump implied by [commits] (major > minor > patch), +/// or null if none of them carry semver weight. +VersionBumpType? aggregateBumpLevel(Iterable commits) { + VersionBumpType? highest; + for (final commit in commits) { + final bump = commit.bumpType; + if (bump == null) continue; + if (highest == null || bump.severity > highest.severity) { + highest = bump; + } + } + return highest; +} diff --git a/tools/releaser/lib/generate_changelog.dart b/tools/releaser/lib/generate_changelog.dart index bc010d426..6c5096913 100644 --- a/tools/releaser/lib/generate_changelog.dart +++ b/tools/releaser/lib/generate_changelog.dart @@ -4,14 +4,16 @@ import 'dart:io'; -import 'package:git/git.dart'; import 'package:logging/logging.dart'; import 'package:path/path.dart' as path; -import 'package:version/version.dart'; import 'command.dart'; +import 'conventional_commits.dart'; +import 'git_history.dart'; import 'helpers.dart'; +final _githubIssueRefPattern = RegExp(r'^#(?\d+)$'); + // Maps common scope abbreviations that are added to conventional commits to more human // readable versions. final scopeAbbreviationMap = { @@ -31,24 +33,34 @@ class GenerateChangelogCommand extends Command { @override Future run(CommandArguments args, Logger logger) async { for (final package in args.packages) { - final lastReleaseSha = await _findLastReleaseSha(logger, args, package); - if (lastReleaseSha == null) { + final lastReleaseTag = await findLastReleaseTag( + args.gitDir, + package.name, + ); + if (lastReleaseTag == null) { Logger.root.shout( - '⚠️ Could not find last release! Hopefully this is a new package!.'); + '⚠️ Could not find last release! Hopefully this is a new package!.', + ); Logger.root.shout( - '‼️ Changelogs cannot be generated for an initial release! Make sure you have what you need in there.'); + '‼️ Changelogs cannot be generated for an initial release! Make sure you have what you need in there.', + ); } else { - final commits = - await _getCommits(args, package, '$lastReleaseSha..HEAD'); + final commits = await commitMessagesSince( + args.gitDir, + pathspec: getPackageRoot(args, package), + sinceSha: lastReleaseTag.objectSha, + ); final changelogItems = _getChangelogItems(commits); logger.fine( - 'Found ${changelogItems.length} changelog items for ${package.name} version ${package.version}'); + 'Found ${changelogItems.length} changelog items for ${package.name} version ${package.version}', + ); final versionChangelog = changelogItems.map((e) => '* $e').join('\n'); - final file = - File(path.join(getPackageRoot(args, package), 'CHANGELOG.md')); + final file = File( + path.join(getPackageRoot(args, package), 'CHANGELOG.md'), + ); if (!file.existsSync()) { Logger.root.shout('❌ Could not find file CHANGELOG.md for package.'); return false; @@ -62,7 +74,8 @@ class GenerateChangelogCommand extends Command { String? oldLine = line; if (line == '## Unreleased') { logger.info( - 'ℹ️ ## Unreleased headers are no longer needed. Removing.'); + 'ℹ️ ## Unreleased headers are no longer needed. Removing.', + ); oldLine = null; } @@ -78,9 +91,11 @@ class GenerateChangelogCommand extends Command { } print( - 'Verify the CHANGELOG.md changes for all packages and add changes from iOS and Android Native SDK updates.'); + 'Verify the CHANGELOG.md changes for all packages and add changes from iOS and Android Native SDK updates.', + ); print( - 'For reference iOS SDK will be updated to ${args.iOSRelease} and Android SDK will be updated to ${args.androidRelease}.'); + 'For reference iOS SDK will be updated to ${args.iOSRelease} and Android SDK will be updated to ${args.androidRelease}.', + ); return _waitForConfirmation(logger); } @@ -98,121 +113,55 @@ class GenerateChangelogCommand extends Command { return false; } else { logger.shout( - '❓ Not sure what you meant by that... stopping just in case.'); + '❓ Not sure what you meant by that... stopping just in case.', + ); return false; } } return true; } - - Future _findLastReleaseSha( - Logger logger, CommandArguments args, PackageRelease package) async { - final packageTags = await args.gitDir - .tags() - .where((t) => t.tag.startsWith('${package.name}/')) - .toList(); - - Version? _getVersion(Tag tag) { - Version? v; - try { - final versionString = tag.tag.split('/').last.replaceFirst('v', ''); - v = Version.parse(versionString); - } catch (_) { - // Nothing to do - } - return v; - } - - packageTags.sort((a, b) { - Version? versionA = _getVersion(a); - Version? versionB = _getVersion(b); - if (versionA == null) return -1; - if (versionB == null) return 1; - - return versionA.compareTo(versionB); - }); - - if (packageTags.isEmpty) return null; - - final lastTag = packageTags.last; - - logger.fine('Found tag ${lastTag.tag} with sha ${lastTag.objectSha}'); - - return packageTags.last.objectSha; - } - - Future> _getCommits( - CommandArguments args, PackageRelease package, String commitRange) async { - final packageRoot = getPackageRoot(args, package); - final result = await args.gitDir.runCommand([ - '--no-pager', - 'log', - commitRange, - '--pretty=format:%H|||%an <%aE>|||%ai|||%B||||', - '--', - packageRoot - ]); - - final rawCommits = (result.stdout as String) - .split('||||\n') - .where((e) => e.trim().isNotEmpty) - .toList(); - - return rawCommits.map((c) { - final parts = c.split('|||'); - return parts[3].trim(); - }).toList(); - } } List _getChangelogItems(List commitMessages) { - RegExp conventionalCommitPattern = - RegExp(r'(?\w*)(\((?.*)\))?(?!)?: (?.*)'); - RegExp githubIssueMention = RegExp(r'\#(?\d+)'); - final items = []; for (final commitMessage in commitMessages) { - final lines = commitMessage.split('\n'); - final summaryLine = lines[0]; - final match = conventionalCommitPattern.firstMatch(summaryLine); - if (match != null) { - final type = match.namedGroup('type'); - if (type == 'fix' || type == 'feat') { - String changelogItem = ''; - if (match.namedGroup('scope') case final scopes?) { - final scopeList = scopes.split(',').map((e) { - final scope = e.trim(); - if (scopeAbbreviationMap[scope] case final scope?) { - return scope; - } + final commit = ConventionalCommit.parse(commitMessage); + if (commit == null) continue; + + if (commit.type == 'fix' || commit.type == 'feat') { + String changelogItem = ''; + if (commit.scope case final scopes?) { + final scopeList = scopes.split(',').map((e) { + final scope = e.trim(); + if (scopeAbbreviationMap[scope] case final scope?) { return scope; - }); - changelogItem = '[${scopeList.join(', ')}] '; - } - - changelogItem += match.namedGroup('rest')!; - if (!changelogItem.endsWith('.')) { - // Commits frequently forget they're sentences. - changelogItem += '.'; - } - - // Check to see if there are any Github issues referenced - final refLines = lines.where((l) => l.startsWith('refs:')); - var githubRefs = []; - for (var refLine in refLines) { - for (var match in githubIssueMention.allMatches(refLine)) { - githubRefs.add(match.namedGroup('issue_number')!); } - } - if (githubRefs.isNotEmpty) { - final seeStrings = githubRefs - .map((r) => '[#$r](${GenerateChangelogCommand.issuesLink}$r)'); - changelogItem += ' See ${seeStrings.join(' ')}'; - } + return scope; + }); + changelogItem = '[${scopeList.join(', ')}] '; + } - items.add(changelogItem); + changelogItem += commit.description; + if (!changelogItem.endsWith('.')) { + // Commits frequently forget they're sentences. + changelogItem += '.'; } + + // refs: can carry more than GitHub issues (JIRA tickets, etc.) -- + // only build links for the ones that look like a GitHub issue. + final githubIssueNumbers = commit.refs + .map((r) => _githubIssueRefPattern.firstMatch(r)) + .nonNulls + .map((m) => m.namedGroup('issue_number')!); + if (githubIssueNumbers.isNotEmpty) { + final seeStrings = githubIssueNumbers.map( + (r) => '[#$r](${GenerateChangelogCommand.issuesLink}$r)', + ); + changelogItem += ' See ${seeStrings.join(' ')}'; + } + + items.add(changelogItem); } } diff --git a/tools/releaser/lib/git_history.dart b/tools/releaser/lib/git_history.dart new file mode 100644 index 000000000..4df5aa54c --- /dev/null +++ b/tools/releaser/lib/git_history.dart @@ -0,0 +1,61 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:collection/collection.dart'; +import 'package:git/git.dart'; +import 'package:version/version.dart'; + +/// Finds the most recent tag matching `{packageName}/v*`, or null if the +/// package has never been tagged (its first release, or a brand-new +/// federated sub-package -- see [commitMessagesSince]'s no-`sinceSha` path). +Future findLastReleaseTag(GitDir gitDir, String packageName) async { + final prefix = '$packageName/v'; + final matchingTags = await gitDir + .tags() + .where((t) => t.tag.startsWith(prefix)) + .toList(); + + Version? versionOf(Tag tag) { + try { + return Version.parse(tag.tag.substring(prefix.length)); + } catch (_) { + return null; + } + } + + final withVersions = + matchingTags + .map((tag) => (tag, versionOf(tag))) + .where((pair) => pair.$2 != null) + .toList() + ..sort((a, b) => a.$2!.compareTo(b.$2!)); + + return withVersions.lastOrNull?.$1; +} + +/// Full commit messages (subject + body/footers) touching [pathspec], from +/// just after [sinceSha] through HEAD. A null [sinceSha] walks the entire +/// history of [pathspec] -- the "since inception" case for a package with +/// no prior tag. +Future> commitMessagesSince( + GitDir gitDir, { + required String pathspec, + String? sinceSha, +}) async { + final range = sinceSha != null ? '$sinceSha..HEAD' : 'HEAD'; + final result = await gitDir.runCommand([ + '--no-pager', + 'log', + range, + '--pretty=format:%B|||END|||', + '--', + pathspec, + ]); + + return (result.stdout as String) + .split('|||END|||') + .map((m) => m.trim()) + .where((m) => m.isNotEmpty) + .toList(); +} diff --git a/tools/releaser/lib/github_cmd_wrapper.dart b/tools/releaser/lib/github_cmd_wrapper.dart index c9fb47fa2..7ef472e1d 100644 --- a/tools/releaser/lib/github_cmd_wrapper.dart +++ b/tools/releaser/lib/github_cmd_wrapper.dart @@ -57,7 +57,7 @@ class GithubCommandWrapper { '--repo', repoSlug, '--json', - 'name,isLatest,tagName' + 'name,isLatest,tagName', ], workingDirectory: cwd, stdout: (line) => buffer.write(line), @@ -79,13 +79,46 @@ class GithubCommandWrapper { } Future getReleaseByTagName( - Logger logger, String repoSlug, String tagName) async { + Logger logger, + String repoSlug, + String tagName, + ) async { final releases = await fetchReleases(logger, repoSlug); return releases.firstWhereOrNull((e) => e.tagName == tagName); } - Future createRelease(Logger logger, String tag, String name, - String changelog, bool isPrerelease) async { + /// Resolves [ref] (a tag or branch name) to the full commit SHA it + /// currently points to, for pinning native SDKs whose config has no + /// dedicated "verify this tag against this commit" field (CMake's + /// `FetchContent_Declare`, notably) -- the SHA is what's actually pinned. + Future getCommitSha( + Logger logger, + String repoSlug, + String ref, + ) async { + final buffer = StringBuffer(); + final exitCode = await runProcess( + 'gh', + ['api', 'repos/$repoSlug/commits/$ref', '--jq', '.sha'], + workingDirectory: cwd, + stdout: (line) => buffer.write(line), + stderr: (line) => logger.shout(line), + ); + + if (exitCode != 0) { + throw Exception('gh returned exit code $exitCode.'); + } + + return buffer.toString().trim(); + } + + Future createRelease( + Logger logger, + String tag, + String name, + String changelog, + bool isPrerelease, + ) async { final buffer = StringBuffer(); final exitCode = await runProcess( 'gh', @@ -98,7 +131,7 @@ class GithubCommandWrapper { '--notes', changelog, '--draft', - if (isPrerelease) '--prerelease' + if (isPrerelease) '--prerelease', ], workingDirectory: cwd, stdout: (line) => buffer.write(line), diff --git a/tools/releaser/lib/gradle_util.dart b/tools/releaser/lib/gradle_util.dart index 967e8e243..2fac8dfa1 100644 --- a/tools/releaser/lib/gradle_util.dart +++ b/tools/releaser/lib/gradle_util.dart @@ -5,12 +5,10 @@ import 'package:path/path.dart' as path; import 'command.dart'; import 'helpers.dart'; +import 'native_sdk.dart'; import 'package_list.dart'; class UpdateGradleFilesCommand extends Command { - static const versionPrefix = 'ext.datadog_version'; - final versionRegex = RegExp('$versionPrefix = "(.*)"'); - @override Future run(CommandArguments args, Logger logger) async { if (!await _updateGradleFiles(args, logger)) { @@ -36,12 +34,12 @@ class UpdateGradleFilesCommand extends Command { await transformFile(file, logger, args.dryRun, (line) { // For the datadog_flutter_plugin, use a tighter constraint if (file.path.contains('datadog_flutter_plugin')) { - final versionMatch = versionRegex.firstMatch(line); + final versionMatch = androidGradleVersionPattern.firstMatch(line); if (versionMatch != null) { - final oldVersion = versionMatch.group(1); + final oldVersion = versionMatch.namedGroup('version'); line = line.replaceFirst( - '$versionPrefix = "$oldVersion"', - '$versionPrefix = "${args.androidRelease}"', + '$androidGradleVersionPrefix = "$oldVersion"', + '$androidGradleVersionPrefix = "${args.androidRelease}"', ); } } diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart new file mode 100644 index 000000000..d590351d9 --- /dev/null +++ b/tools/releaser/lib/native_sdk.dart @@ -0,0 +1,205 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'dart:io'; + +import 'package:path/path.dart' as p; + +import 'trigger_context.dart'; + +/// A native SDK a Flutter package can depend on. +enum NativeSdk { + ios(repoSlug: 'DataDog/dd-sdk-ios'), + android(repoSlug: 'DataDog/dd-sdk-android'), + cpp(repoSlug: 'DataDog/dd-sdk-cpp'); + + final String repoSlug; + + const NativeSdk({required this.repoSlug}); +} + +/// Matches a podspec's `s.dependency 'Datadog...', ''` lines -- +/// public so `cocoapod_util.dart`'s pin-rewriting step uses the exact same +/// pattern this file reads the current pin with, instead of a second, +/// separately-maintained regex for the same line shape. +final iosPodspecDependencyPattern = RegExp( + r"s\.dependency\s+'(?Datadog\w*)'\s*,\s*'(?[^']+)'", +); + +/// The `ext.datadog_version = "..."` line prefix in a `build.gradle`, and +/// the pattern built from it -- both public so `gradle_util.dart`'s +/// pin-rewriting step shares this file's exact definition of the line +/// rather than maintaining its own copy. +const androidGradleVersionPrefix = 'ext.datadog_version'; +final androidGradleVersionPattern = RegExp( + '$androidGradleVersionPrefix\\s*=\\s*"(?[^"]+)"', +); +final _cmakeGitTagPattern = RegExp( + r'^\s*GIT_TAG\s+(?[\w./-]+).*?(?:#\s*(?\S+))?$', + multiLine: true, +); + +/// The current iOS pin from a podspec's `s.dependency 'Datadog...'` lines +/// (they all share one constraint), or null if it has none. +String? readIosPodspecPin(String podspecContent) => iosPodspecDependencyPattern + .firstMatch(podspecContent) + ?.namedGroup('constraint'); + +/// The current Android pin from a `build.gradle`'s `ext.datadog_version`, +/// or null if it has none. +String? readAndroidGradlePin(String buildGradleContent) => + androidGradleVersionPattern + .firstMatch(buildGradleContent) + ?.namedGroup('version'); + +/// The current C++ pin from a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line, or +/// null if it has none. Once pinned by this tooling, `GIT_TAG` holds a +/// commit SHA with the human-meaningful tag kept as a trailing `# ` +/// comment (see [pinCppVersion] in cmake_util.dart) -- that comment is +/// preferred here so the *tag* is what gets compared run-over-run, not an +/// opaque SHA that would never equal a freshly-resolved target tag. +String? readCppCMakePin(String cmakeListsContent) { + for (final line in cmakeListsContent.split('\n')) { + final match = _cmakeGitTagPattern.firstMatch(line); + if (match != null) { + return match.namedGroup('comment') ?? match.namedGroup('ref'); + } + } + return null; +} + +/// The native-dependency files found in a package's own directory -- +/// resolved by checking what's actually there, not assumed from the +/// package's name or role. +class NativeDependencyFiles { + final File? iosPodspec; + final File? androidGradle; + final List cppCMakeLists; + + NativeDependencyFiles({ + this.iosPodspec, + this.androidGradle, + this.cppCMakeLists = const [], + }); + + bool get isEmpty => + iosPodspec == null && androidGradle == null && cppCMakeLists.isEmpty; +} + +/// Walks [packageRoot] (a package's own directory, not its example/test +/// apps) for the native-dependency files this tooling knows how to read and +/// pin: an iOS podspec with a Datadog pod dependency, an Android +/// `build.gradle` with a `datadog_version`, and/or a `windows/`/`linux/` +/// `CMakeLists.txt` with a dd-sdk-cpp `GIT_TAG`. +NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { + File? iosPodspec; + final iosDir = Directory(p.join(packageRoot, 'ios')); + if (iosDir.existsSync()) { + for (final entity in iosDir.listSync()) { + if (entity is File && + entity.path.endsWith('.podspec') && + iosPodspecDependencyPattern.hasMatch(entity.readAsStringSync())) { + iosPodspec = entity; + break; + } + } + } + + File? androidGradle; + final gradleFile = File(p.join(packageRoot, 'android', 'build.gradle')); + if (gradleFile.existsSync() && + androidGradleVersionPattern.hasMatch(gradleFile.readAsStringSync())) { + androidGradle = gradleFile; + } + + final cppCMakeLists = []; + for (final platformDir in ['windows', 'linux']) { + final cmakeFile = File(p.join(packageRoot, platformDir, 'CMakeLists.txt')); + if (cmakeFile.existsSync() && + _cmakeGitTagPattern.hasMatch(cmakeFile.readAsStringSync())) { + cppCMakeLists.add(cmakeFile); + } + } + + return NativeDependencyFiles( + iosPodspec: iosPodspec, + androidGradle: androidGradle, + cppCMakeLists: cppCMakeLists, + ); +} + +/// What's changing (if anything) for one native SDK dependency of a +/// package. [targetVersion] is null when nothing should change -- the +/// patch-branch default, absent an explicit override. +/// +/// [targetSha] is only meaningful for [NativeSdk.cpp]: CMake's +/// `FetchContent_Declare` has no field for pinning a tag *and* verifying +/// its commit, so the resolved SHA is what actually gets written to +/// `GIT_TAG` (see cmake_util.dart's `pinCppVersion`) -- a full commit SHA +/// is immutable, unlike a tag, which can be moved. +class NativeSdkDelta { + final NativeSdk sdk; + final String? currentPin; + final String? targetVersion; + final String? targetSha; + + NativeSdkDelta({ + required this.sdk, + required this.currentPin, + required this.targetVersion, + this.targetSha, + }); + + bool get isChange => targetVersion != null && targetVersion != currentPin; + + @override + String toString() => isChange + ? '${sdk.name}: $currentPin -> $targetVersion' + : '${sdk.name}: $currentPin (no change)'; +} + +/// The network calls native SDK resolution needs -- bundled so callers +/// (`release_plan.dart`) don't thread three separate function parameters +/// through every layer between `computeReleasePlan` and +/// [resolveNativeSdkTarget]. All three are keyed by a GitHub repo slug +/// (e.g. `DataDog/dd-sdk-ios`) so one instance covers all three SDKs. +class NativeSdkGateways { + final Future Function(String repoSlug) fetchLatest; + final Future Function(String repoSlug, String ref) resolveCommitSha; + final Future Function(String repoSlug, String version) releaseExists; + + const NativeSdkGateways({ + required this.fetchLatest, + required this.resolveCommitSha, + required this.releaseExists, + }); +} + +/// Resolves what a native SDK's pin should become this run: +/// - an explicit [override] wins, but only once [releaseExists] confirms +/// it's a real release -- this is the check `release_validator.dart`'s +/// `_validateReleaseVersion` already did for iOS/Android before this file +/// existed; skipping it would let a typo'd `IOS_SDK_VERSION`/ +/// `ANDROID_SDK_VERSION` sail through undetected until a much later, +/// harder-to-diagnose build failure; +/// - on a patch branch (default: no change) the pin is left alone, since +/// auto-jumping to the latest native SDK defeats the point of an +/// isolated patch; +/// - otherwise (mainline or pre-release), it defaults to the latest +/// published release, resolved via [fetchLatest]. +Future resolveNativeSdkTarget({ + required TriggerContext trigger, + required String? override, + required Future Function() fetchLatest, + required Future Function(String version) releaseExists, +}) async { + if (override != null) { + if (!await releaseExists(override)) { + throw StateError('Release "$override" was not found.'); + } + return override; + } + if (trigger == TriggerContext.patch) return null; + return await fetchLatest(); +} diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index f3be62121..7939fbeea 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -3,20 +3,20 @@ // Copyright 2019-Present Datadog, Inc. import 'package:collection/collection.dart'; +import 'package:git/git.dart'; +import 'package:logging/logging.dart'; +import 'package:version/version.dart'; +import 'conventional_commits.dart'; +import 'git_history.dart'; +import 'github_cmd_wrapper.dart'; +import 'native_sdk.dart'; import 'package_discovery.dart'; +import 'trigger_context.dart'; +import 'version_updater.dart'; -/// Which of the three GitLab trigger contexts a run is happening under: -/// mainline (`develop`), patch (a standing `release/{package}/vX.Y.x` -/// branch), or pre-release (a whitelisted long-lived branch like `v4`). -enum TriggerContext { mainline, patch, preRelease } - -final _patchBranchPattern = RegExp(r'^release/([^/]+)/v\d+\.\d+\.x$'); - -/// Extracts the package name from a `release/{package}/v{major}.{minor}.x` -/// patch-branch name, or null if [branch] doesn't match that convention. -String? packageNameFromPatchBranch(String branch) => - _patchBranchPattern.firstMatch(branch)?.group(1); +export 'trigger_context.dart'; +export 'version_updater.dart'; /// Everything about how a `prepare-release`/`preview-release` run was /// invoked -- shared by both entry points so they can't compute different @@ -54,20 +54,24 @@ class RunContext { class PackagePlan { final DiscoveredPackage package; final String currentVersion; + final String newVersion; + + /// Null for a first release whose history had nothing carrying semver + /// weight. + final VersionBumpType? bumpLevel; - // TODO: compute from conventional-commit bump detection / overrides. - final String? newVersion; - // TODO: compute alongside newVersion. - final String? bumpLevel; - // TODO: resolve from the commits contributing to this release. - final List contributingPrs; + /// The commits that justified this release. + final List contributingCommits; + + final List nativeSdkDeltas; PackagePlan({ required this.package, required this.currentVersion, - this.newVersion, + required this.newVersion, this.bumpLevel, - this.contributingPrs = const [], + this.contributingCommits = const [], + this.nativeSdkDeltas = const [], }); } @@ -83,20 +87,337 @@ class ReleasePlan { /// `preview_release.dart` (which only prints it) so the two can't drift /// apart. /// -/// TODO: conventional-commit bump detection, native SDK version checks, and -/// PR resolution -- `newVersion`/`bumpLevel`/`contributingPrs` are null or -/// empty on every returned plan until those land. -Future computeReleasePlan(RunContext ctx) async { +/// [gitDir] and [nativeSdkGateways] are injectable so tests don't need a +/// real git history or network access; both default to real +/// implementations rooted at [RunContext.repoRoot]. +Future computeReleasePlan( + RunContext ctx, { + GitDir? gitDir, + NativeSdkGateways? nativeSdkGateways, +}) async { + final resolvedGitDir = + gitDir ?? + await GitDir.fromExisting(ctx.repoRoot, allowSubdirectory: true); + final github = GithubCommandWrapper(ctx.repoRoot); + final gateways = + nativeSdkGateways ?? + NativeSdkGateways( + fetchLatest: (repoSlug) async { + final release = await github.getLatestRelease( + Logger('native_sdk'), + repoSlug, + ); + return release.tagName; + }, + resolveCommitSha: (repoSlug, ref) => + github.getCommitSha(Logger('native_sdk'), repoSlug, ref), + releaseExists: (repoSlug, version) async { + final release = await github.getReleaseByTagName( + Logger('native_sdk'), + repoSlug, + version, + ); + return release != null; + }, + ); + final groups = await _resolveGroups(ctx); final selected = _selectPackages(groups, ctx); - final plans = selected - .map((pkg) => PackagePlan(package: pkg, currentVersion: pkg.version)) - .toList(); + final plans = []; + for (final pkg in selected) { + final plan = await _computePackagePlan( + pkg, + ctx, + resolvedGitDir, + gateways, + isExplicitlyRequested: ctx.requestedPackages.contains(pkg.name), + ); + if (plan != null) plans.add(plan); + } return ReleasePlan(trigger: ctx.trigger, packages: plans); } +Future _computePackagePlan( + DiscoveredPackage pkg, + RunContext ctx, + GitDir gitDir, + NativeSdkGateways gateways, { + required bool isExplicitlyRequested, +}) async { + final nativeSdkDeltas = await _computeNativeSdkDeltas(pkg, ctx, gateways); + final hasNativeSdkChange = nativeSdkDeltas.any((d) => d.isChange); + + switch (ctx.trigger) { + case TriggerContext.patch: + return await _computePatchPlan(pkg, gitDir, nativeSdkDeltas); + case TriggerContext.preRelease: + return await _computePrereleasePlan(pkg, ctx, gitDir, nativeSdkDeltas); + case TriggerContext.mainline: + return await _computeMainlinePlan( + pkg, + ctx, + gitDir, + nativeSdkDeltas, + isExplicitlyRequested: isExplicitlyRequested, + hasNativeSdkChange: hasNativeSdkChange, + ); + } +} + +Future _computePatchPlan( + DiscoveredPackage pkg, + GitDir gitDir, + List nativeSdkDeltas, +) async { + final lastTag = await findLastReleaseTag(gitDir, pkg.name); + final commits = await _conventionalCommitsSince( + gitDir, + pathspec: pkg.relativePath, + sinceSha: lastTag?.objectSha, + ); + + for (final commit in commits) { + final bump = commit.bumpType; + if (bump == VersionBumpType.major || bump == VersionBumpType.minor) { + throw StateError( + 'Commit looks like a ${bump!.name} change, which does not belong on ' + 'a patch branch (only fixes are allowed here):\n' + '${commit.type}: ${commit.description}', + ); + } + } + + final newVersion = lastTag == null + ? pkg.version + : Version.parse( + _versionFromTag(lastTag, pkg.name), + ).incrementPatch().toString(); + + return PackagePlan( + package: pkg, + currentVersion: pkg.version, + newVersion: newVersion, + bumpLevel: VersionBumpType.patch, + contributingCommits: commits, + nativeSdkDeltas: nativeSdkDeltas, + ); +} + +Future _computePrereleasePlan( + DiscoveredPackage pkg, + RunContext ctx, + GitDir gitDir, + List nativeSdkDeltas, +) async { + final lastTag = await findLastReleaseTag(gitDir, pkg.name); + final base = Version.parse( + lastTag != null ? _versionFromTag(lastTag, pkg.name) : pkg.version, + ); + + final Version newVersion; + if (base.isPreRelease && + (ctx.prereleaseLabel == null || + base.preRelease.first == ctx.prereleaseLabel)) { + newVersion = base.incrementPreRelease(); + } else if (ctx.prereleaseLabel != null) { + newVersion = Version( + base.major, + base.minor, + base.patch, + preRelease: [ctx.prereleaseLabel!, '1'], + ); + } else { + throw StateError( + 'No prior pre-release tag for "${pkg.name}" at base version $base -- ' + 'PRERELEASE_LABEL is required the first time a label is used against ' + 'a given base version.', + ); + } + + return PackagePlan( + package: pkg, + currentVersion: pkg.version, + newVersion: newVersion.toString(), + bumpLevel: VersionBumpType.prerelease, + nativeSdkDeltas: nativeSdkDeltas, + ); +} + +Future _computeMainlinePlan( + DiscoveredPackage pkg, + RunContext ctx, + GitDir gitDir, + List nativeSdkDeltas, { + required bool isExplicitlyRequested, + required bool hasNativeSdkChange, +}) async { + final lastTag = await findLastReleaseTag(gitDir, pkg.name); + final commits = await _conventionalCommitsSince( + gitDir, + pathspec: pkg.relativePath, + sinceSha: lastTag?.objectSha, + ); + + var bump = + VersionBumpType.parseOverride(ctx.bumpTypeOverride) ?? + aggregateBumpLevel(commits); + + if (bump == null) { + if (!isExplicitlyRequested && !hasNativeSdkChange) { + // Nothing changed for this package and nobody asked for it by name -- + // --all auto-detection leaves it out of this run entirely. + return null; + } + // Explicitly requested, or only a native SDK bump is driving this + // release: still worth a release, treated as a maintenance patch. + bump = lastTag == null ? null : VersionBumpType.patch; + } + + final newVersion = lastTag == null + ? pkg.version + : _applyBump( + Version.parse(_versionFromTag(lastTag, pkg.name)), + bump!, + ).toString(); + + return PackagePlan( + package: pkg, + currentVersion: pkg.version, + newVersion: newVersion, + bumpLevel: bump, + contributingCommits: commits, + nativeSdkDeltas: nativeSdkDeltas, + ); +} + +Version _applyBump(Version base, VersionBumpType bump) { + switch (bump) { + case VersionBumpType.major: + return base.incrementMajor(); + case VersionBumpType.minor: + return base.incrementMinor(); + case VersionBumpType.patch: + return base.incrementPatch(); + case VersionBumpType.prerelease: + throw ArgumentError( + '_applyBump does not handle prerelease bumps -- see ' + '_computePrereleasePlan for that path.', + ); + } +} + +/// A tag's name is `{package}/v{version}` -- strip the known prefix rather +/// than trusting [Tag.tag]'s shape blindly. +String _versionFromTag(Tag tag, String packageName) => + tag.tag.substring('$packageName/v'.length); + +Future> _computeNativeSdkDeltas( + DiscoveredPackage pkg, + RunContext ctx, + NativeSdkGateways gateways, +) async { + final files = resolveNativeDependencyFiles(pkg.absolutePath(ctx.repoRoot)); + final deltas = []; + + if (files.iosPodspec != null) { + final currentPin = readIosPodspecPin(files.iosPodspec!.readAsStringSync()); + final target = await resolveNativeSdkTarget( + trigger: ctx.trigger, + override: ctx.iosSdkVersionOverride, + fetchLatest: () => gateways.fetchLatest(NativeSdk.ios.repoSlug), + releaseExists: (version) => + gateways.releaseExists(NativeSdk.ios.repoSlug, version), + ); + deltas.add( + NativeSdkDelta( + sdk: NativeSdk.ios, + currentPin: currentPin, + targetVersion: target, + ), + ); + } + + if (files.androidGradle != null) { + final currentPin = readAndroidGradlePin( + files.androidGradle!.readAsStringSync(), + ); + final target = await resolveNativeSdkTarget( + trigger: ctx.trigger, + override: ctx.androidSdkVersionOverride, + fetchLatest: () => gateways.fetchLatest(NativeSdk.android.repoSlug), + releaseExists: (version) => + gateways.releaseExists(NativeSdk.android.repoSlug, version), + ); + deltas.add( + NativeSdkDelta( + sdk: NativeSdk.android, + currentPin: currentPin, + targetVersion: target, + ), + ); + } + + if (files.cppCMakeLists.isNotEmpty) { + final currentPin = readCppCMakePin( + files.cppCMakeLists.first.readAsStringSync(), + ); + final target = await resolveNativeSdkTarget( + trigger: ctx.trigger, + override: ctx.cppVersionOverride, + fetchLatest: () => gateways.fetchLatest(NativeSdk.cpp.repoSlug), + releaseExists: (version) => + gateways.releaseExists(NativeSdk.cpp.repoSlug, version), + ); + // CMake's FetchContent_Declare has no field for pinning a tag and + // verifying its commit -- the resolved SHA is what actually gets + // written to GIT_TAG (see cmake_util.dart's pinCppVersion). + final targetSha = target != null + ? await gateways.resolveCommitSha(NativeSdk.cpp.repoSlug, target) + : null; + deltas.add( + NativeSdkDelta( + sdk: NativeSdk.cpp, + currentPin: currentPin, + targetVersion: target, + targetSha: targetSha, + ), + ); + } + + return deltas; +} + +/// [commitMessagesSince]'s raw messages, parsed into [ConventionalCommit]s +/// and narrowed to the ones that carry semver weight -- commits that fail +/// to parse, or parse but don't bump anything (`chore:`, `docs:`, etc.), +/// are dropped since nothing here cares about them either as bump input or +/// as a [PackagePlan.contributingCommits] entry. +Future> _conventionalCommitsSince( + GitDir gitDir, { + required String pathspec, + String? sinceSha, +}) async { + final messages = await commitMessagesSince( + gitDir, + pathspec: pathspec, + sinceSha: sinceSha, + ); + return messages + .map(ConventionalCommit.parse) + .nonNulls + .where((c) => c.bumpType != null) + .toList(); +} + +final _patchBranchPattern = RegExp(r'^release/([^/]+)/v\d+\.\d+\.x$'); + +/// Extracts the package name from a `release/{package}/v{major}.{minor}.x` +/// patch-branch name, or null if [branch] doesn't match that convention. +String? _packageNameFromPatchBranch(String branch) => + _patchBranchPattern.firstMatch(branch)?.group(1); + Future> _resolveGroups(RunContext ctx) async { final allGroups = await discoverPackages(ctx.repoRoot); @@ -107,7 +428,7 @@ Future> _resolveGroups(RunContext ctx) async { // Patch branches never use topology -- a caret constraint already // tolerates a sibling staying behind, so only the one named package // moves, with no grouping applied to it. - final packageName = packageNameFromPatchBranch(ctx.currentBranch); + final packageName = _packageNameFromPatchBranch(ctx.currentBranch); if (packageName == null) { throw StateError( 'Branch "${ctx.currentBranch}" does not match the ' diff --git a/tools/releaser/lib/trigger_context.dart b/tools/releaser/lib/trigger_context.dart new file mode 100644 index 000000000..7ab73bacc --- /dev/null +++ b/tools/releaser/lib/trigger_context.dart @@ -0,0 +1,14 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +/// Which of the three GitLab trigger contexts a run is happening under: +/// mainline (`develop`), patch (a standing `release/{package}/vX.Y.x` +/// branch), or pre-release (a whitelisted long-lived branch like `v4`). +/// +/// Lives in its own file (rather than alongside `release_plan.dart`, its +/// only real "owner") because `native_sdk.dart` also needs it, and +/// `release_plan.dart` in turn needs types from `native_sdk.dart` -- +/// putting the enum in either file would create a cyclic import between +/// the two. +enum TriggerContext { mainline, patch, preRelease } diff --git a/tools/releaser/lib/version_updater.dart b/tools/releaser/lib/version_updater.dart index 76be986bb..665a53495 100644 --- a/tools/releaser/lib/version_updater.dart +++ b/tools/releaser/lib/version_updater.dart @@ -12,7 +12,42 @@ import 'package:version/version.dart'; import 'command.dart'; import 'helpers.dart'; -enum VersionBumpType { major, minor, rev, prerelease } +enum VersionBumpType { + patch(severity: 0), + minor(severity: 1), + major(severity: 2), + // Not part of the major/minor/patch severity ordering below -- assigned + // directly from a prerelease label/counter, never derived by comparing + // against another bump level. + prerelease(severity: -1); + + /// Where this bump ranks against another major/minor/patch bump (higher + /// wins). Meaningless for [prerelease], which is never compared this way. + final int severity; + + const VersionBumpType({required this.severity}); + + /// Parses a `BUMP_TYPE` per-run override -- major/minor/patch only, or + /// null for "no override given" (a null or empty [raw]). `prerelease` + /// isn't a valid override; that path is driven by `PRERELEASE_LABEL` on + /// a pre-release branch instead. Throws rather than silently falling + /// back on anything unrecognized (a typo'd `BUMP_TYPE` used to silently + /// become a patch bump). + static VersionBumpType? parseOverride(String? raw) { + if (raw == null || raw.isEmpty) return null; + + switch (raw) { + case 'major': + return VersionBumpType.major; + case 'minor': + return VersionBumpType.minor; + case 'patch': + return VersionBumpType.patch; + default: + throw StateError('BUMP_TYPE "$raw" is not one of major, minor, patch.'); + } + } +} class UpdateVersionsCommand extends Command { @override @@ -20,13 +55,18 @@ class UpdateVersionsCommand extends Command { for (final package in args.packages) { final packageRoot = getPackageRoot(args, package); if (!await updateVersions( - packageRoot, package.version, logger, args.dryRun)) { + packageRoot, + package.version, + logger, + args.dryRun, + )) { return false; } } - final corePackage = args.packages - .firstWhereOrNull((e) => e.name == 'datadog_flutter_plugin'); + final corePackage = args.packages.firstWhereOrNull( + (e) => e.name == 'datadog_flutter_plugin', + ); if (corePackage != null) { if (!await _updateReadmeVersions(args, corePackage, logger)) { @@ -60,7 +100,7 @@ class BumpVersionCommand extends Command { case VersionBumpType.minor: newVersion = version.incrementMinor(); break; - case VersionBumpType.rev: + case VersionBumpType.patch: newVersion = version.incrementPatch(); break; case VersionBumpType.prerelease: @@ -68,15 +108,20 @@ class BumpVersionCommand extends Command { newVersion = version.incrementPreRelease(); } catch (e) { logger.shout( - '❌ Failed to increment the pre-release version of $version. Is it not a pre-release?'); + '❌ Failed to increment the pre-release version of $version. Is it not a pre-release?', + ); return false; } break; } logger.info('🔀 Bumping version to $newVersion'); - success &= await updateVersions(getPackageRoot(args, package), - newVersion.toString(), logger, args.dryRun); + success &= await updateVersions( + getPackageRoot(args, package), + newVersion.toString(), + logger, + args.dryRun, + ); } return success; } @@ -85,7 +130,11 @@ class BumpVersionCommand extends Command { final _versionCapture = RegExp(r'^version\: (?.*)'); Future updateVersions( - String packageRoot, String version, Logger logger, bool dryRun) async { + String packageRoot, + String version, + Logger logger, + bool dryRun, +) async { if (!await _updatePackagePubspec(packageRoot, version, logger, dryRun)) { return false; } @@ -96,7 +145,11 @@ Future updateVersions( } Future _updatePackagePubspec( - String packageRoot, String version, Logger logger, bool dryRun) async { + String packageRoot, + String version, + Logger logger, + bool dryRun, +) async { final pubspecFile = File(path.join(packageRoot, 'pubspec.yaml')); if (!pubspecFile.existsSync()) { logger.shout('⁉️ Could not find pubspec.yaml at ${pubspecFile.path}'); @@ -107,8 +160,9 @@ Future _updatePackagePubspec( final match = _versionCapture.firstMatch(element); if (match != null) { final oldVersion = match.namedGroup('version'); - logger - .info(' - 🔀 Replacing version $oldVersion with $version in pubspec'); + logger.info( + ' - 🔀 Replacing version $oldVersion with $version in pubspec', + ); element = 'version: $version'; } return element; @@ -118,7 +172,11 @@ Future _updatePackagePubspec( } Future _updateVersionDartFile( - String packageRoot, String version, Logger logger, bool dryRun) async { + String packageRoot, + String version, + Logger logger, + bool dryRun, +) async { final versionFile = File(path.join(packageRoot, 'lib/src/version.dart')); if (!versionFile.existsSync()) { logger.shout('⁉️ Could not find version.dart at ${versionFile.path}'); @@ -137,7 +195,10 @@ Future _updateVersionDartFile( } Future _updateReadmeVersions( - CommandArguments args, PackageRelease package, Logger logger) async { + CommandArguments args, + PackageRelease package, + Logger logger, +) async { final packageRoot = getPackageRoot(args, package); final changelogFile = File(path.join(packageRoot, 'README.md')); if (!changelogFile.existsSync()) { @@ -152,7 +213,8 @@ Future _updateReadmeVersions( inVersionTable = false; // Write the new version table: - line = '''[//]: # (SDK Table) + line = + '''[//]: # (SDK Table) | iOS SDK | Android SDK | Browser SDK | | :-----: | :---------: | :---------: | @@ -176,20 +238,26 @@ Future _updateReadmeVersions( } Future _updateNativeSDKVersions( - CommandArguments args, PackageRelease package, Logger logger) async { + CommandArguments args, + PackageRelease package, + Logger logger, +) async { final packageRoot = getPackageRoot(args, package); - final nativeSDKVersionsFile = - File(path.join(packageRoot, 'NATIVE_SDK_VERSIONS.md')); + final nativeSDKVersionsFile = File( + path.join(packageRoot, 'NATIVE_SDK_VERSIONS.md'), + ); final newVersionEntry = '| ${package.version} | ${args.iOSRelease} | ${args.androidRelease} |'; final header = '| Flutter | iOS SDK | Android SDK |'; final separator = '|---------|---------|-------------|'; if (!nativeSDKVersionsFile.existsSync()) { - logger - .warning('⚠️ NATIVE_SDK_VERSIONS.md does not exist, creating it now.'); - await nativeSDKVersionsFile - .writeAsString('$header\n$separator\n$newVersionEntry'); + logger.warning( + '⚠️ NATIVE_SDK_VERSIONS.md does not exist, creating it now.', + ); + await nativeSDKVersionsFile.writeAsString( + '$header\n$separator\n$newVersionEntry', + ); return true; } @@ -200,7 +268,8 @@ Future _updateNativeSDKVersions( final parts = line.split('|').map((s) => s.trim()).toList(); if (parts.length > 1 && parts[1] == package.version) { logger.info( - '✅ Version ${package.version} already exists in NATIVE_SDK_VERSIONS.md, skipping.'); + '✅ Version ${package.version} already exists in NATIVE_SDK_VERSIONS.md, skipping.', + ); return true; } } diff --git a/tools/releaser/pubspec.lock b/tools/releaser/pubspec.lock index 2f7629cc6..b96a67a8e 100644 --- a/tools/releaser/pubspec.lock +++ b/tools/releaser/pubspec.lock @@ -258,7 +258,7 @@ packages: source: hosted version: "0.12.17" meta: - dependency: transitive + dependency: "direct main" description: name: meta sha256: "23f08335362185a5ea2ad3a4e597f1375e78bce8a040df5c600c8d3552ef2394" diff --git a/tools/releaser/pubspec.yaml b/tools/releaser/pubspec.yaml index 29ad42667..276189ff6 100644 --- a/tools/releaser/pubspec.yaml +++ b/tools/releaser/pubspec.yaml @@ -16,6 +16,7 @@ dependencies: collection: ^1.19.1 glob: ^2.1.3 pubspec_parse: ^1.5.0 + meta: ^1.16.0 dev_dependencies: build_runner: ^2.15.0 diff --git a/tools/releaser/test/cmake_util_test.dart b/tools/releaser/test/cmake_util_test.dart new file mode 100644 index 000000000..9b8116c56 --- /dev/null +++ b/tools/releaser/test/cmake_util_test.dart @@ -0,0 +1,107 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'dart:io'; + +import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'package:releaser/cmake_util.dart'; + +const _sha = 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'; + +void main() { + late Directory root; + final logger = Logger('cmake_util_test'); + + setUp(() async { + root = await Directory.systemTemp.createTemp('cmake_util_test_'); + }); + + tearDown(() => root.delete(recursive: true)); + + test('pins GIT_TAG to the SHA, keeping the tag as a trailing comment, ' + 'preserving a trailing GIT_CONFIG line', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString(''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop + GIT_CONFIG core.longpaths=true) +'''); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + final contents = await file.readAsString(); + expect(contents, contains('GIT_TAG $_sha # v1.4.0')); + expect(contents, contains('GIT_CONFIG core.longpaths=true)')); + expect(contents, isNot(contains('develop'))); + }); + + test( + 'keeps the closing paren before the comment when it is on the same line', + () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString(''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +'''); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + final contents = await file.readAsString(); + // The `)` must stay *before* the comment -- moving it after would + // comment out the closing paren and break FetchContent_Declare(...). + expect(contents, contains('GIT_TAG $_sha) # v1.4.0')); + }, + ); + + test( + 're-pinning replaces both the old SHA and the old comment cleanly', + () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString( + ' GIT_TAG oldsha1234567890oldsha1234567890oldsha1) # v1.3.0\n', + ); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + final contents = await file.readAsString(); + // trimRight only -- trim() would also eat the leading indentation + // this test is specifically checking gets preserved. + expect(contents.trimRight(), ' GIT_TAG $_sha) # v1.4.0'); + }, + ); + + test('dry run leaves the file untouched', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + const original = ' GIT_TAG develop\n'; + await file.writeAsString(original); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, true); + + expect(await file.readAsString(), original); + }); + + test('leaves everything else in the file untouched', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString(''' +cmake_minimum_required(VERSION 3.14) +set(PROJECT_NAME "datadog_flutter_plugin_desktop") + +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +FetchContent_MakeAvailable(dd-sdk-cpp) +'''); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + final contents = await file.readAsString(); + expect(contents, contains('cmake_minimum_required(VERSION 3.14)')); + expect(contents, contains('FetchContent_MakeAvailable(dd-sdk-cpp)')); + }); +} diff --git a/tools/releaser/test/conventional_commits_test.dart b/tools/releaser/test/conventional_commits_test.dart new file mode 100644 index 000000000..862af1f71 --- /dev/null +++ b/tools/releaser/test/conventional_commits_test.dart @@ -0,0 +1,103 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:test/test.dart'; + +import 'package:releaser/conventional_commits.dart'; +import 'package:releaser/version_updater.dart'; + +/// Shorthand matching the old `classifyCommitBump` free function's +/// behavior, for tests that just want a message's bump type. +VersionBumpType? _bumpOf(String commitMessage) => + ConventionalCommit.parse(commitMessage)?.bumpType; + +/// Parses each message, dropping any that don't parse -- for feeding +/// [aggregateBumpLevel], which now operates on [ConventionalCommit]s. +List _parseAll(List messages) => + messages.map(ConventionalCommit.parse).nonNulls.toList(); + +void main() { + group('ConventionalCommit.parse -- bumpType', () { + test('feat: is a minor bump', () { + expect( + _bumpOf('feat: add frustration signal tracking'), + VersionBumpType.minor, + ); + }); + + test('fix: and perf: are patch bumps', () { + expect( + _bumpOf('fix: correct crash on session init'), + VersionBumpType.patch, + ); + expect(_bumpOf('perf: reduce startup overhead'), VersionBumpType.patch); + }); + + test('a ! after the type/scope is a major bump regardless of type', () { + expect( + _bumpOf('feat!: remove deprecated trackEvent API'), + VersionBumpType.major, + ); + expect( + _bumpOf('fix(ios)!: change method signature'), + VersionBumpType.major, + ); + }); + + test('a BREAKING CHANGE footer is a major bump', () { + final message = + 'feat: add new config option\n\n' + 'BREAKING CHANGE: the old option is removed'; + expect(_bumpOf(message), VersionBumpType.major); + }); + + test('chore/docs/test do not carry semver weight on their own', () { + expect(_bumpOf('chore: bump dependencies'), isNull); + expect(_bumpOf('docs: fix typo in README'), isNull); + expect(_bumpOf('test: add missing coverage'), isNull); + }); + + test('a non-conventional-commit message fails to parse entirely', () { + expect( + ConventionalCommit.parse('Merge pull request #123 from foo/bar'), + isNull, + ); + }); + }); + + group('aggregateBumpLevel', () { + test('picks the highest severity across all commits', () { + final bump = aggregateBumpLevel( + _parseAll([ + 'fix: correct crash', + 'chore: cleanup', + 'feat: add a thing', + ]), + ); + expect(bump, VersionBumpType.minor); + }); + + test('a single breaking commit outranks everything else', () { + final bump = aggregateBumpLevel( + _parseAll([ + 'fix: correct crash', + 'feat!: remove old API', + 'feat: add a thing', + ]), + ); + expect(bump, VersionBumpType.major); + }); + + test('returns null when nothing carries semver weight', () { + final bump = aggregateBumpLevel( + _parseAll(['chore: cleanup', 'docs: fix typo']), + ); + expect(bump, isNull); + }); + + test('returns null for an empty list', () { + expect(aggregateBumpLevel([]), isNull); + }); + }); +} diff --git a/tools/releaser/test/git_history_test.dart b/tools/releaser/test/git_history_test.dart new file mode 100644 index 000000000..7037b19d3 --- /dev/null +++ b/tools/releaser/test/git_history_test.dart @@ -0,0 +1,107 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:test/test.dart'; + +import 'package:releaser/git_history.dart'; +import 'support/fixture_repo.dart'; + +void main() { + late FixtureRepo fixture; + + setUp(() async { + fixture = await FixtureRepo.create(); + }); + + tearDown(() => fixture.delete()); + + test('finds the highest-versioned tag for a package', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.2.0 work'); + await fixture.commit('fix: something for 2.2.0'); + await fixture.tag('datadog_dio/v2.2.0'); + + fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.3.0 work'); + await fixture.commit('feat: something for 2.3.0'); + await fixture.tag('datadog_dio/v2.3.0'); + + final gitDir = await fixture.gitDir; + final tag = await findLastReleaseTag(gitDir, 'datadog_dio'); + + expect(tag, isNotNull); + expect(tag!.tag, 'datadog_dio/v2.3.0'); + }); + + test('returns null when a package has never been tagged', () async { + final gitDir = await fixture.gitDir; + final tag = await findLastReleaseTag(gitDir, 'datadog_flags'); + expect(tag, isNull); + }); + + test( + 'commitMessagesSince only returns commits after the given sha', + () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'released'); + await fixture.commit('fix: shipped in 2.3.0'); + await fixture.tag('datadog_dio/v2.3.0'); + final tagSha = (await findLastReleaseTag( + await fixture.gitDir, + 'datadog_dio', + ))!.objectSha; + + fixture.writeFile('packages/datadog_dio/CHANGES', 'unreleased'); + await fixture.commit('feat: not yet released'); + + final commits = await commitMessagesSince( + await fixture.gitDir, + pathspec: 'packages/datadog_dio', + sinceSha: tagSha, + ); + + expect(commits, hasLength(1)); + expect(commits.single, contains('feat: not yet released')); + }, + ); + + test( + 'commitMessagesSince walks full history when sinceSha is null', + () async { + fixture.writeFile('packages/datadog_flags/CHANGES', 'a'); + await fixture.commit('feat: first ever commit for datadog_flags'); + fixture.writeFile('packages/datadog_flags/CHANGES', 'b'); + await fixture.commit('fix: second commit for datadog_flags'); + + final commits = await commitMessagesSince( + await fixture.gitDir, + pathspec: 'packages/datadog_flags', + sinceSha: null, + ); + + expect(commits, hasLength(2)); + }, + ); + + test( + 'commitMessagesSince excludes commits touching other packages', + () async { + // datadog_flags doesn't exist in the fixture layout, so unlike + // datadog_dio (touched by the fixture's own initial commit) its + // history starts clean here. + fixture.writeFile('packages/datadog_flags/CHANGES', 'flags change 1'); + await fixture.commit('feat: change to flags only'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'dio change'); + await fixture.commit('feat: change to dio only'); + fixture.writeFile('packages/datadog_flags/CHANGES', 'flags change 2'); + await fixture.commit('fix: another change to flags only'); + + final commits = await commitMessagesSince( + await fixture.gitDir, + pathspec: 'packages/datadog_flags', + sinceSha: null, + ); + + expect(commits, hasLength(2)); + expect(commits.every((c) => !c.contains('dio only')), isTrue); + }, + ); +} diff --git a/tools/releaser/test/native_sdk_test.dart b/tools/releaser/test/native_sdk_test.dart new file mode 100644 index 000000000..d01b7a55e --- /dev/null +++ b/tools/releaser/test/native_sdk_test.dart @@ -0,0 +1,241 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'dart:io'; + +import 'package:path/path.dart' as p; +import 'package:test/test.dart'; + +import 'package:releaser/native_sdk.dart'; +import 'package:releaser/trigger_context.dart'; + +const _iosPodspec = ''' +Pod::Spec.new do |s| + s.name = 'datadog_flutter_plugin_ios' + s.dependency 'Flutter' + s.dependency 'DatadogCore', '~> 3' + s.dependency 'DatadogLogs', '~> 3' +end +'''; + +const _androidGradle = ''' +buildscript { + ext.kotlin_version = "2.2.20" + ext.datadog_version = "3.11.0" +} +'''; + +const _windowsCMakeLists = ''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop + GIT_CONFIG core.longpaths=true) +'''; + +const _linuxCMakeLists = ''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +'''; + +void main() { + group('reading current pins', () { + test('readIosPodspecPin finds the shared Datadog pod constraint', () { + expect(readIosPodspecPin(_iosPodspec), '~> 3'); + }); + + test('readIosPodspecPin returns null with no Datadog dependency', () { + expect(readIosPodspecPin("s.dependency 'Flutter'"), isNull); + }); + + test('readAndroidGradlePin finds ext.datadog_version', () { + expect(readAndroidGradlePin(_androidGradle), '3.11.0'); + }); + + test('readAndroidGradlePin returns null with no datadog_version', () { + expect(readAndroidGradlePin('ext.kotlin_version = "2.2.20"'), isNull); + }); + + test('readCppCMakePin finds GIT_TAG regardless of trailing syntax', () { + expect(readCppCMakePin(_windowsCMakeLists), 'develop'); + expect(readCppCMakePin(_linuxCMakeLists), 'develop'); + }); + + test( + 'readCppCMakePin prefers the trailing tag comment over a pinned SHA', + () { + const pinned = ''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2) # v1.4.0 +'''; + // The tag, not the opaque SHA -- that's what a freshly-resolved + // target tag needs to compare against to detect "no change". + expect(readCppCMakePin(pinned), 'v1.4.0'); + }, + ); + + test('readCppCMakePin returns null with no GIT_TAG', () { + expect(readCppCMakePin('FetchContent_Declare(something_else)'), isNull); + }); + }); + + group('resolveNativeDependencyFiles', () { + late Directory root; + + setUp(() async { + root = await Directory.systemTemp.createTemp('native_sdk_test_'); + }); + + tearDown(() => root.delete(recursive: true)); + + void write(String relativePath, String contents) { + final file = File(p.join(root.path, relativePath)); + file.parent.createSync(recursive: true); + file.writeAsStringSync(contents); + } + + test('finds all three native dependency files when present', () { + write('ios/datadog_flutter_plugin_ios.podspec', _iosPodspec); + write('android/build.gradle', _androidGradle); + write('windows/CMakeLists.txt', _windowsCMakeLists); + write('linux/CMakeLists.txt', _linuxCMakeLists); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.iosPodspec, isNotNull); + expect(files.androidGradle, isNotNull); + expect(files.cppCMakeLists, hasLength(2)); + expect(files.isEmpty, isFalse); + }); + + test('ignores a build.gradle with no Datadog dependency', () { + write('android/build.gradle', 'ext.kotlin_version = "2.2.20"'); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.androidGradle, isNull); + expect(files.isEmpty, isTrue); + }); + + test('ignores a podspec with no Datadog dependency', () { + write('ios/some_other_plugin.podspec', "s.dependency 'Flutter'"); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.iosPodspec, isNull); + }); + + test( + 'a pure-Dart package with no ios/android/windows/linux dirs is empty', + () { + final files = resolveNativeDependencyFiles(root.path); + expect(files.isEmpty, isTrue); + }, + ); + }); + + group('resolveNativeSdkTarget', () { + test( + 'an explicit override wins once confirmed to be a real release', + () async { + final target = await resolveNativeSdkTarget( + trigger: TriggerContext.mainline, + override: '3.12.0', + fetchLatest: () => Future.value('9.9.9'), + releaseExists: (version) async => version == '3.12.0', + ); + expect(target, '3.12.0'); + }, + ); + + test('an override that is not a real release fails loudly', () async { + await expectLater( + resolveNativeSdkTarget( + trigger: TriggerContext.mainline, + override: 'not-a-real-version', + fetchLatest: () => Future.value('9.9.9'), + releaseExists: (version) async => false, + ), + throwsStateError, + ); + }); + + test('on a patch branch, no override means no change (and no ' + 'releaseExists call)', () async { + final target = await resolveNativeSdkTarget( + trigger: TriggerContext.patch, + override: null, + fetchLatest: () => Future.value('9.9.9'), + releaseExists: (version) => + throw StateError('should not be called with no override'), + ); + expect(target, isNull); + }); + + test('a patch-branch override still applies', () async { + final target = await resolveNativeSdkTarget( + trigger: TriggerContext.patch, + override: '3.12.1', + fetchLatest: () => Future.value('9.9.9'), + releaseExists: (version) async => true, + ); + expect(target, '3.12.1'); + }); + + test( + 'on develop, no override means latest (and no releaseExists call)', + () async { + final target = await resolveNativeSdkTarget( + trigger: TriggerContext.mainline, + override: null, + fetchLatest: () => Future.value('3.13.0'), + releaseExists: (version) => + throw StateError('should not be called with no override'), + ); + expect(target, '3.13.0'); + }, + ); + + test('on a pre-release branch, no override also means latest', () async { + final target = await resolveNativeSdkTarget( + trigger: TriggerContext.preRelease, + override: null, + fetchLatest: () => Future.value('3.13.0'), + releaseExists: (version) => + throw StateError('should not be called with no override'), + ); + expect(target, '3.13.0'); + }); + }); + + group('NativeSdkDelta.isChange', () { + test('is false when the target matches the current pin', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.android, + currentPin: '3.11.0', + targetVersion: '3.11.0', + ); + expect(delta.isChange, isFalse); + }); + + test('is false when there is no target (no change)', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.android, + currentPin: '3.11.0', + targetVersion: null, + ); + expect(delta.isChange, isFalse); + }); + + test('is true when the target differs from the current pin', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.android, + currentPin: '3.11.0', + targetVersion: '3.12.0', + ); + expect(delta.isChange, isTrue); + }); + }); +} diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index df2e906ff..e07a48c18 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -2,11 +2,24 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import 'package:releaser/native_sdk.dart'; +import 'package:releaser/release_plan.dart'; import 'package:test/test.dart'; -import 'package:releaser/release_plan.dart'; import 'support/fixture_repo.dart'; +const _iosPodspecWithDatadogDependency = ''' +Pod::Spec.new do |s| + s.dependency 'DatadogCore', '~> 3' +end +'''; + +const _windowsCMakeListsWithGitTag = ''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +'''; + void main() { late FixtureRepo fixture; @@ -16,117 +29,398 @@ void main() { tearDown(() => fixture.delete()); - test( - 'mainline with no requestedPackages returns every discovered package', - () async { - final plan = await computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.mainline, - currentBranch: 'develop', - ), - ); + Future plan( + RunContext ctx, { + Future Function(String repoSlug)? fetchLatestNativeSdkVersion, + Future Function(String repoSlug, String ref)? resolveCommitSha, + Future Function(String repoSlug, String version)? releaseExists, + }) async => computeReleasePlan( + ctx, + gitDir: await fixture.gitDir, + nativeSdkGateways: NativeSdkGateways( + fetchLatest: + fetchLatestNativeSdkVersion ?? + (repoSlug) => throw StateError('fetchLatest not stubbed'), + resolveCommitSha: + resolveCommitSha ?? + (repoSlug, ref) => throw StateError('resolveCommitSha not stubbed'), + // Real releaseExists calls `gh`, which isn't available/desired in + // tests -- default to "yes" unless a test specifically cares. + releaseExists: releaseExists ?? (repoSlug, version) async => true, + ), + ); - final names = plan.packages.map((p) => p.package.name); - expect( - names, - containsAll([ - 'datadog_dio', - 'datadog_flutter_plugin', - 'datadog_flutter_plugin_ios', - 'lonely_ios', - ]), - ); - expect(names, isNot(contains('datadog_common_test'))); - }, + RunContext mainlineCtx({ + List requestedPackages = const [], + String? bumpTypeOverride, + String? iosSdkVersionOverride, + String? cppVersionOverride, + }) => RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.mainline, + currentBranch: 'develop', + requestedPackages: requestedPackages, + bumpTypeOverride: bumpTypeOverride, + iosSdkVersionOverride: iosSdkVersionOverride, + cppVersionOverride: cppVersionOverride, ); - test('mainline with requestedPackages filters to just those', () async { - final plan = await computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.mainline, - currentBranch: 'develop', - requestedPackages: ['datadog_dio', 'datadog_flutter_plugin_ios'], - ), + group('mainline, package selection', () { + test('--all excludes packages with no qualifying commits', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'a real feature'); + await fixture.commit('feat: add a real feature to dio'); + + final result = await plan(mainlineCtx()); + final names = result.packages.map((p) => p.package.name); + + expect(names, contains('datadog_dio')); + // Nothing but the fixture's own initial chore commit ever touched + // this -- excluded from an --all run. + expect(names, isNot(contains('lonely_ios'))); + }); + + test( + 'explicitly requesting a package includes it even with nothing new', + () async { + final result = await plan( + mainlineCtx(requestedPackages: ['lonely_ios']), + ); + + expect(result.packages, hasLength(1)); + expect(result.packages.single.package.name, 'lonely_ios'); + }, ); - expect( - plan.packages.map((p) => p.package.name), - unorderedEquals(['datadog_dio', 'datadog_flutter_plugin_ios']), + test( + 'a native SDK override alone is enough to include a package in --all', + () async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + await fixture.commit('chore: add podspec fixture'); + + final result = await plan(mainlineCtx(iosSdkVersionOverride: '3.12.0')); + final names = result.packages.map((p) => p.package.name); + + expect(names, contains('datadog_flutter_plugin_ios')); + }, ); }); - test('mainline with an unknown requested package throws', () async { - await expectLater( - computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.mainline, - currentBranch: 'develop', - requestedPackages: ['datadog_dio', 'does_not_exist'], + group('mainline, version computation', () { + test('mainline with an unknown requested package throws', () async { + await expectLater( + computeReleasePlan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.mainline, + currentBranch: 'develop', + requestedPackages: ['datadog_dio', 'does_not_exist'], + ), ), - ), - throwsStateError, + throwsStateError, + ); + }); + + test( + 'a package with no prior tag ships its declared version as-is', + () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'a feature'); + await fixture.commit('feat: add a feature'); + + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_dio']), + ); + final dio = result.packages.single; + + expect(dio.bumpLevel, VersionBumpType.minor); + expect(dio.newVersion, '2.3.0'); // unchanged -- first release + }, ); - }); - test( - 'patch branch resolves the single named package with no grouping', - () async { - final plan = await computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.patch, - currentBranch: 'release/datadog_dio/v1.1.x', + test('a package with a prior tag increments from that tag, not from ' + 'whatever pubspec currently says', () async { + await fixture.tag('datadog_dio/v2.0.0'); + // pubspec still says 2.3.0 (set up by the fixture), simulating + // drift between the last real tag and an already-bumped pubspec. + fixture.writeFile('packages/datadog_dio/CHANGES', 'a fix'); + await fixture.commit('fix: correct a bug'); + + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_dio']), + ); + final dio = result.packages.single; + + expect(dio.bumpLevel, VersionBumpType.patch); + expect(dio.newVersion, '2.0.1'); + }); + + test( + 'BUMP_TYPE override wins over conventional-commit detection', + () async { + await fixture.tag('datadog_dio/v2.0.0'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'a fix'); + await fixture.commit('fix: correct a bug'); + + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_dio'], + bumpTypeOverride: 'major', + ), + ); + + expect(result.packages.single.newVersion, '3.0.0'); + }, + ); + + test( + 'an invalid BUMP_TYPE fails loudly rather than defaulting to patch', + () async { + await expectLater( + plan( + mainlineCtx( + requestedPackages: ['datadog_dio'], + bumpTypeOverride: 'oops', + ), + ), + throwsStateError, + ); + }, + ); + + test('BUMP_TYPE=prerelease is rejected on mainline', () async { + await expectLater( + plan( + mainlineCtx( + requestedPackages: ['datadog_dio'], + bumpTypeOverride: 'prerelease', + ), ), + throwsStateError, ); + }); - expect(plan.packages, hasLength(1)); - expect(plan.packages.single.package.name, 'datadog_dio'); - }, - ); + test('explicitly requested with nothing qualifying and a prior tag ' + 'still gets a maintenance patch bump', () async { + await fixture.tag('lonely_ios/v1.1.0'); + + final result = await plan(mainlineCtx(requestedPackages: ['lonely_ios'])); + + expect(result.packages.single.bumpLevel, VersionBumpType.patch); + expect(result.packages.single.newVersion, '1.1.1'); + }); + }); - test( - 'patch branch on a federated member still selects only that one package', - () async { - final plan = await computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.patch, - currentBranch: 'release/datadog_flutter_plugin_ios/v1.0.x', + group('mainline, native SDK deltas', () { + setUp(() async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + await fixture.commit('chore: add podspec fixture'); + }); + + test('an override is used directly, without calling fetchLatest', () async { + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_flutter_plugin_ios'], + iosSdkVersionOverride: '3.12.0', ), + fetchLatestNativeSdkVersion: (_) => + throw StateError('should not be called when overridden'), ); - expect(plan.packages, hasLength(1)); - expect(plan.packages.single.package.name, 'datadog_flutter_plugin_ios'); - }, - ); + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.currentPin, '~> 3'); + expect(delta.targetVersion, '3.12.0'); + expect(delta.isChange, isTrue); + }); + + test('with no override, resolves via fetchLatest', () async { + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_flutter_plugin_ios']), + fetchLatestNativeSdkVersion: (repoSlug) async { + expect(repoSlug, 'DataDog/dd-sdk-ios'); + return 'v3.13.0'; + }, + ); - test('patch branch with a malformed name throws', () async { - await expectLater( - computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.patch, - currentBranch: 'not-a-patch-branch', + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.targetVersion, 'v3.13.0'); + }); + + test('a package with no native dependency files has no deltas', () async { + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_dio']), + ); + expect(result.packages.single.nativeSdkDeltas, isEmpty); + }); + }); + + group('mainline, C++ native SDK delta (CMake GIT_TAG + SHA)', () { + setUp(() async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' + 'windows/CMakeLists.txt', + _windowsCMakeListsWithGitTag, + ); + await fixture.commit('chore: add CMakeLists fixture'); + }); + + test('resolves the target tag to a commit SHA', () async { + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_flutter_plugin_desktop'], + cppVersionOverride: 'v1.4.0', ), - ), - throwsStateError, + resolveCommitSha: (repoSlug, ref) async { + expect(repoSlug, 'DataDog/dd-sdk-cpp'); + expect(ref, 'v1.4.0'); + return 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'; + }, + ); + + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.currentPin, 'develop'); + expect(delta.targetVersion, 'v1.4.0'); + expect(delta.targetSha, 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'); + }); + + test( + 'does not resolve a SHA when there is no target (patch, no override)', + () async { + final result = await plan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.patch, + currentBranch: 'release/datadog_flutter_plugin_desktop/v1.0.x', + ), + resolveCommitSha: (repoSlug, ref) => + throw StateError('should not be called with no target'), + ); + + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.targetVersion, isNull); + expect(delta.targetSha, isNull); + }, ); }); - test('patch branch naming an unknown package throws', () async { - await expectLater( - computeReleasePlan( - RunContext( - repoRoot: fixture.root.path, - trigger: TriggerContext.patch, - currentBranch: 'release/does_not_exist/v1.0.x', - ), - ), - throwsStateError, + group('patch branch', () { + RunContext patchCtx(String branch) => RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.patch, + currentBranch: branch, ); + + test('resolves the single named package with no grouping', () async { + final result = await plan(patchCtx('release/datadog_dio/v1.1.x')); + expect(result.packages, hasLength(1)); + expect(result.packages.single.package.name, 'datadog_dio'); + }); + + test('on a federated member still selects only that one package', () async { + final result = await plan( + patchCtx('release/datadog_flutter_plugin_ios/v1.0.x'), + ); + expect(result.packages, hasLength(1)); + expect(result.packages.single.package.name, 'datadog_flutter_plugin_ios'); + }); + + test('a malformed branch name throws', () async { + await expectLater(plan(patchCtx('not-a-patch-branch')), throwsStateError); + }); + + test('naming an unknown package throws', () async { + await expectLater( + plan(patchCtx('release/does_not_exist/v1.0.x')), + throwsStateError, + ); + }); + + test('forces a patch bump, incrementing from the last tag', () async { + await fixture.tag('datadog_dio/v2.0.0'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'a fix'); + await fixture.commit('fix: a cherry-picked fix'); + + final result = await plan(patchCtx('release/datadog_dio/v2.0.x')); + + expect(result.packages, hasLength(1)); + expect(result.packages.single.bumpLevel, VersionBumpType.patch); + expect(result.packages.single.newVersion, '2.0.1'); + }); + + test('fails loudly if a feat commit snuck onto the patch branch', () async { + await fixture.tag('datadog_dio/v2.0.0'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'oops'); + await fixture.commit('feat: this should not be on a patch branch'); + + await expectLater( + plan(patchCtx('release/datadog_dio/v2.0.x')), + throwsStateError, + ); + }); + + test('fails loudly on a breaking change too', () async { + await fixture.tag('datadog_dio/v2.0.0'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'oops'); + await fixture.commit('fix!: this breaks things'); + + await expectLater( + plan(patchCtx('release/datadog_dio/v2.0.x')), + throwsStateError, + ); + }); + }); + + group('pre-release branch', () { + RunContext preReleaseCtx({String? prereleaseLabel}) => RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.preRelease, + currentBranch: 'v4', + requestedPackages: ['datadog_flutter_plugin'], + prereleaseLabel: prereleaseLabel, + ); + + test('the first prerelease for a base version requires a label', () async { + await expectLater(plan(preReleaseCtx()), throwsStateError); + }); + + test('starts a new label at .1', () async { + final result = await plan(preReleaseCtx(prereleaseLabel: 'beta')); + expect(result.packages.single.newVersion, '4.0.0-beta.1'); + expect(result.packages.single.bumpLevel, VersionBumpType.prerelease); + }); + + test('increments the counter for an already-used label', () async { + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.1'); + + final result = await plan(preReleaseCtx(prereleaseLabel: 'beta')); + + expect(result.packages.single.newVersion, '4.0.0-beta.2'); + }); + + test( + 'omitting the label continues whatever label is already tagged', + () async { + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.1'); + + final result = await plan(preReleaseCtx()); + + expect(result.packages.single.newVersion, '4.0.0-beta.2'); + }, + ); + + test('switching to a new label restarts the counter at .1', () async { + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.3'); + + final result = await plan(preReleaseCtx(prereleaseLabel: 'rc')); + + expect(result.packages.single.newVersion, '4.0.0-rc.1'); + }); }); test('patch branch ignores an inherited requestedPackages filter', () async { diff --git a/tools/releaser/test/support/fixture_repo.dart b/tools/releaser/test/support/fixture_repo.dart index 0b75c354b..d0f671f04 100644 --- a/tools/releaser/test/support/fixture_repo.dart +++ b/tools/releaser/test/support/fixture_repo.dart @@ -4,21 +4,26 @@ import 'dart:io'; +import 'package:git/git.dart'; import 'package:path/path.dart' as p; /// A throwaway pubspec.yaml tree, mirroring the shapes that matter in -/// dd-sdk-flutter's real `packages/` layout (a federated group with all -/// four real platform implementations, several singletons, an -/// intentionally-unpublished support package, an example app, and a -/// package whose name merely *looks* federated). Discovery tests run -/// against this instead of the real tree so they don't depend on -- or -/// get broken by -- packages/ changing over time. +/// dd-sdk-flutter's real `packages/` layout (a federated group, several +/// singletons, an intentionally-unpublished support package, an example +/// app, and a package whose name merely *looks* federated). Discovery +/// tests run against this instead of the real tree so they don't depend +/// on -- or get broken by -- packages/ changing over time. +/// +/// Also a real git repo (unless [withGit] is false) with an initial commit, +/// so release-plan tests can layer real commits/tags on top of this same +/// layout instead of needing a second, separately-maintained fixture. class FixtureRepo { final Directory root; + GitDir? _gitDir; FixtureRepo._(this.root); - static Future create() async { + static Future create({bool withGit = true}) async { final root = await Directory.systemTemp.createTemp('releaser_test_'); final repo = FixtureRepo._(root); @@ -85,9 +90,43 @@ class FixtureRepo { version: '4.0.0', ); + if (withGit) { + await repo._run(['init', '-q', '-b', 'main']); + await repo._run(['config', 'user.email', 'releaser-test@example.com']); + await repo._run(['config', 'user.name', 'Releaser Test']); + await repo.commit('chore: Initial fixture layout'); + } + return repo; } + /// Writes (or overwrites) a file at [relativePath], creating parent + /// directories as needed. Does not commit -- call [commit] separately. + void writeFile(String relativePath, String contents) { + final file = File(p.join(root.path, relativePath)); + file.parent.createSync(recursive: true); + file.writeAsStringSync(contents); + } + + /// Stages everything and commits. [body] lines (e.g. a `BREAKING CHANGE:` + /// footer) are appended after a blank line, matching real commit shape. + Future commit(String subject, {List body = const []}) async { + await _run(['add', '.']); + final message = body.isEmpty ? subject : '$subject\n\n${body.join('\n')}'; + await _run(['commit', '-q', '--allow-empty', '-m', message]); + } + + // Annotated, not lightweight -- this machine's git config signs tags, + // which requires a message (`git tag ` alone fails with + // "no tag message?"). + Future tag(String name) => _run(['tag', '-a', '-m', name, name]); + + Future get gitDir async => + _gitDir ??= await GitDir.fromExisting(root.path); + + Future _run(List args) => + Process.run('git', args, workingDirectory: root.path); + void _writePubspec( String relativeDir, { required String name, From 3c6a613e29047957d4c71443757696517d94bb9d Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Mon, 24 Aug 2026 16:43:49 -0400 Subject: [PATCH 02/10] Add spm support to version pinning / release plan --- tools/releaser/lib/native_sdk.dart | 77 ++++++++++++++++-- tools/releaser/lib/release_plan.dart | 10 ++- tools/releaser/lib/release_validator.dart | 80 +++++++++++++------ tools/releaser/lib/spm_util.dart | 18 +++-- tools/releaser/test/native_sdk_test.dart | 90 +++++++++++++++++++++- tools/releaser/test/release_plan_test.dart | 32 ++++++++ 6 files changed, 266 insertions(+), 41 deletions(-) diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index d590351d9..874ab215e 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -35,6 +35,14 @@ const androidGradleVersionPrefix = 'ext.datadog_version'; final androidGradleVersionPattern = RegExp( '$androidGradleVersionPrefix\\s*=\\s*"(?[^"]+)"', ); + +/// Matches a `Package.swift`'s dd-sdk-ios dependency line (e.g. +/// `.package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: +/// "3.0.0")`) -- public so `spm_util.dart`'s pin-rewriting step uses the +/// exact same pattern this file reads the current pin with. +final iosSpmDependencyPattern = RegExp( + r'\.package\(url:\s*"(?[^"]*dd-sdk-ios[^"]*)",\s*(?[^)]+)\)', +); final _cmakeGitTagPattern = RegExp( r'^\s*GIT_TAG\s+(?[\w./-]+).*?(?:#\s*(?\S+))?$', multiLine: true, @@ -53,6 +61,26 @@ String? readAndroidGradlePin(String buildGradleContent) => .firstMatch(buildGradleContent) ?.namedGroup('version'); +/// The current iOS pin from a `Package.swift`'s dd-sdk-ios dependency spec +/// (e.g. `from: "3.0.0"`, `exact: "3.5.0"`, `branch: "develop"`), or null if +/// it has none. Kept separate from [readIosPodspecPin] since a package can +/// carry both a podspec (CocoaPods) and a `Package.swift` (SPM) pinning the +/// same dependency in independently-formatted ways. +String? readSpmPin(String packageSwiftContent) => + iosSpmDependencyPattern.firstMatch(packageSwiftContent)?.namedGroup('spec'); + +/// Pulls the bare version literal out of an SPM dependency spec whose kind +/// pins to a specific version (`exact: "3.12.0"` / `from: "3.0.0"` -> +/// `3.12.0`/`3.0.0`), or null for a spec with nothing to compare (`branch: +/// "develop"`) -- used to tell whether a `Package.swift` needs to be +/// brought in line with a resolved target, the same way [readIosPodspecPin] +/// is compared against one. +final _spmVersionLiteralPattern = RegExp( + r'^(?:exact|from|upToNextMajor|upToNextMinor):\s*"(?[^"]+)"', +); +String? _spmPinnedVersion(String spec) => + _spmVersionLiteralPattern.firstMatch(spec)?.namedGroup('version'); + /// The current C++ pin from a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line, or /// null if it has none. Once pinned by this tooling, `GIT_TAG` holds a /// commit SHA with the human-meaningful tag kept as a trailing `# ` @@ -74,26 +102,37 @@ String? readCppCMakePin(String cmakeListsContent) { /// package's name or role. class NativeDependencyFiles { final File? iosPodspec; + + /// A `Package.swift` pinning dd-sdk-ios via SPM -- independent of + /// [iosPodspec] since a package can ship both, each pinning the same + /// dependency in its own file format. + final File? iosSpmManifest; final File? androidGradle; final List cppCMakeLists; NativeDependencyFiles({ this.iosPodspec, + this.iosSpmManifest, this.androidGradle, this.cppCMakeLists = const [], }); bool get isEmpty => - iosPodspec == null && androidGradle == null && cppCMakeLists.isEmpty; + iosPodspec == null && + iosSpmManifest == null && + androidGradle == null && + cppCMakeLists.isEmpty; } /// Walks [packageRoot] (a package's own directory, not its example/test /// apps) for the native-dependency files this tooling knows how to read and -/// pin: an iOS podspec with a Datadog pod dependency, an Android -/// `build.gradle` with a `datadog_version`, and/or a `windows/`/`linux/` -/// `CMakeLists.txt` with a dd-sdk-cpp `GIT_TAG`. +/// pin: an iOS podspec with a Datadog pod dependency, a `Package.swift` +/// pinning dd-sdk-ios via SPM, an Android `build.gradle` with a +/// `datadog_version`, and/or a `windows/`/`linux/` `CMakeLists.txt` with a +/// dd-sdk-cpp `GIT_TAG`. NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { File? iosPodspec; + File? iosSpmManifest; final iosDir = Directory(p.join(packageRoot, 'ios')); if (iosDir.existsSync()) { for (final entity in iosDir.listSync()) { @@ -101,7 +140,15 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { entity.path.endsWith('.podspec') && iosPodspecDependencyPattern.hasMatch(entity.readAsStringSync())) { iosPodspec = entity; - break; + } else if (entity is Directory) { + // The real layout is `ios//Package.swift` -- a + // manifest for building the plugin's iOS code via SPM instead of + // CocoaPods. + final packageSwift = File(p.join(entity.path, 'Package.swift')); + if (packageSwift.existsSync() && + iosSpmDependencyPattern.hasMatch(packageSwift.readAsStringSync())) { + iosSpmManifest = packageSwift; + } } } } @@ -124,6 +171,7 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { return NativeDependencyFiles( iosPodspec: iosPodspec, + iosSpmManifest: iosSpmManifest, androidGradle: androidGradle, cppCMakeLists: cppCMakeLists, ); @@ -144,14 +192,31 @@ class NativeSdkDelta { final String? targetVersion; final String? targetSha; + /// The separate pin an [NativeSdk.ios] package's `Package.swift` (SPM) + /// carries, when it has one -- always null for [NativeSdk.android]/ + /// [NativeSdk.cpp]. A podspec and a `Package.swift` pin the same + /// dependency independently, in their own file formats, so this is read + /// (and compared in [isChange]) separately from [currentPin] -- this + /// tool always pins both to the exact same version, so either one + /// drifting from [targetVersion] counts as a change. + final String? currentSpmPin; + NativeSdkDelta({ required this.sdk, required this.currentPin, required this.targetVersion, this.targetSha, + this.currentSpmPin, }); - bool get isChange => targetVersion != null && targetVersion != currentPin; + bool get isChange { + if (targetVersion == null) return false; + final podspecOutOfDate = currentPin != null && currentPin != targetVersion; + final spmOutOfDate = + currentSpmPin != null && + _spmPinnedVersion(currentSpmPin!) != targetVersion; + return podspecOutOfDate || spmOutOfDate; + } @override String toString() => isChange diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index 7939fbeea..20069db43 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -321,8 +321,13 @@ Future> _computeNativeSdkDeltas( final files = resolveNativeDependencyFiles(pkg.absolutePath(ctx.repoRoot)); final deltas = []; - if (files.iosPodspec != null) { - final currentPin = readIosPodspecPin(files.iosPodspec!.readAsStringSync()); + if (files.iosPodspec != null || files.iosSpmManifest != null) { + final currentPin = files.iosPodspec != null + ? readIosPodspecPin(files.iosPodspec!.readAsStringSync()) + : null; + final currentSpmPin = files.iosSpmManifest != null + ? readSpmPin(files.iosSpmManifest!.readAsStringSync()) + : null; final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.iosSdkVersionOverride, @@ -335,6 +340,7 @@ Future> _computeNativeSdkDeltas( sdk: NativeSdk.ios, currentPin: currentPin, targetVersion: target, + currentSpmPin: currentSpmPin, ), ); } diff --git a/tools/releaser/lib/release_validator.dart b/tools/releaser/lib/release_validator.dart index 9755c6e30..eb91eae0e 100644 --- a/tools/releaser/lib/release_validator.dart +++ b/tools/releaser/lib/release_validator.dart @@ -9,7 +9,9 @@ import 'package:path/path.dart' as path; import 'command.dart'; import 'github_cmd_wrapper.dart'; import 'helpers.dart'; +import 'native_sdk.dart'; import 'process_helper.dart'; +import 'trigger_context.dart'; final versionHeadingRegEx = RegExp(r'\s*#'); final changeItemRegEx = RegExp(r'\s*\*'); @@ -48,7 +50,8 @@ class ValidateReleaseCommand extends Command { // Don't allow unstaged changes if (!await gitDir.isWorkingTreeClean()) { logger.shout( - '❌ Working tree is not clean. Please stage or revert your changes before attempting to release.'); + '❌ Working tree is not clean. Please stage or revert your changes before attempting to release.', + ); return false; } @@ -57,7 +60,8 @@ class ValidateReleaseCommand extends Command { if (!(currentBranch.branchName == 'develop' || currentBranch.branchName.startsWith('release'))) { logger.shout( - '❌ We really should only release from `develop` or another `release` branch.'); + '❌ We really should only release from `develop` or another `release` branch.', + ); return false; } @@ -65,16 +69,32 @@ class ValidateReleaseCommand extends Command { } Future _validateiOSRelease( - String packagePath, CommandArguments args, Logger logger) async { + String packagePath, + CommandArguments args, + Logger logger, + ) async { args.iOSRelease = await _validateReleaseVersion( - args, 'DataDog/dd-sdk-ios', 'iOS', args.iOSRelease, logger); + args, + 'DataDog/dd-sdk-ios', + 'iOS', + args.iOSRelease, + logger, + ); return args.iOSRelease != null; } Future _validateAndroidRelease( - String packagePath, CommandArguments args, Logger logger) async { + String packagePath, + CommandArguments args, + Logger logger, + ) async { args.androidRelease = await _validateReleaseVersion( - args, 'DataDog/dd-sdk-android', 'Android', args.androidRelease, logger); + args, + 'DataDog/dd-sdk-android', + 'Android', + args.androidRelease, + logger, + ); return args.androidRelease != null; } @@ -86,26 +106,38 @@ class ValidateReleaseCommand extends Command { String? release, Logger logger, ) async { - // If we didn't specify a version get the current latest release from github. - // If we did specify a release, check that it actually exists. final gh = GithubCommandWrapper(args.gitDir.path); - if (release == null) { - logger.fine('🌎 Fetching latest $platform release from github... '); - final latestRelease = await gh.getLatestRelease(logger, repoName); - logger.fine('ℹ️ Latest $platform release is ${latestRelease.name}'); - release = latestRelease.tagName; - } else { - final ghRelease = await gh.getReleaseByTagName(logger, repoName, release); - if (ghRelease == null) { - logger.shout( - '❌ Could not find target $platform release $release. Please check the tag name'); - return null; - } - } - - logger.info('ℹ️ Releasing with $platform version $release.'); + try { + // This legacy CLI has no notion of a patch/pre-release trigger + // context -- it always behaves like `mainline`: default to the + // latest release when none is given, or validate an explicit one. + final resolved = await resolveNativeSdkTarget( + trigger: TriggerContext.mainline, + override: release, + fetchLatest: () async { + logger.fine('🌎 Fetching latest $platform release from github... '); + final latestRelease = await gh.getLatestRelease(logger, repoName); + logger.fine('ℹ️ Latest $platform release is ${latestRelease.name}'); + return latestRelease.tagName; + }, + releaseExists: (version) async { + final ghRelease = await gh.getReleaseByTagName( + logger, + repoName, + version, + ); + return ghRelease != null; + }, + ); - return release; + logger.info('ℹ️ Releasing with $platform version $resolved.'); + return resolved; + } on StateError { + logger.shout( + '❌ Could not find target $platform release $release. Please check the tag name', + ); + return null; + } } } diff --git a/tools/releaser/lib/spm_util.dart b/tools/releaser/lib/spm_util.dart index 0b3613e80..c984057da 100644 --- a/tools/releaser/lib/spm_util.dart +++ b/tools/releaser/lib/spm_util.dart @@ -9,18 +9,17 @@ import 'package:path/path.dart' as path; import 'command.dart'; import 'helpers.dart'; +import 'native_sdk.dart'; const datadogIosRepo = 'https://github.com/Datadog/dd-sdk-ios.git'; -final packageDependencyPattern = RegExp( - r'\s+\.package\(url\: "(?.+)", .+\)', -); class PinSwiftPackageVersion extends Command { @override Future run(CommandArguments args, Logger logger) async { // Other packages can keep looser version constraints - final corePacakge = args.packages - .firstWhereOrNull((e) => e.name == 'datadog_flutter_plugin'); + final corePacakge = args.packages.firstWhereOrNull( + (e) => e.name == 'datadog_flutter_plugin', + ); if (corePacakge != null) { if (!await _pinSpmVersion(args, corePacakge, logger)) { return false; @@ -31,7 +30,10 @@ class PinSwiftPackageVersion extends Command { } Future _pinSpmVersion( - CommandArguments args, PackageRelease package, Logger logger) { + CommandArguments args, + PackageRelease package, + Logger logger, + ) { return pinSpmVersion( args.gitDir.path, package.name, @@ -63,8 +65,8 @@ class PinSwiftPackageVersion extends Command { logger.info('ℹ️ Setting the iOS Pod Dependency to $versionString'); await transformFile(file, logger, dryRun, (line) { - final match = packageDependencyPattern.firstMatch(line); - if (match != null && match.namedGroup('package') == datadogIosRepo) { + final match = iosSpmDependencyPattern.firstMatch(line); + if (match != null && match.namedGroup('url') == datadogIosRepo) { final needsComma = line.trimRight().endsWith(','); line = ' .package(url: "$datadogIosRepo", $versionString)${needsComma ? ',' : ''}'; diff --git a/tools/releaser/test/native_sdk_test.dart b/tools/releaser/test/native_sdk_test.dart index d01b7a55e..b34ac431f 100644 --- a/tools/releaser/test/native_sdk_test.dart +++ b/tools/releaser/test/native_sdk_test.dart @@ -39,6 +39,15 @@ FetchContent_Declare(dd-sdk-cpp GIT_TAG develop) '''; +const _packageSwift = ''' +let package = Package( + name: "datadog_session_replay", + dependencies: [ + .package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: "3.0.0") + ] +) +'''; + void main() { group('reading current pins', () { test('readIosPodspecPin finds the shared Datadog pod constraint', () { @@ -79,6 +88,19 @@ FetchContent_Declare(dd-sdk-cpp test('readCppCMakePin returns null with no GIT_TAG', () { expect(readCppCMakePin('FetchContent_Declare(something_else)'), isNull); }); + + test('readSpmPin finds the dd-sdk-ios dependency spec', () { + expect(readSpmPin(_packageSwift), 'from: "3.0.0"'); + }); + + test('readSpmPin returns null with no dd-sdk-ios dependency', () { + expect( + readSpmPin( + '.package(url: "https://github.com/other/pkg.git", from: "1.0.0")', + ), + isNull, + ); + }); }); group('resolveNativeDependencyFiles', () { @@ -96,8 +118,9 @@ FetchContent_Declare(dd-sdk-cpp file.writeAsStringSync(contents); } - test('finds all three native dependency files when present', () { + test('finds all native dependency files when present', () { write('ios/datadog_flutter_plugin_ios.podspec', _iosPodspec); + write('ios/datadog_flutter_plugin_ios/Package.swift', _packageSwift); write('android/build.gradle', _androidGradle); write('windows/CMakeLists.txt', _windowsCMakeLists); write('linux/CMakeLists.txt', _linuxCMakeLists); @@ -105,11 +128,35 @@ FetchContent_Declare(dd-sdk-cpp final files = resolveNativeDependencyFiles(root.path); expect(files.iosPodspec, isNotNull); + expect(files.iosSpmManifest, isNotNull); expect(files.androidGradle, isNotNull); expect(files.cppCMakeLists, hasLength(2)); expect(files.isEmpty, isFalse); }); + test( + 'finds a Package.swift even when there is no podspec alongside it', + () { + write('ios/datadog_flutter_plugin_ios/Package.swift', _packageSwift); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.iosPodspec, isNull); + expect(files.iosSpmManifest, isNotNull); + }, + ); + + test('ignores a Package.swift with no dd-sdk-ios dependency', () { + write( + 'ios/some_other_plugin/Package.swift', + '.package(url: "https://github.com/other/pkg.git", from: "1.0.0")', + ); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.iosSpmManifest, isNull); + }); + test('ignores a build.gradle with no Datadog dependency', () { write('android/build.gradle', 'ext.kotlin_version = "2.2.20"'); @@ -237,5 +284,46 @@ FetchContent_Declare(dd-sdk-cpp ); expect(delta.isChange, isTrue); }); + + test('is true when the podspec matches but the SPM pin lags behind -- ' + 'both must track the same version', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.ios, + currentPin: '3.12.0', + currentSpmPin: 'from: "3.0.0"', + targetVersion: '3.12.0', + ); + expect(delta.isChange, isTrue); + }); + + test('is true when the SPM pin matches but the podspec lags behind', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.ios, + currentPin: '~> 3', + currentSpmPin: 'exact: "3.12.0"', + targetVersion: '3.12.0', + ); + expect(delta.isChange, isTrue); + }); + + test('is false when both the podspec and SPM pin match the target', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.ios, + currentPin: '3.12.0', + currentSpmPin: 'exact: "3.12.0"', + targetVersion: '3.12.0', + ); + expect(delta.isChange, isFalse); + }); + + test('a branch-tracking SPM pin always counts as needing a change', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.ios, + currentPin: '3.12.0', + currentSpmPin: 'branch: "develop"', + targetVersion: '3.12.0', + ); + expect(delta.isChange, isTrue); + }); }); } diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index e07a48c18..1052b820a 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -20,6 +20,14 @@ FetchContent_Declare(dd-sdk-cpp GIT_TAG develop) '''; +const _packageSwiftWithDatadogDependency = ''' +let package = Package( + dependencies: [ + .package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: "3.0.0") + ] +) +'''; + void main() { late FixtureRepo fixture; @@ -258,6 +266,30 @@ void main() { ); expect(result.packages.single.nativeSdkDeltas, isEmpty); }); + + test( + 'a Package.swift alongside the podspec surfaces its own current pin', + () async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios/Package.swift', + _packageSwiftWithDatadogDependency, + ); + await fixture.commit('chore: add Package.swift fixture'); + + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_flutter_plugin_ios'], + iosSdkVersionOverride: '3.12.0', + ), + ); + + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.currentPin, '~> 3'); + expect(delta.currentSpmPin, 'from: "3.0.0"'); + expect(delta.targetVersion, '3.12.0'); + }, + ); }); group('mainline, C++ native SDK delta (CMake GIT_TAG + SHA)', () { From 13f1f31e88964afa21d6db7d4ccf16f92138fb62 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Tue, 25 Aug 2026 15:25:20 -0400 Subject: [PATCH 03/10] Address review feedback Refactor version pinning information to eliminate special cases and instead have the "source" of the pin and the version that was pinned. This is so the iOS SDK and the C++ SDK can report what versions are pinned in SPM / Cocoapods or Windows / Linux respectively without special cases for each, and allows us to look at pinned version for both Linux and Windows (linux was previously being ignored.) Fix issue where we would get the wrong last release when patching older releases. --- tools/releaser/lib/git_history.dart | 17 +++- tools/releaser/lib/native_sdk.dart | 55 +++++++------ tools/releaser/lib/release_plan.dart | 96 ++++++++++++++++------ tools/releaser/test/git_history_test.dart | 22 +++++ tools/releaser/test/native_sdk_test.dart | 66 ++++++++++++--- tools/releaser/test/release_plan_test.dart | 83 ++++++++++++++++++- 6 files changed, 271 insertions(+), 68 deletions(-) diff --git a/tools/releaser/lib/git_history.dart b/tools/releaser/lib/git_history.dart index 4df5aa54c..bb9c60fba 100644 --- a/tools/releaser/lib/git_history.dart +++ b/tools/releaser/lib/git_history.dart @@ -9,7 +9,16 @@ import 'package:version/version.dart'; /// Finds the most recent tag matching `{packageName}/v*`, or null if the /// package has never been tagged (its first release, or a brand-new /// federated sub-package -- see [commitMessagesSince]'s no-`sinceSha` path). -Future findLastReleaseTag(GitDir gitDir, String packageName) async { +/// +/// [releaseLine], when given, restricts the search to tags whose +/// major/minor matches -- a patch branch's own release line, so a tag +/// mainline has since cut for a newer major/minor (which is not an +/// ancestor of the patch branch) can't be picked up instead. +Future findLastReleaseTag( + GitDir gitDir, + String packageName, { + (int major, int minor)? releaseLine, +}) async { final prefix = '$packageName/v'; final matchingTags = await gitDir .tags() @@ -28,6 +37,12 @@ Future findLastReleaseTag(GitDir gitDir, String packageName) async { matchingTags .map((tag) => (tag, versionOf(tag))) .where((pair) => pair.$2 != null) + .where( + (pair) => + releaseLine == null || + (pair.$2!.major == releaseLine.$1 && + pair.$2!.minor == releaseLine.$2), + ) .toList() ..sort((a, b) => a.$2!.compareTo(b.$2!)); diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index 874ab215e..17be9b656 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -71,15 +71,16 @@ String? readSpmPin(String packageSwiftContent) => /// Pulls the bare version literal out of an SPM dependency spec whose kind /// pins to a specific version (`exact: "3.12.0"` / `from: "3.0.0"` -> -/// `3.12.0`/`3.0.0`), or null for a spec with nothing to compare (`branch: -/// "develop"`) -- used to tell whether a `Package.swift` needs to be -/// brought in line with a resolved target, the same way [readIosPodspecPin] -/// is compared against one. +/// `3.12.0`/`3.0.0`). A spec with nothing to compare (`branch: "develop"`) +/// is returned unchanged, so it never accidentally equals a resolved +/// target and always reads as needing a pin -- used to normalize a +/// `Package.swift` pin into the same shape as [readIosPodspecPin]'s before +/// the two are compared as just another entry in [NativeSdkDelta.pins]. +String spmPinForComparison(String spec) => + _spmVersionLiteralPattern.firstMatch(spec)?.namedGroup('version') ?? spec; final _spmVersionLiteralPattern = RegExp( r'^(?:exact|from|upToNextMajor|upToNextMinor):\s*"(?[^"]+)"', ); -String? _spmPinnedVersion(String spec) => - _spmVersionLiteralPattern.firstMatch(spec)?.namedGroup('version'); /// The current C++ pin from a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line, or /// null if it has none. Once pinned by this tooling, `GIT_TAG` holds a @@ -177,6 +178,12 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { ); } +/// One file's current pin on a native SDK dependency, and where it came +/// from (e.g. `'podspec'`, `'Package.swift'`, `'windows/CMakeLists.txt'`) -- +/// the label exists purely so a stale pin can be reported back to whoever's +/// reading the plan, not for any comparison logic. +typedef NativeSdkPin = ({String source, String value}); + /// What's changing (if anything) for one native SDK dependency of a /// package. [targetVersion] is null when nothing should change -- the /// patch-branch default, absent an explicit override. @@ -188,40 +195,38 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { /// is immutable, unlike a tag, which can be moved. class NativeSdkDelta { final NativeSdk sdk; - final String? currentPin; final String? targetVersion; final String? targetSha; - /// The separate pin an [NativeSdk.ios] package's `Package.swift` (SPM) - /// carries, when it has one -- always null for [NativeSdk.android]/ - /// [NativeSdk.cpp]. A podspec and a `Package.swift` pin the same - /// dependency independently, in their own file formats, so this is read - /// (and compared in [isChange]) separately from [currentPin] -- this - /// tool always pins both to the exact same version, so either one - /// drifting from [targetVersion] counts as a change. - final String? currentSpmPin; + /// Every file's current pin on this dependency -- one for [NativeSdk. + /// android] (`build.gradle`), up to two for [NativeSdk.ios] (podspec and/ + /// or `Package.swift`), and one per platform for [NativeSdk.cpp] + /// (`windows/CMakeLists.txt`, `linux/CMakeLists.txt`). Outside of a bug, + /// every pin on a dependency should already agree with every other -- + /// they're all pinned to the same target by this same tooling -- so + /// [isChange] just checks that they all still match [targetVersion] + /// rather than tracking each file's staleness independently. + final List pins; NativeSdkDelta({ required this.sdk, - required this.currentPin, required this.targetVersion, this.targetSha, - this.currentSpmPin, + this.pins = const [], }); bool get isChange { if (targetVersion == null) return false; - final podspecOutOfDate = currentPin != null && currentPin != targetVersion; - final spmOutOfDate = - currentSpmPin != null && - _spmPinnedVersion(currentSpmPin!) != targetVersion; - return podspecOutOfDate || spmOutOfDate; + return pins.any((pin) => pin.value != targetVersion); } @override - String toString() => isChange - ? '${sdk.name}: $currentPin -> $targetVersion' - : '${sdk.name}: $currentPin (no change)'; + String toString() { + final current = pins.map((pin) => '${pin.source}=${pin.value}').join(', '); + return isChange + ? '${sdk.name}: $current -> $targetVersion' + : '${sdk.name}: $current (no change)'; + } } /// The network calls native SDK resolution needs -- bundled so callers diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index 20069db43..0527e2456 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -5,6 +5,7 @@ import 'package:collection/collection.dart'; import 'package:git/git.dart'; import 'package:logging/logging.dart'; +import 'package:path/path.dart' as p; import 'package:version/version.dart'; import 'conventional_commits.dart'; @@ -151,7 +152,7 @@ Future _computePackagePlan( switch (ctx.trigger) { case TriggerContext.patch: - return await _computePatchPlan(pkg, gitDir, nativeSdkDeltas); + return await _computePatchPlan(pkg, ctx, gitDir, nativeSdkDeltas); case TriggerContext.preRelease: return await _computePrereleasePlan(pkg, ctx, gitDir, nativeSdkDeltas); case TriggerContext.mainline: @@ -168,10 +169,15 @@ Future _computePackagePlan( Future _computePatchPlan( DiscoveredPackage pkg, + RunContext ctx, GitDir gitDir, List nativeSdkDeltas, ) async { - final lastTag = await findLastReleaseTag(gitDir, pkg.name); + final lastTag = await findLastReleaseTag( + gitDir, + pkg.name, + releaseLine: _releaseLineFromPatchBranch(ctx.currentBranch), + ); final commits = await _conventionalCommitsSince( gitDir, pathspec: pkg.relativePath, @@ -212,9 +218,24 @@ Future _computePrereleasePlan( List nativeSdkDeltas, ) async { final lastTag = await findLastReleaseTag(gitDir, pkg.name); - final base = Version.parse( - lastTag != null ? _versionFromTag(lastTag, pkg.name) : pkg.version, - ); + final target = Version.parse(pkg.version); + + // A prior tag only continues the current prerelease sequence when it's + // for the exact version pubspec.yaml is declaring as the target -- e.g. + // a `4.0.0-beta.1` tag continues towards a pubspec of `4.0.0`. A tag for + // an older, already-published line (say the last stable `3.2.0`, with + // pubspec since bumped to `4.0.0` for this pre-release line) must not be + // used as the base, or the new prerelease would sort below that already- + // published release. + final tagVersion = lastTag != null + ? Version.parse(_versionFromTag(lastTag, pkg.name)) + : null; + final tagIsOnTargetLine = + tagVersion != null && + tagVersion.major == target.major && + tagVersion.minor == target.minor && + tagVersion.patch == target.patch; + final base = tagIsOnTargetLine ? tagVersion : target; final Version newVersion; if (base.isPreRelease && @@ -322,12 +343,17 @@ Future> _computeNativeSdkDeltas( final deltas = []; if (files.iosPodspec != null || files.iosSpmManifest != null) { - final currentPin = files.iosPodspec != null - ? readIosPodspecPin(files.iosPodspec!.readAsStringSync()) - : null; - final currentSpmPin = files.iosSpmManifest != null - ? readSpmPin(files.iosSpmManifest!.readAsStringSync()) - : null; + final pins = []; + if (files.iosPodspec != null) { + final pin = readIosPodspecPin(files.iosPodspec!.readAsStringSync()); + if (pin != null) pins.add((source: 'podspec', value: pin)); + } + if (files.iosSpmManifest != null) { + final pin = readSpmPin(files.iosSpmManifest!.readAsStringSync()); + if (pin != null) { + pins.add((source: 'Package.swift', value: spmPinForComparison(pin))); + } + } final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.iosSdkVersionOverride, @@ -336,19 +362,12 @@ Future> _computeNativeSdkDeltas( gateways.releaseExists(NativeSdk.ios.repoSlug, version), ); deltas.add( - NativeSdkDelta( - sdk: NativeSdk.ios, - currentPin: currentPin, - targetVersion: target, - currentSpmPin: currentSpmPin, - ), + NativeSdkDelta(sdk: NativeSdk.ios, targetVersion: target, pins: pins), ); } if (files.androidGradle != null) { - final currentPin = readAndroidGradlePin( - files.androidGradle!.readAsStringSync(), - ); + final pin = readAndroidGradlePin(files.androidGradle!.readAsStringSync()); final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.androidSdkVersionOverride, @@ -359,16 +378,25 @@ Future> _computeNativeSdkDeltas( deltas.add( NativeSdkDelta( sdk: NativeSdk.android, - currentPin: currentPin, targetVersion: target, + pins: [if (pin != null) (source: 'build.gradle', value: pin)], ), ); } if (files.cppCMakeLists.isNotEmpty) { - final currentPin = readCppCMakePin( - files.cppCMakeLists.first.readAsStringSync(), - ); + // A package can ship a CMakeLists.txt per platform (windows, linux), + // each pinning dd-sdk-cpp independently -- every one of them needs to + // be checked, not just the first, or a still-stale platform would + // silently be missed (see NativeSdkDelta.pins). + final pins = [ + for (final file in files.cppCMakeLists) + if (readCppCMakePin(file.readAsStringSync()) case final pin?) + ( + source: p.relative(file.path, from: pkg.absolutePath(ctx.repoRoot)), + value: pin, + ), + ]; final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.cppVersionOverride, @@ -385,9 +413,9 @@ Future> _computeNativeSdkDeltas( deltas.add( NativeSdkDelta( sdk: NativeSdk.cpp, - currentPin: currentPin, targetVersion: target, targetSha: targetSha, + pins: pins, ), ); } @@ -417,12 +445,26 @@ Future> _conventionalCommitsSince( .toList(); } -final _patchBranchPattern = RegExp(r'^release/([^/]+)/v\d+\.\d+\.x$'); +final _patchBranchPattern = RegExp( + r'^release/(?[^/]+)/v(?\d+)\.(?\d+)\.x$', +); /// Extracts the package name from a `release/{package}/v{major}.{minor}.x` /// patch-branch name, or null if [branch] doesn't match that convention. String? _packageNameFromPatchBranch(String branch) => - _patchBranchPattern.firstMatch(branch)?.group(1); + _patchBranchPattern.firstMatch(branch)?.namedGroup('package'); + +/// Extracts the `{major}.{minor}` release line from a +/// `release/{package}/v{major}.{minor}.x` patch-branch name. Only called +/// once [_resolveGroups] has already validated the branch matches the +/// convention, so a non-match here would be a bug in that validation. +(int major, int minor) _releaseLineFromPatchBranch(String branch) { + final match = _patchBranchPattern.firstMatch(branch)!; + return ( + int.parse(match.namedGroup('major')!), + int.parse(match.namedGroup('minor')!), + ); +} Future> _resolveGroups(RunContext ctx) async { final allGroups = await discoverPackages(ctx.repoRoot); diff --git a/tools/releaser/test/git_history_test.dart b/tools/releaser/test/git_history_test.dart index 7037b19d3..ca17b71b5 100644 --- a/tools/releaser/test/git_history_test.dart +++ b/tools/releaser/test/git_history_test.dart @@ -38,6 +38,28 @@ void main() { expect(tag, isNull); }); + test('releaseLine restricts the search to that major/minor, ignoring a ' + 'newer tag from a different line', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.0.0 work'); + await fixture.commit('fix: something for 2.0.0'); + await fixture.tag('datadog_dio/v2.0.0'); + + // Mainline has since moved on to a new major. + fixture.writeFile('packages/datadog_dio/CHANGES', 'v3.0.0 work'); + await fixture.commit('feat!: something for 3.0.0'); + await fixture.tag('datadog_dio/v3.0.0'); + + final gitDir = await fixture.gitDir; + final tag = await findLastReleaseTag( + gitDir, + 'datadog_dio', + releaseLine: (2, 0), + ); + + expect(tag, isNotNull); + expect(tag!.tag, 'datadog_dio/v2.0.0'); + }); + test( 'commitMessagesSince only returns commits after the given sha', () async { diff --git a/tools/releaser/test/native_sdk_test.dart b/tools/releaser/test/native_sdk_test.dart index b34ac431f..6d6aaa38e 100644 --- a/tools/releaser/test/native_sdk_test.dart +++ b/tools/releaser/test/native_sdk_test.dart @@ -261,7 +261,7 @@ FetchContent_Declare(dd-sdk-cpp test('is false when the target matches the current pin', () { final delta = NativeSdkDelta( sdk: NativeSdk.android, - currentPin: '3.11.0', + pins: [(source: 'build.gradle', value: '3.11.0')], targetVersion: '3.11.0', ); expect(delta.isChange, isFalse); @@ -270,7 +270,7 @@ FetchContent_Declare(dd-sdk-cpp test('is false when there is no target (no change)', () { final delta = NativeSdkDelta( sdk: NativeSdk.android, - currentPin: '3.11.0', + pins: [(source: 'build.gradle', value: '3.11.0')], targetVersion: null, ); expect(delta.isChange, isFalse); @@ -279,7 +279,7 @@ FetchContent_Declare(dd-sdk-cpp test('is true when the target differs from the current pin', () { final delta = NativeSdkDelta( sdk: NativeSdk.android, - currentPin: '3.11.0', + pins: [(source: 'build.gradle', value: '3.11.0')], targetVersion: '3.12.0', ); expect(delta.isChange, isTrue); @@ -289,8 +289,10 @@ FetchContent_Declare(dd-sdk-cpp 'both must track the same version', () { final delta = NativeSdkDelta( sdk: NativeSdk.ios, - currentPin: '3.12.0', - currentSpmPin: 'from: "3.0.0"', + pins: [ + (source: 'podspec', value: '3.12.0'), + (source: 'Package.swift', value: '3.0.0'), + ], targetVersion: '3.12.0', ); expect(delta.isChange, isTrue); @@ -299,8 +301,10 @@ FetchContent_Declare(dd-sdk-cpp test('is true when the SPM pin matches but the podspec lags behind', () { final delta = NativeSdkDelta( sdk: NativeSdk.ios, - currentPin: '~> 3', - currentSpmPin: 'exact: "3.12.0"', + pins: [ + (source: 'podspec', value: '~> 3'), + (source: 'Package.swift', value: '3.12.0'), + ], targetVersion: '3.12.0', ); expect(delta.isChange, isTrue); @@ -309,8 +313,10 @@ FetchContent_Declare(dd-sdk-cpp test('is false when both the podspec and SPM pin match the target', () { final delta = NativeSdkDelta( sdk: NativeSdk.ios, - currentPin: '3.12.0', - currentSpmPin: 'exact: "3.12.0"', + pins: [ + (source: 'podspec', value: '3.12.0'), + (source: 'Package.swift', value: '3.12.0'), + ], targetVersion: '3.12.0', ); expect(delta.isChange, isFalse); @@ -319,11 +325,49 @@ FetchContent_Declare(dd-sdk-cpp test('a branch-tracking SPM pin always counts as needing a change', () { final delta = NativeSdkDelta( sdk: NativeSdk.ios, - currentPin: '3.12.0', - currentSpmPin: 'branch: "develop"', + pins: [ + (source: 'podspec', value: '3.12.0'), + (source: 'Package.swift', value: 'branch: "develop"'), + ], targetVersion: '3.12.0', ); expect(delta.isChange, isTrue); }); + + test('is true when the first pin matches but an additional pin ' + '(e.g. a second CMakeLists) lags behind', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.cpp, + pins: [ + (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), + (source: 'linux/CMakeLists.txt', value: 'develop'), + ], + targetVersion: 'v1.4.0', + ); + expect(delta.isChange, isTrue); + }); + + test('is false when the first pin and every additional pin match', () { + final delta = NativeSdkDelta( + sdk: NativeSdk.cpp, + pins: [ + (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), + (source: 'linux/CMakeLists.txt', value: 'v1.4.0'), + ], + targetVersion: 'v1.4.0', + ); + expect(delta.isChange, isFalse); + }); + }); + + group('spmPinForComparison', () { + test('extracts the bare version literal from a version-pinned spec', () { + expect(spmPinForComparison('from: "3.0.0"'), '3.0.0'); + expect(spmPinForComparison('exact: "3.12.0"'), '3.12.0'); + }); + + test('returns a branch-tracking spec unchanged', () { + expect(spmPinForComparison('branch: "develop"'), 'branch: "develop"'); + }); }); } diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index 1052b820a..898f1b0a3 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -242,7 +242,7 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.currentPin, '~> 3'); + expect(delta.pins, [(source: 'podspec', value: '~> 3')]); expect(delta.targetVersion, '3.12.0'); expect(delta.isChange, isTrue); }); @@ -285,8 +285,10 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.currentPin, '~> 3'); - expect(delta.currentSpmPin, 'from: "3.0.0"'); + expect(delta.pins, [ + (source: 'podspec', value: '~> 3'), + (source: 'Package.swift', value: '3.0.0'), + ]); expect(delta.targetVersion, '3.12.0'); }, ); @@ -316,7 +318,9 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.currentPin, 'develop'); + expect(delta.pins, [ + (source: 'windows/CMakeLists.txt', value: 'develop'), + ]); expect(delta.targetVersion, 'v1.4.0'); expect(delta.targetSha, 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'); }); @@ -339,6 +343,44 @@ void main() { expect(delta.targetSha, isNull); }, ); + + test('a still-stale second CMakeLists (e.g. linux) is not masked by a ' + 'first one (windows) that already matches the target', () async { + // windows/CMakeLists.txt (from setUp, checked first) is rewritten + // to already match the target; linux/CMakeLists.txt is still + // pinned to "develop". + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' + 'windows/CMakeLists.txt', + ''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG deadbeefdeadbeefdeadbeefdeadbeefdeadbeef) # v1.4.0 +''', + ); + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' + 'linux/CMakeLists.txt', + _windowsCMakeListsWithGitTag, + ); + await fixture.commit('chore: update CMakeLists fixtures'); + + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_flutter_plugin_desktop'], + cppVersionOverride: 'v1.4.0', + ), + resolveCommitSha: (repoSlug, ref) async => + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.pins, [ + (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), + (source: 'linux/CMakeLists.txt', value: 'develop'), + ]); + expect(delta.isChange, isTrue); + }); }); group('patch branch', () { @@ -406,6 +448,28 @@ void main() { throwsStateError, ); }); + + test( + 'ignores a newer tag from a line mainline has already moved past', + () async { + await fixture.tag('datadog_dio/v2.0.0'); + // Mainline has since released a new major -- not an ancestor of + // the 2.0.x patch branch, and must not be picked up as "the last + // release" for it. (Tagged directly, rather than via a feat!/major + // commit, so this test isolates tag selection from the separate + // "no major/minor commits on a patch branch" check.) + fixture.writeFile('packages/datadog_dio/CHANGES', 'v3.0.0 work'); + await fixture.commit('fix: something for 3.0.0'); + await fixture.tag('datadog_dio/v3.0.0'); + + fixture.writeFile('packages/datadog_dio/CHANGES', 'a cherry-pick'); + await fixture.commit('fix: a cherry-picked fix'); + + final result = await plan(patchCtx('release/datadog_dio/v2.0.x')); + + expect(result.packages.single.newVersion, '2.0.1'); + }, + ); }); group('pre-release branch', () { @@ -453,6 +517,17 @@ void main() { expect(result.packages.single.newVersion, '4.0.0-rc.1'); }); + + test('bases a new prerelease line on the declared target version, not a ' + 'stale prior stable tag', () async { + // pubspec.version is already 4.0.0 (a new major, not yet released), + // but the last real tag is the old 3.x stable line. + await fixture.tag('datadog_flutter_plugin/v3.2.0'); + + final result = await plan(preReleaseCtx(prereleaseLabel: 'beta')); + + expect(result.packages.single.newVersion, '4.0.0-beta.1'); + }); }); test('patch branch ignores an inherited requestedPackages filter', () async { From 0cf4ae021d6fd7a8c82760f1469d66427e398c9d Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Tue, 25 Aug 2026 16:52:34 -0400 Subject: [PATCH 04/10] tools(releaser): Fix two more Codex-flagged native SDK pinning bugs. findLastReleaseTag now requires the chosen tag to be an ancestor of HEAD, so a tag cut on a line that never merged back (e.g. a long-lived prerelease branch) can't be mistaken for the real last release just because it sorts higher by version number. pinCppVersion now only rewrites a GIT_TAG line while inside the dd-sdk-cpp FetchContent_Declare block, so a second, unrelated FetchContent_Declare in the same CMakeLists.txt keeps its own GIT_TAG. --- tools/releaser/lib/cmake_util.dart | 46 +++++++++++++++++-- tools/releaser/lib/git_history.dart | 35 +++++++++++--- tools/releaser/test/cmake_util_test.dart | 30 ++++++++++-- tools/releaser/test/git_history_test.dart | 22 +++++++++ tools/releaser/test/support/fixture_repo.dart | 9 ++++ 5 files changed, 126 insertions(+), 16 deletions(-) diff --git a/tools/releaser/lib/cmake_util.dart b/tools/releaser/lib/cmake_util.dart index a8fad384c..dcfeec50a 100644 --- a/tools/releaser/lib/cmake_util.dart +++ b/tools/releaser/lib/cmake_util.dart @@ -12,6 +12,17 @@ final _gitTagLinePattern = RegExp( r'^(?\s*GIT_TAG\s+)(?[\w./-]+)(?[^#]*)(?:#.*)?$', ); +/// Matches the start of the `FetchContent_Declare(dd-sdk-cpp ...` call -- +/// used to find where that block begins, since a `CMakeLists.txt` can +/// declare more than one `FetchContent` dependency and only this one's +/// `GIT_TAG` should be rewritten. +final _ddSdkCppDeclareStartPattern = RegExp( + r'FetchContent_Declare\(\s*dd-sdk-cpp\b', +); + +int _parenBalance(String line) => + '('.allMatches(line).length - ')'.allMatches(line).length; + /// Rewrites a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line to pin at /// [targetSha] -- the resolved commit SHA for [targetTag] (e.g. `v1.4.0`), /// kept as a trailing `# ` comment since CMake's `FetchContent_Declare` @@ -24,6 +35,12 @@ final _gitTagLinePattern = RegExp( /// call is common, and the new comment is appended *after* it, since a `#` /// placed before would comment out the paren too and break the call. /// +/// Only rewrites a `GIT_TAG` line while inside the `dd-sdk-cpp` +/// `FetchContent_Declare(...)` block (tracked by paren balance across +/// lines, since the call can span several) -- a file that also vendors +/// another dependency via its own `FetchContent_Declare` must have that +/// dependency's `GIT_TAG` left untouched. +/// /// Only ever called against a release-prep/patch/pre-release branch's copy /// of the file -- `develop`'s own floating `GIT_TAG develop` is never /// touched by release tooling. @@ -39,12 +56,31 @@ Future pinCppVersion( '${cmakeListsFile.path}', ); + var insideDdSdkCppDeclare = false; + var parenDepth = 0; + await transformFile(cmakeListsFile, logger, dryRun, (line) { - final match = _gitTagLinePattern.firstMatch(line); - if (match == null) return line; + if (_ddSdkCppDeclareStartPattern.hasMatch(line)) { + insideDdSdkCppDeclare = true; + parenDepth = _parenBalance(line); + } else if (insideDdSdkCppDeclare) { + parenDepth += _parenBalance(line); + } + + var result = line; + if (insideDdSdkCppDeclare) { + final match = _gitTagLinePattern.firstMatch(line); + if (match != null) { + final prefix = match.namedGroup('prefix')!; + final trailing = (match.namedGroup('trailing') ?? '').trimRight(); + result = '$prefix$targetSha$trailing # $targetTag'; + } + } + + if (insideDdSdkCppDeclare && parenDepth <= 0) { + insideDdSdkCppDeclare = false; + } - final prefix = match.namedGroup('prefix')!; - final trailing = (match.namedGroup('trailing') ?? '').trimRight(); - return '$prefix$targetSha$trailing # $targetTag'; + return result; }); } diff --git a/tools/releaser/lib/git_history.dart b/tools/releaser/lib/git_history.dart index bb9c60fba..d090c2ec4 100644 --- a/tools/releaser/lib/git_history.dart +++ b/tools/releaser/lib/git_history.dart @@ -2,18 +2,34 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import 'package:collection/collection.dart'; import 'package:git/git.dart'; import 'package:version/version.dart'; -/// Finds the most recent tag matching `{packageName}/v*`, or null if the -/// package has never been tagged (its first release, or a brand-new -/// federated sub-package -- see [commitMessagesSince]'s no-`sinceSha` path). +/// Whether [ref] (a tag name -- passed as-is, not its peeled object sha, so +/// this works for both annotated and lightweight tags) is reachable from +/// HEAD -- a release tag cut on some other branch (a long-lived prerelease +/// line, a newer major mainline has since moved past) is not an ancestor of +/// the branch currently being planned, even though it may still sort as +/// the highest version by number alone. +Future _isAncestorOfHead(GitDir gitDir, String ref) async { + final result = await gitDir.runCommand([ + 'merge-base', + '--is-ancestor', + ref, + 'HEAD', + ], throwOnError: false); + return result.exitCode == 0; +} + +/// Finds the most recent tag matching `{packageName}/v*` that HEAD is +/// descended from, or null if the package has never been tagged on this +/// line (its first release, a brand-new federated sub-package -- see +/// [commitMessagesSince]'s no-`sinceSha` path -- or simply no ancestor tag). /// /// [releaseLine], when given, restricts the search to tags whose /// major/minor matches -- a patch branch's own release line, so a tag -/// mainline has since cut for a newer major/minor (which is not an -/// ancestor of the patch branch) can't be picked up instead. +/// mainline has since cut for a newer major/minor can't be picked up +/// instead even if it happened to be an ancestor. Future findLastReleaseTag( GitDir gitDir, String packageName, { @@ -46,7 +62,12 @@ Future findLastReleaseTag( .toList() ..sort((a, b) => a.$2!.compareTo(b.$2!)); - return withVersions.lastOrNull?.$1; + for (final pair in withVersions.reversed) { + if (await _isAncestorOfHead(gitDir, pair.$1.tag)) { + return pair.$1; + } + } + return null; } /// Full commit messages (subject + body/footers) touching [pathspec], from diff --git a/tools/releaser/test/cmake_util_test.dart b/tools/releaser/test/cmake_util_test.dart index 9b8116c56..72d95c639 100644 --- a/tools/releaser/test/cmake_util_test.dart +++ b/tools/releaser/test/cmake_util_test.dart @@ -64,21 +64,21 @@ FetchContent_Declare(dd-sdk-cpp () async { final file = File(p.join(root.path, 'CMakeLists.txt')); await file.writeAsString( + 'FetchContent_Declare(dd-sdk-cpp\n' ' GIT_TAG oldsha1234567890oldsha1234567890oldsha1) # v1.3.0\n', ); await pinCppVersion(file, 'v1.4.0', _sha, logger, false); final contents = await file.readAsString(); - // trimRight only -- trim() would also eat the leading indentation - // this test is specifically checking gets preserved. - expect(contents.trimRight(), ' GIT_TAG $_sha) # v1.4.0'); + expect(contents, contains(' GIT_TAG $_sha) # v1.4.0')); }, ); test('dry run leaves the file untouched', () async { final file = File(p.join(root.path, 'CMakeLists.txt')); - const original = ' GIT_TAG develop\n'; + const original = + 'FetchContent_Declare(dd-sdk-cpp\n GIT_TAG develop)\n'; await file.writeAsString(original); await pinCppVersion(file, 'v1.4.0', _sha, logger, true); @@ -86,6 +86,28 @@ FetchContent_Declare(dd-sdk-cpp expect(await file.readAsString(), original); }); + test('a second FetchContent_Declare for a different dependency keeps its ' + 'own GIT_TAG untouched', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString(''' +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +FetchContent_MakeAvailable(dd-sdk-cpp) + +FetchContent_Declare(some_other_dep + GIT_REPOSITORY https://github.com/example/some_other_dep.git + GIT_TAG v9.9.9) +FetchContent_MakeAvailable(some_other_dep) +'''); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + final contents = await file.readAsString(); + expect(contents, contains('GIT_TAG $_sha) # v1.4.0')); + expect(contents, contains('GIT_TAG v9.9.9)')); + }); + test('leaves everything else in the file untouched', () async { final file = File(p.join(root.path, 'CMakeLists.txt')); await file.writeAsString(''' diff --git a/tools/releaser/test/git_history_test.dart b/tools/releaser/test/git_history_test.dart index ca17b71b5..c8aafb3a7 100644 --- a/tools/releaser/test/git_history_test.dart +++ b/tools/releaser/test/git_history_test.dart @@ -60,6 +60,28 @@ void main() { expect(tag!.tag, 'datadog_dio/v2.0.0'); }); + test( + 'ignores a higher-versioned tag that is not an ancestor of HEAD -- ' + 'e.g. cut on a long-lived prerelease branch that never merged back', + () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.2.0 work'); + await fixture.commit('fix: something for 2.2.0'); + await fixture.tag('datadog_dio/v2.2.0'); + + await fixture.checkoutNewBranch('prerelease-line'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'v4.0.0-beta.1 work'); + await fixture.commit('feat!: something for 4.0.0-beta.1'); + await fixture.tag('datadog_dio/v4.0.0-beta.1'); + await fixture.checkout('main'); + + final gitDir = await fixture.gitDir; + final tag = await findLastReleaseTag(gitDir, 'datadog_dio'); + + expect(tag, isNotNull); + expect(tag!.tag, 'datadog_dio/v2.2.0'); + }, + ); + test( 'commitMessagesSince only returns commits after the given sha', () async { diff --git a/tools/releaser/test/support/fixture_repo.dart b/tools/releaser/test/support/fixture_repo.dart index d0f671f04..a3100cefe 100644 --- a/tools/releaser/test/support/fixture_repo.dart +++ b/tools/releaser/test/support/fixture_repo.dart @@ -121,6 +121,15 @@ class FixtureRepo { // "no tag message?"). Future tag(String name) => _run(['tag', '-a', '-m', name, name]); + /// Creates and switches to a new branch off the current commit -- used + /// to simulate a tag cut on a line that never merged back (a long-lived + /// prerelease branch), so it exists in the repo but isn't an ancestor of + /// `main` once [checkout] switches back. + Future checkoutNewBranch(String name) => + _run(['checkout', '-q', '-b', name]); + + Future checkout(String name) => _run(['checkout', '-q', name]); + Future get gitDir async => _gitDir ??= await GitDir.fromExisting(root.path); From 3cef8c2aa3a3695ce4d1af1c91e83e7858a43cf6 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Wed, 26 Aug 2026 15:20:43 -0400 Subject: [PATCH 05/10] tools(releaser): Fix native SDK pin comparison baseline and CMake block scoping - Compare mainline's native SDK pins against what was actually pinned at the last release tag (via git show), instead of develop's own intentionally-floating file, which made every mainline run report a spurious native SDK change. - Scope readCppCMakePin to the dd-sdk-cpp FetchContent_Declare block, sharing the paren-balance tracking with cmake_util.dart's pinCppVersion so a CMakeLists.txt with another FetchContent_Declare above dd-sdk-cpp's isn't misread. - Dedupe the patch/preRelease _computeNativeSdkDeltas call and log instead of silently dropping a pin when its file can't be read at the last release tag. --- tools/releaser/lib/cmake_util.dart | 16 +-- tools/releaser/lib/native_sdk.dart | 29 ++++- tools/releaser/lib/release_plan.dart | 120 +++++++++++++++++---- tools/releaser/test/native_sdk_test.dart | 15 +++ tools/releaser/test/release_plan_test.dart | 42 ++++++++ 5 files changed, 193 insertions(+), 29 deletions(-) diff --git a/tools/releaser/lib/cmake_util.dart b/tools/releaser/lib/cmake_util.dart index dcfeec50a..274d37c64 100644 --- a/tools/releaser/lib/cmake_util.dart +++ b/tools/releaser/lib/cmake_util.dart @@ -15,12 +15,16 @@ final _gitTagLinePattern = RegExp( /// Matches the start of the `FetchContent_Declare(dd-sdk-cpp ...` call -- /// used to find where that block begins, since a `CMakeLists.txt` can /// declare more than one `FetchContent` dependency and only this one's -/// `GIT_TAG` should be rewritten. -final _ddSdkCppDeclareStartPattern = RegExp( +/// `GIT_TAG` should be read/rewritten. Public so `native_sdk.dart`'s +/// `readCppCMakePin` scopes its read the same way this file scopes its +/// write, instead of risking the two drifting out of sync. +final ddSdkCppDeclareStartPattern = RegExp( r'FetchContent_Declare\(\s*dd-sdk-cpp\b', ); -int _parenBalance(String line) => +/// Public alongside [ddSdkCppDeclareStartPattern] for the same reason -- +/// `native_sdk.dart` needs to track the same multi-line block boundary. +int parenBalance(String line) => '('.allMatches(line).length - ')'.allMatches(line).length; /// Rewrites a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line to pin at @@ -60,11 +64,11 @@ Future pinCppVersion( var parenDepth = 0; await transformFile(cmakeListsFile, logger, dryRun, (line) { - if (_ddSdkCppDeclareStartPattern.hasMatch(line)) { + if (ddSdkCppDeclareStartPattern.hasMatch(line)) { insideDdSdkCppDeclare = true; - parenDepth = _parenBalance(line); + parenDepth = parenBalance(line); } else if (insideDdSdkCppDeclare) { - parenDepth += _parenBalance(line); + parenDepth += parenBalance(line); } var result = line; diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index 17be9b656..5e8cd3379 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -6,6 +6,7 @@ import 'dart:io'; import 'package:path/path.dart' as p; +import 'cmake_util.dart'; import 'trigger_context.dart'; /// A native SDK a Flutter package can depend on. @@ -88,11 +89,33 @@ final _spmVersionLiteralPattern = RegExp( /// comment (see [pinCppVersion] in cmake_util.dart) -- that comment is /// preferred here so the *tag* is what gets compared run-over-run, not an /// opaque SHA that would never equal a freshly-resolved target tag. +/// +/// Only reads a `GIT_TAG` line while inside the `dd-sdk-cpp` +/// `FetchContent_Declare(...)` block (tracked the same way [pinCppVersion] +/// tracks it) -- a file that also vendors another dependency via its own +/// `FetchContent_Declare` must not have that dependency's `GIT_TAG` read as +/// if it were dd-sdk-cpp's pin. String? readCppCMakePin(String cmakeListsContent) { + var insideDdSdkCppDeclare = false; + var parenDepth = 0; + for (final line in cmakeListsContent.split('\n')) { - final match = _cmakeGitTagPattern.firstMatch(line); - if (match != null) { - return match.namedGroup('comment') ?? match.namedGroup('ref'); + if (ddSdkCppDeclareStartPattern.hasMatch(line)) { + insideDdSdkCppDeclare = true; + parenDepth = parenBalance(line); + } else if (insideDdSdkCppDeclare) { + parenDepth += parenBalance(line); + } + + if (insideDdSdkCppDeclare) { + final match = _cmakeGitTagPattern.firstMatch(line); + if (match != null) { + return match.namedGroup('comment') ?? match.namedGroup('ref'); + } + } + + if (insideDdSdkCppDeclare && parenDepth <= 0) { + insideDdSdkCppDeclare = false; } } return null; diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index 0527e2456..ba777ae1f 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -2,6 +2,8 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import 'dart:io'; + import 'package:collection/collection.dart'; import 'package:git/git.dart'; import 'package:logging/logging.dart'; @@ -147,22 +149,34 @@ Future _computePackagePlan( NativeSdkGateways gateways, { required bool isExplicitlyRequested, }) async { - final nativeSdkDeltas = await _computeNativeSdkDeltas(pkg, ctx, gateways); - final hasNativeSdkChange = nativeSdkDeltas.any((d) => d.isChange); - switch (ctx.trigger) { case TriggerContext.patch: - return await _computePatchPlan(pkg, ctx, gitDir, nativeSdkDeltas); case TriggerContext.preRelease: - return await _computePrereleasePlan(pkg, ctx, gitDir, nativeSdkDeltas); + final nativeSdkDeltas = await _computeNativeSdkDeltas(pkg, ctx, gateways); + return ctx.trigger == TriggerContext.patch + ? await _computePatchPlan(pkg, ctx, gitDir, nativeSdkDeltas) + : await _computePrereleasePlan(pkg, ctx, gitDir, nativeSdkDeltas); case TriggerContext.mainline: + // Fetched up front (rather than inside _computeNativeSdkDeltas / + // _computeMainlinePlan separately) so both use the exact same tag -- + // it's also what the native SDK comparison below reads its + // "previously released" pin from. + final lastTag = await findLastReleaseTag(gitDir, pkg.name); + final nativeSdkDeltas = await _computeNativeSdkDeltas( + pkg, + ctx, + gateways, + gitDir: gitDir, + lastReleaseTag: lastTag, + ); return await _computeMainlinePlan( pkg, ctx, gitDir, nativeSdkDeltas, + lastTag: lastTag, isExplicitlyRequested: isExplicitlyRequested, - hasNativeSdkChange: hasNativeSdkChange, + hasNativeSdkChange: nativeSdkDeltas.any((d) => d.isChange), ); } } @@ -271,10 +285,10 @@ Future _computeMainlinePlan( RunContext ctx, GitDir gitDir, List nativeSdkDeltas, { + required Tag? lastTag, required bool isExplicitlyRequested, required bool hasNativeSdkChange, }) async { - final lastTag = await findLastReleaseTag(gitDir, pkg.name); final commits = await _conventionalCommitsSince( gitDir, pathspec: pkg.relativePath, @@ -337,19 +351,34 @@ String _versionFromTag(Tag tag, String packageName) => Future> _computeNativeSdkDeltas( DiscoveredPackage pkg, RunContext ctx, - NativeSdkGateways gateways, -) async { + NativeSdkGateways gateways, { + GitDir? gitDir, + Tag? lastReleaseTag, +}) async { final files = resolveNativeDependencyFiles(pkg.absolutePath(ctx.repoRoot)); final deltas = []; + Future content(File file) => _nativeDependencyPinSource( + file, + repoRoot: ctx.repoRoot, + gitDir: gitDir, + lastReleaseTag: lastReleaseTag, + ); + if (files.iosPodspec != null || files.iosSpmManifest != null) { final pins = []; if (files.iosPodspec != null) { - final pin = readIosPodspecPin(files.iosPodspec!.readAsStringSync()); + final pin = switch (await content(files.iosPodspec!)) { + final c? => readIosPodspecPin(c), + null => null, + }; if (pin != null) pins.add((source: 'podspec', value: pin)); } if (files.iosSpmManifest != null) { - final pin = readSpmPin(files.iosSpmManifest!.readAsStringSync()); + final pin = switch (await content(files.iosSpmManifest!)) { + final c? => readSpmPin(c), + null => null, + }; if (pin != null) { pins.add((source: 'Package.swift', value: spmPinForComparison(pin))); } @@ -367,7 +396,10 @@ Future> _computeNativeSdkDeltas( } if (files.androidGradle != null) { - final pin = readAndroidGradlePin(files.androidGradle!.readAsStringSync()); + final pin = switch (await content(files.androidGradle!)) { + final c? => readAndroidGradlePin(c), + null => null, + }; final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.androidSdkVersionOverride, @@ -389,14 +421,17 @@ Future> _computeNativeSdkDeltas( // each pinning dd-sdk-cpp independently -- every one of them needs to // be checked, not just the first, or a still-stale platform would // silently be missed (see NativeSdkDelta.pins). - final pins = [ - for (final file in files.cppCMakeLists) - if (readCppCMakePin(file.readAsStringSync()) case final pin?) - ( - source: p.relative(file.path, from: pkg.absolutePath(ctx.repoRoot)), - value: pin, - ), - ]; + final pins = []; + for (final file in files.cppCMakeLists) { + final fileContent = await content(file); + final pin = fileContent != null ? readCppCMakePin(fileContent) : null; + if (pin != null) { + pins.add(( + source: p.relative(file.path, from: pkg.absolutePath(ctx.repoRoot)), + value: pin, + )); + } + } final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.cppVersionOverride, @@ -423,6 +458,51 @@ Future> _computeNativeSdkDeltas( return deltas; } +/// The content to read a native dependency file's current pin from, for +/// comparison against [NativeSdkDelta.targetVersion]. +/// +/// With no [lastReleaseTag] (patch/pre-release branches, or a package's +/// first-ever mainline release), [file]'s own on-disk content already +/// reflects what was actually pinned last -- release tooling rewrites it in +/// place. On mainline, though, [file] is `develop`'s own copy, which is +/// deliberately left on a floating constraint (`~> 3`, `branch: "develop"`, +/// `GIT_TAG develop`) by that same tooling -- comparing the resolved target +/// against a floating constraint would report a native SDK change on every +/// run. What was actually shipped is whatever got pinned into this same +/// file right before [lastReleaseTag] was cut, so read that historical +/// blob instead. Returns null (nothing to compare) if the file didn't +/// exist yet at that tag. +Future _nativeDependencyPinSource( + File file, { + required String repoRoot, + GitDir? gitDir, + Tag? lastReleaseTag, +}) async { + if (gitDir == null || lastReleaseTag == null) { + return file.readAsStringSync(); + } + final relativePath = p.posix.joinAll( + p.split(p.relative(file.path, from: repoRoot)), + ); + final result = await gitDir.runCommand([ + 'show', + '${lastReleaseTag.tag}:$relativePath', + ], throwOnError: false); + if (result.exitCode != 0) { + // Usually means the file didn't exist yet at lastReleaseTag, but it's + // equally what a moved/renamed file looks like -- log so a real stale + // pin hiding behind a rename isn't dropped completely silently. + Logger('native_sdk').warning( + "⚠️ Couldn't read $relativePath as of ${lastReleaseTag.tag} " + '(git show exit ${result.exitCode}); skipping it for native SDK ' + 'comparison. If this file moved since that release, the comparison ' + 'may miss a stale pin.', + ); + return null; + } + return result.stdout as String; +} + /// [commitMessagesSince]'s raw messages, parsed into [ConventionalCommit]s /// and narrowed to the ones that carry semver weight -- commits that fail /// to parse, or parse but don't bump anything (`chore:`, `docs:`, etc.), diff --git a/tools/releaser/test/native_sdk_test.dart b/tools/releaser/test/native_sdk_test.dart index 6d6aaa38e..edce2bb78 100644 --- a/tools/releaser/test/native_sdk_test.dart +++ b/tools/releaser/test/native_sdk_test.dart @@ -89,6 +89,21 @@ FetchContent_Declare(dd-sdk-cpp expect(readCppCMakePin('FetchContent_Declare(something_else)'), isNull); }); + test('readCppCMakePin ignores a GIT_TAG belonging to another ' + 'FetchContent_Declare that appears before dd-sdk-cpp', () { + const content = ''' +FetchContent_Declare(some_other_dep + GIT_REPOSITORY https://github.com/example/some_other_dep.git + GIT_TAG v9.9.9) +FetchContent_MakeAvailable(some_other_dep) + +FetchContent_Declare(dd-sdk-cpp + GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git + GIT_TAG develop) +'''; + expect(readCppCMakePin(content), 'develop'); + }); + test('readSpmPin finds the dd-sdk-ios dependency spec', () { expect(readSpmPin(_packageSwift), 'from: "3.0.0"'); }); diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index 898f1b0a3..eb3b91ad8 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -267,6 +267,48 @@ void main() { expect(result.packages.single.nativeSdkDeltas, isEmpty); }); + test('isChange compares against the pin from the last release tag, not ' + "develop's intentionally-floating current pin", () async { + // Simulate what release tooling actually pins into the file right + // before cutting a release. + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + "Pod::Spec.new do |s|\n s.dependency 'DatadogCore', '3.10.0'\nend\n", + ); + await fixture.commit('chore: release datadog_flutter_plugin_ios 1.0.0'); + await fixture.tag('datadog_flutter_plugin_ios/v1.0.0'); + + // develop moves on, reverting back to its usual floating pin -- + // this must not be mistaken for a native SDK change. + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + await fixture.commit('chore: back to floating on develop'); + + final unchanged = await plan( + mainlineCtx(), + fetchLatestNativeSdkVersion: (_) async => '3.10.0', + ); + expect( + unchanged.packages.map((p) => p.package.name), + isNot(contains('datadog_flutter_plugin_ios')), + ); + + final changed = await plan( + mainlineCtx(), + fetchLatestNativeSdkVersion: (_) async => '3.13.0', + ); + final delta = changed.packages + .firstWhere((p) => p.package.name == 'datadog_flutter_plugin_ios') + .nativeSdkDeltas + .single; + expect(delta.pins, [(source: 'podspec', value: '3.10.0')]); + expect(delta.isChange, isTrue); + }); + test( 'a Package.swift alongside the podspec surfaces its own current pin', () async { From f805e51f982e142aed26f738d35f89bbcf338c38 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Wed, 26 Aug 2026 15:41:10 -0400 Subject: [PATCH 06/10] tools(releaser): Fix mainline prerelease promotion, missing-pin, and release-list paging bugs - Mainline now promotes a merged prerelease tag to the stable version it was leading up to, instead of bumping past it (e.g. 4.0.0-beta.5 -> 4.0.1, skipping the intended 4.0.0 release). - A native dependency manifest with no historical pin to compare against (added or renamed since the last release) now counts as a native SDK change instead of being silently dropped from consideration. - Starting a new pre-release for a target version that's already been released stably now fails loudly instead of producing a version that sorts below the published release. - gh release list now passes --limit so an explicit SDK version override against an older release isn't rejected just for falling past the CLI's default page of 30. --- tools/releaser/lib/github_cmd_wrapper.dart | 5 ++ tools/releaser/lib/native_sdk.dart | 11 ++- tools/releaser/lib/release_plan.dart | 85 ++++++++++++++++------ tools/releaser/test/release_plan_test.dart | 52 +++++++++++++ 4 files changed, 129 insertions(+), 24 deletions(-) diff --git a/tools/releaser/lib/github_cmd_wrapper.dart b/tools/releaser/lib/github_cmd_wrapper.dart index 7ef472e1d..220164def 100644 --- a/tools/releaser/lib/github_cmd_wrapper.dart +++ b/tools/releaser/lib/github_cmd_wrapper.dart @@ -58,6 +58,11 @@ class GithubCommandWrapper { repoSlug, '--json', 'name,isLatest,tagName', + // `gh release list` defaults to a page of 30 -- without this, a repo + // with more releases than that silently drops older ones, making + // getReleaseByTagName reject a perfectly valid older override. + '--limit', + '1000', ], workingDirectory: cwd, stdout: (line) => buffer.write(line), diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index 5e8cd3379..0240dd778 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -231,16 +231,25 @@ class NativeSdkDelta { /// rather than tracking each file's staleness independently. final List pins; + /// True when a dependency file this package currently ships couldn't be + /// read at [lastReleaseTag] on `release_plan.dart`'s mainline path -- + /// most commonly because the file is new (added since that release) or + /// was renamed/moved, so it has no historical pin to compare against. + /// Treated as a change: silently treating "no historical pin" as "no + /// change" would leave a newly-added or renamed manifest floating. + final bool hasUnknownPin; + NativeSdkDelta({ required this.sdk, required this.targetVersion, this.targetSha, this.pins = const [], + this.hasUnknownPin = false, }); bool get isChange { if (targetVersion == null) return false; - return pins.any((pin) => pin.value != targetVersion); + return hasUnknownPin || pins.any((pin) => pin.value != targetVersion); } @override diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index ba777ae1f..e92ae1aac 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -256,6 +256,16 @@ Future _computePrereleasePlan( (ctx.prereleaseLabel == null || base.preRelease.first == ctx.prereleaseLabel)) { newVersion = base.incrementPreRelease(); + } else if (tagIsOnTargetLine && !tagVersion.isPreRelease) { + // [lastTag] is a *stable* tag for this exact target version (e.g. the + // target line was already released as 4.0.0 and pubspec.yaml hasn't + // been bumped since) -- a new pre-release here would sort below that + // already-published release (`4.0.0-beta.1` < `4.0.0`). + throw StateError( + 'Package "${pkg.name}" version $target has already been released ' + 'stably as ${lastTag!.tag} -- bump the version in pubspec.yaml ' + 'before starting a new pre-release line.', + ); } else if (ctx.prereleaseLabel != null) { newVersion = Version( base.major, @@ -310,12 +320,26 @@ Future _computeMainlinePlan( bump = lastTag == null ? null : VersionBumpType.patch; } - final newVersion = lastTag == null - ? pkg.version - : _applyBump( - Version.parse(_versionFromTag(lastTag, pkg.name)), - bump!, - ).toString(); + final tagVersion = lastTag != null + ? Version.parse(_versionFromTag(lastTag, pkg.name)) + : null; + + // A prerelease tag (e.g. `4.0.0-beta.5`) becomes an ancestor of HEAD once + // its branch merges back into mainline, so it can be selected as + // [lastTag] here. Applying a bump on top of it (e.g. incrementPatch -> + // `4.0.1`) would skip the stable release the prerelease line was leading + // up to entirely. Promote it instead: this release publishes that same + // `major.minor.patch` stably, dropping the prerelease suffix, rather than + // bumping past it. + final newVersion = switch (tagVersion) { + null => pkg.version, + final v when v.isPreRelease => Version( + v.major, + v.minor, + v.patch, + ).toString(), + final v => _applyBump(v, bump!).toString(), + }; return PackagePlan( package: pkg, @@ -367,20 +391,25 @@ Future> _computeNativeSdkDeltas( if (files.iosPodspec != null || files.iosSpmManifest != null) { final pins = []; + var hasUnknownPin = false; if (files.iosPodspec != null) { - final pin = switch (await content(files.iosPodspec!)) { - final c? => readIosPodspecPin(c), - null => null, - }; - if (pin != null) pins.add((source: 'podspec', value: pin)); + final fileContent = await content(files.iosPodspec!); + if (fileContent == null) { + hasUnknownPin = true; + } else { + final pin = readIosPodspecPin(fileContent); + if (pin != null) pins.add((source: 'podspec', value: pin)); + } } if (files.iosSpmManifest != null) { - final pin = switch (await content(files.iosSpmManifest!)) { - final c? => readSpmPin(c), - null => null, - }; - if (pin != null) { - pins.add((source: 'Package.swift', value: spmPinForComparison(pin))); + final fileContent = await content(files.iosSpmManifest!); + if (fileContent == null) { + hasUnknownPin = true; + } else { + final pin = readSpmPin(fileContent); + if (pin != null) { + pins.add((source: 'Package.swift', value: spmPinForComparison(pin))); + } } } final target = await resolveNativeSdkTarget( @@ -391,15 +420,18 @@ Future> _computeNativeSdkDeltas( gateways.releaseExists(NativeSdk.ios.repoSlug, version), ); deltas.add( - NativeSdkDelta(sdk: NativeSdk.ios, targetVersion: target, pins: pins), + NativeSdkDelta( + sdk: NativeSdk.ios, + targetVersion: target, + pins: pins, + hasUnknownPin: hasUnknownPin, + ), ); } if (files.androidGradle != null) { - final pin = switch (await content(files.androidGradle!)) { - final c? => readAndroidGradlePin(c), - null => null, - }; + final fileContent = await content(files.androidGradle!); + final pin = fileContent != null ? readAndroidGradlePin(fileContent) : null; final target = await resolveNativeSdkTarget( trigger: ctx.trigger, override: ctx.androidSdkVersionOverride, @@ -412,6 +444,7 @@ Future> _computeNativeSdkDeltas( sdk: NativeSdk.android, targetVersion: target, pins: [if (pin != null) (source: 'build.gradle', value: pin)], + hasUnknownPin: fileContent == null, ), ); } @@ -422,9 +455,14 @@ Future> _computeNativeSdkDeltas( // be checked, not just the first, or a still-stale platform would // silently be missed (see NativeSdkDelta.pins). final pins = []; + var hasUnknownPin = false; for (final file in files.cppCMakeLists) { final fileContent = await content(file); - final pin = fileContent != null ? readCppCMakePin(fileContent) : null; + if (fileContent == null) { + hasUnknownPin = true; + continue; + } + final pin = readCppCMakePin(fileContent); if (pin != null) { pins.add(( source: p.relative(file.path, from: pkg.absolutePath(ctx.repoRoot)), @@ -451,6 +489,7 @@ Future> _computeNativeSdkDeltas( targetVersion: target, targetSha: targetSha, pins: pins, + hasUnknownPin: hasUnknownPin, ), ); } diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index eb3b91ad8..845f58c39 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -219,6 +219,20 @@ void main() { expect(result.packages.single.bumpLevel, VersionBumpType.patch); expect(result.packages.single.newVersion, '1.1.1'); }); + + test('a merged prerelease tag is promoted to the stable version it was ' + 'leading up to, rather than bumped past it', () async { + // Simulates a long-lived prerelease branch (`v4`) merging back into + // mainline -- its beta tag becomes an ancestor of HEAD and would + // otherwise be picked as lastTag and bumped from, skipping 4.0.0. + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.3'); + + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_flutter_plugin']), + ); + + expect(result.packages.single.newVersion, '4.0.0'); + }); }); group('mainline, native SDK deltas', () { @@ -425,6 +439,32 @@ FetchContent_Declare(dd-sdk-cpp }); }); + group('mainline, native SDK deltas with a manifest added after the last ' + 'release', () { + test('a manifest with no historical pin to compare against is treated as ' + 'a change rather than silently skipped', () async { + // Released with no podspec at all -- nothing to compare against. + await fixture.tag('datadog_flutter_plugin_ios/v1.0.0'); + + // The podspec is only added afterwards. + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + await fixture.commit('chore: add podspec fixture after the release'); + + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_flutter_plugin_ios']), + fetchLatestNativeSdkVersion: (_) async => '3.13.0', + ); + + final delta = result.packages.single.nativeSdkDeltas.single; + expect(delta.pins, isEmpty); + expect(delta.isChange, isTrue); + }); + }); + group('patch branch', () { RunContext patchCtx(String branch) => RunContext( repoRoot: fixture.root.path, @@ -570,6 +610,18 @@ FetchContent_Declare(dd-sdk-cpp expect(result.packages.single.newVersion, '4.0.0-beta.1'); }); + + test('rejects starting a new pre-release once the target version has ' + 'already been released stably', () async { + // pubspec.version is 4.0.0, and it's already been released stably + // as such -- a new "4.0.0-beta.1" would sort below that release. + await fixture.tag('datadog_flutter_plugin/v4.0.0'); + + await expectLater( + plan(preReleaseCtx(prereleaseLabel: 'beta')), + throwsStateError, + ); + }); }); test('patch branch ignores an inherited requestedPackages filter', () async { From ea37d0af849f231716ea19a8e92468e4914a6cca Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Wed, 26 Aug 2026 17:26:49 -0400 Subject: [PATCH 07/10] tools(releaser): Replace native SDK pin comparison with target resolution The native SDK check grew a current-pin-vs-target comparison that isn't in the design doc, where step 1c only *determines* the target version. That comparison is unanswerable on `develop` by construction -- develop keeps its manifests on floating constraints by design and only the release-prep branch is ever pinned -- so each review round patched around it with another concept: per-file pins, then reading the pin out of the last release tag's blob, then hasUnknownPin for when that blob is missing, then again for when it exists without a pin. Drop the comparison instead. - NativeSdkDelta is now {sdk, targetVersion, targetSha, files}: a target plus the files to rewrite. Deletes pins/NativeSdkPin, hasUnknownPin, isChange, spmPinForComparison, the four read*Pin helpers, and the historical-blob read. The shared regexes stay -- discovery and the pin-writing utils still share one definition of each line shape. - Eligibility follows the design doc: qualifying commits, explicitly requested, or an explicit native SDK override for an SDK the package actually ships a manifest for (so IOS_SDK_VERSION can't sweep pure-Dart packages into an --all run). Decided before any network call, which also means the three plan functions are synchronous and non-nullable. findLastReleaseTag no longer filters by reachability from HEAD. Release tags land on `release/...` branches that are never merged back, so no release tag is an ancestor of `develop` or of a pre-release branch like `v4` -- the ancestor check rejected every real release and fell back to ancient prerelease tags (datadog_flutter_plugin computed 3.6.0 -> 2.0.0, a downgrade, from a 140-commit range). The concern it addressed -- a 4.0.0-beta.3 tag off v4 outranking mainline's last stable -- is handled by version shape instead: mainline takes the highest stable tag, patch is constrained by releaseLine, pre-release wants those betas and constrains neither. That also makes the prerelease-promotion branch unreachable, since mainline can no longer select a prerelease tag; the natural path (last stable + aggregated bump) reaches the same 4.0.0. Also fixed, all found while removing the above: - CMakeLists discovery and rewriting disagreed about scope -- discovery matched any GIT_TAG in windows/linux CMakeLists.txt while pinCppVersion was scoped to the dd-sdk-cpp FetchContent_Declare block. Both now share one block scanner. - getReleaseByTagName scanned a paged `gh release list` (30 by default, so a valid older override could be rejected for falling off the page). Replaced with releaseExists hitting repos/{slug}/releases/tags/{tag} directly. - release_validator caught a bare StateError and reported "Could not find target release" for it, mislabeling any unrelated failure. It now checks releaseExists directly rather than routing through a throwing helper. - _computePrereleasePlan left contributingCommits empty, which would have given the changelog step no input on a pre-release ship. Co-Authored-By: Claude Opus 5 (1M context) --- tools/releaser/lib/cmake_util.dart | 91 +++-- tools/releaser/lib/generate_changelog.dart | 2 +- tools/releaser/lib/git_history.dart | 77 ++-- tools/releaser/lib/github_cmd_wrapper.dart | 27 +- tools/releaser/lib/native_sdk.dart | 156 ++------ tools/releaser/lib/release_plan.dart | 413 +++++++-------------- tools/releaser/lib/release_validator.dart | 39 +- tools/releaser/test/git_history_test.dart | 29 +- tools/releaser/test/native_sdk_test.dart | 197 +--------- tools/releaser/test/release_plan_test.dart | 195 +++++----- 10 files changed, 415 insertions(+), 811 deletions(-) diff --git a/tools/releaser/lib/cmake_util.dart b/tools/releaser/lib/cmake_util.dart index 274d37c64..38127772c 100644 --- a/tools/releaser/lib/cmake_util.dart +++ b/tools/releaser/lib/cmake_util.dart @@ -12,20 +12,54 @@ final _gitTagLinePattern = RegExp( r'^(?\s*GIT_TAG\s+)(?[\w./-]+)(?[^#]*)(?:#.*)?$', ); -/// Matches the start of the `FetchContent_Declare(dd-sdk-cpp ...` call -- -/// used to find where that block begins, since a `CMakeLists.txt` can -/// declare more than one `FetchContent` dependency and only this one's -/// `GIT_TAG` should be read/rewritten. Public so `native_sdk.dart`'s -/// `readCppCMakePin` scopes its read the same way this file scopes its -/// write, instead of risking the two drifting out of sync. -final ddSdkCppDeclareStartPattern = RegExp( +final _ddSdkCppDeclareStartPattern = RegExp( r'FetchContent_Declare\(\s*dd-sdk-cpp\b', ); -/// Public alongside [ddSdkCppDeclareStartPattern] for the same reason -- -/// `native_sdk.dart` needs to track the same multi-line block boundary. -int parenBalance(String line) => - '('.allMatches(line).length - ')'.allMatches(line).length; +/// Tracks whether a line belongs to the `FetchContent_Declare(dd-sdk-cpp ...)` +/// call, which can span several lines -- a `CMakeLists.txt` is free to declare +/// other `FetchContent` dependencies, and only dd-sdk-cpp's `GIT_TAG` is this +/// tooling's to read or rewrite. +/// +/// Shared by [hasDdSdkCppGitTag] (which decides whether a file counts as a +/// dd-sdk-cpp dependency file at all) and [pinCppVersion] (which rewrites it), +/// so the two can't disagree about where the block starts and ends. +class _DdSdkCppBlockScanner { + var _inBlock = false; + var _parenDepth = 0; + + static int _parenBalance(String line) => + '('.allMatches(line).length - ')'.allMatches(line).length; + + /// Advances the scanner over [line] and returns whether that line is inside + /// the block -- the opening `FetchContent_Declare(dd-sdk-cpp` line included, + /// since a single-line declaration carries its `GIT_TAG` there. + bool accept(String line) { + if (_ddSdkCppDeclareStartPattern.hasMatch(line)) { + _inBlock = true; + _parenDepth = _parenBalance(line); + } else if (_inBlock) { + _parenDepth += _parenBalance(line); + } + + final result = _inBlock; + if (_inBlock && _parenDepth <= 0) _inBlock = false; + return result; + } +} + +/// Whether [cmakeListsContent] pins dd-sdk-cpp -- a `GIT_TAG` line inside a +/// `FetchContent_Declare(dd-sdk-cpp ...)` call. This is what makes a +/// `windows/`/`linux/` `CMakeLists.txt` a native-dependency file as far as +/// `native_sdk.dart`'s discovery is concerned; a file whose only `GIT_TAG` +/// belongs to some other vendored dependency isn't one. +bool hasDdSdkCppGitTag(String cmakeListsContent) { + final scanner = _DdSdkCppBlockScanner(); + for (final line in cmakeListsContent.split('\n')) { + if (scanner.accept(line) && _gitTagLinePattern.hasMatch(line)) return true; + } + return false; +} /// Rewrites a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line to pin at /// [targetSha] -- the resolved commit SHA for [targetTag] (e.g. `v1.4.0`), @@ -39,12 +73,6 @@ int parenBalance(String line) => /// call is common, and the new comment is appended *after* it, since a `#` /// placed before would comment out the paren too and break the call. /// -/// Only rewrites a `GIT_TAG` line while inside the `dd-sdk-cpp` -/// `FetchContent_Declare(...)` block (tracked by paren balance across -/// lines, since the call can span several) -- a file that also vendors -/// another dependency via its own `FetchContent_Declare` must have that -/// dependency's `GIT_TAG` left untouched. -/// /// Only ever called against a release-prep/patch/pre-release branch's copy /// of the file -- `develop`'s own floating `GIT_TAG develop` is never /// touched by release tooling. @@ -60,31 +88,16 @@ Future pinCppVersion( '${cmakeListsFile.path}', ); - var insideDdSdkCppDeclare = false; - var parenDepth = 0; + final scanner = _DdSdkCppBlockScanner(); await transformFile(cmakeListsFile, logger, dryRun, (line) { - if (ddSdkCppDeclareStartPattern.hasMatch(line)) { - insideDdSdkCppDeclare = true; - parenDepth = parenBalance(line); - } else if (insideDdSdkCppDeclare) { - parenDepth += parenBalance(line); - } + if (!scanner.accept(line)) return line; - var result = line; - if (insideDdSdkCppDeclare) { - final match = _gitTagLinePattern.firstMatch(line); - if (match != null) { - final prefix = match.namedGroup('prefix')!; - final trailing = (match.namedGroup('trailing') ?? '').trimRight(); - result = '$prefix$targetSha$trailing # $targetTag'; - } - } + final match = _gitTagLinePattern.firstMatch(line); + if (match == null) return line; - if (insideDdSdkCppDeclare && parenDepth <= 0) { - insideDdSdkCppDeclare = false; - } - - return result; + final prefix = match.namedGroup('prefix')!; + final trailing = (match.namedGroup('trailing') ?? '').trimRight(); + return '$prefix$targetSha$trailing # $targetTag'; }); } diff --git a/tools/releaser/lib/generate_changelog.dart b/tools/releaser/lib/generate_changelog.dart index 6c5096913..92ea6aa51 100644 --- a/tools/releaser/lib/generate_changelog.dart +++ b/tools/releaser/lib/generate_changelog.dart @@ -48,7 +48,7 @@ class GenerateChangelogCommand extends Command { final commits = await commitMessagesSince( args.gitDir, pathspec: getPackageRoot(args, package), - sinceSha: lastReleaseTag.objectSha, + sinceSha: lastReleaseTag.tag.objectSha, ); final changelogItems = _getChangelogItems(commits); diff --git a/tools/releaser/lib/git_history.dart b/tools/releaser/lib/git_history.dart index d090c2ec4..f45fc3e9d 100644 --- a/tools/releaser/lib/git_history.dart +++ b/tools/releaser/lib/git_history.dart @@ -2,44 +2,42 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import 'package:collection/collection.dart'; import 'package:git/git.dart'; import 'package:version/version.dart'; -/// Whether [ref] (a tag name -- passed as-is, not its peeled object sha, so -/// this works for both annotated and lightweight tags) is reachable from -/// HEAD -- a release tag cut on some other branch (a long-lived prerelease -/// line, a newer major mainline has since moved past) is not an ancestor of -/// the branch currently being planned, even though it may still sort as -/// the highest version by number alone. -Future _isAncestorOfHead(GitDir gitDir, String ref) async { - final result = await gitDir.runCommand([ - 'merge-base', - '--is-ancestor', - ref, - 'HEAD', - ], throwOnError: false); - return result.exitCode == 0; -} +/// A package's release tag paired with the version parsed out of its name -- +/// returned together so callers don't re-parse `{package}/v{version}` (and +/// can't disagree with [findLastReleaseTag] about how to). +typedef ReleaseTag = ({Tag tag, Version version}); -/// Finds the most recent tag matching `{packageName}/v*` that HEAD is -/// descended from, or null if the package has never been tagged on this -/// line (its first release, a brand-new federated sub-package -- see -/// [commitMessagesSince]'s no-`sinceSha` path -- or simply no ancestor tag). +/// The highest-versioned tag matching `{packageName}/v*`, or null if the +/// package has never been tagged (its first release, or a brand-new federated +/// sub-package -- see [commitMessagesSince]'s no-`sinceSha` path). +/// +/// Deliberately *not* filtered by reachability from HEAD. Release tags in this +/// repo land on `release/...` branches that are never merged back, so no +/// release tag is an ancestor of `develop` or of a pre-release branch like +/// `v4` -- an ancestor check would reject every real release and fall back to +/// ancient tags. The two callers that need to exclude something exclude it by +/// what the tag *is*, not by where it sits in the graph: +/// +/// [releaseLine], when given, restricts the search to tags whose major/minor +/// matches -- a patch branch's own release line, so a tag mainline has since +/// cut for a newer major/minor can't be picked up instead. /// -/// [releaseLine], when given, restricts the search to tags whose -/// major/minor matches -- a patch branch's own release line, so a tag -/// mainline has since cut for a newer major/minor can't be picked up -/// instead even if it happened to be an ancestor. -Future findLastReleaseTag( +/// [stableOnly] drops pre-release versions. Mainline passes this: a +/// `4.0.0-beta.3` tag cut off a long-lived pre-release line sorts above the +/// last stable `3.5.0` but doesn't represent anything shipped stably, so it +/// must not become mainline's baseline. The pre-release path itself wants the +/// opposite (its whole job is continuing that counter) and leaves it false. +Future findLastReleaseTag( GitDir gitDir, String packageName, { (int major, int minor)? releaseLine, + bool stableOnly = false, }) async { final prefix = '$packageName/v'; - final matchingTags = await gitDir - .tags() - .where((t) => t.tag.startsWith(prefix)) - .toList(); Version? versionOf(Tag tag) { try { @@ -49,25 +47,22 @@ Future findLastReleaseTag( } } - final withVersions = - matchingTags - .map((tag) => (tag, versionOf(tag))) - .where((pair) => pair.$2 != null) + final candidates = + (await gitDir.tags().where((t) => t.tag.startsWith(prefix)).toList()) + .map((tag) => (tag: tag, version: versionOf(tag))) + .where((pair) => pair.version != null) + .map((pair) => (tag: pair.tag, version: pair.version!)) + .where((pair) => !stableOnly || !pair.version.isPreRelease) .where( (pair) => releaseLine == null || - (pair.$2!.major == releaseLine.$1 && - pair.$2!.minor == releaseLine.$2), + (pair.version.major == releaseLine.$1 && + pair.version.minor == releaseLine.$2), ) .toList() - ..sort((a, b) => a.$2!.compareTo(b.$2!)); + ..sort((a, b) => a.version.compareTo(b.version)); - for (final pair in withVersions.reversed) { - if (await _isAncestorOfHead(gitDir, pair.$1.tag)) { - return pair.$1; - } - } - return null; + return candidates.lastOrNull; } /// Full commit messages (subject + body/footers) touching [pathspec], from diff --git a/tools/releaser/lib/github_cmd_wrapper.dart b/tools/releaser/lib/github_cmd_wrapper.dart index 220164def..8a2f598e3 100644 --- a/tools/releaser/lib/github_cmd_wrapper.dart +++ b/tools/releaser/lib/github_cmd_wrapper.dart @@ -4,7 +4,6 @@ import 'dart:convert'; -import 'package:collection/collection.dart'; import 'package:json_annotation/json_annotation.dart'; import 'package:logging/logging.dart'; @@ -58,11 +57,6 @@ class GithubCommandWrapper { repoSlug, '--json', 'name,isLatest,tagName', - // `gh release list` defaults to a page of 30 -- without this, a repo - // with more releases than that silently drops older ones, making - // getReleaseByTagName reject a perfectly valid older override. - '--limit', - '1000', ], workingDirectory: cwd, stdout: (line) => buffer.write(line), @@ -83,13 +77,28 @@ class GithubCommandWrapper { return releases.firstWhere((e) => e.isLatest); } - Future getReleaseByTagName( + /// Whether [repoSlug] has a published release tagged [tagName]. + /// + /// Hits the by-tag endpoint directly rather than scanning [fetchReleases] -- + /// `gh release list` is paged (30 by default), so a repo with a long release + /// history would silently drop older tags and make a perfectly valid version + /// override look nonexistent. + Future releaseExists( Logger logger, String repoSlug, String tagName, ) async { - final releases = await fetchReleases(logger, repoSlug); - return releases.firstWhereOrNull((e) => e.tagName == tagName); + final exitCode = await runProcess( + 'gh', + ['api', 'repos/$repoSlug/releases/tags/$tagName', '--silent'], + workingDirectory: cwd, + stdout: (_) {}, + // A 404 here is the expected "no such release" answer, not a failure + // worth shouting about -- the non-zero exit is what reports it. + stderr: (line) => logger.fine(line), + ); + + return exitCode == 0; } /// Resolves [ref] (a tag or branch name) to the full commit SHA it diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index 0240dd778..fd73f27ce 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -21,9 +21,9 @@ enum NativeSdk { } /// Matches a podspec's `s.dependency 'Datadog...', ''` lines -- -/// public so `cocoapod_util.dart`'s pin-rewriting step uses the exact same -/// pattern this file reads the current pin with, instead of a second, -/// separately-maintained regex for the same line shape. +/// public so `cocoapod_util.dart`'s pin-rewriting step and this file's +/// discovery share one definition of the line shape instead of maintaining +/// two separate regexes for it. final iosPodspecDependencyPattern = RegExp( r"s\.dependency\s+'(?Datadog\w*)'\s*,\s*'(?[^']+)'", ); @@ -32,6 +32,9 @@ final iosPodspecDependencyPattern = RegExp( /// the pattern built from it -- both public so `gradle_util.dart`'s /// pin-rewriting step shares this file's exact definition of the line /// rather than maintaining its own copy. +/// +/// Discovery below uses the same pattern to decide whether a `build.gradle` +/// is an Android native-dependency file at all. const androidGradleVersionPrefix = 'ext.datadog_version'; final androidGradleVersionPattern = RegExp( '$androidGradleVersionPrefix\\s*=\\s*"(?[^"]+)"', @@ -39,87 +42,11 @@ final androidGradleVersionPattern = RegExp( /// Matches a `Package.swift`'s dd-sdk-ios dependency line (e.g. /// `.package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: -/// "3.0.0")`) -- public so `spm_util.dart`'s pin-rewriting step uses the -/// exact same pattern this file reads the current pin with. +/// "3.0.0")`) -- public so `spm_util.dart`'s pin-rewriting step and this +/// file's discovery share one definition of it. final iosSpmDependencyPattern = RegExp( r'\.package\(url:\s*"(?[^"]*dd-sdk-ios[^"]*)",\s*(?[^)]+)\)', ); -final _cmakeGitTagPattern = RegExp( - r'^\s*GIT_TAG\s+(?[\w./-]+).*?(?:#\s*(?\S+))?$', - multiLine: true, -); - -/// The current iOS pin from a podspec's `s.dependency 'Datadog...'` lines -/// (they all share one constraint), or null if it has none. -String? readIosPodspecPin(String podspecContent) => iosPodspecDependencyPattern - .firstMatch(podspecContent) - ?.namedGroup('constraint'); - -/// The current Android pin from a `build.gradle`'s `ext.datadog_version`, -/// or null if it has none. -String? readAndroidGradlePin(String buildGradleContent) => - androidGradleVersionPattern - .firstMatch(buildGradleContent) - ?.namedGroup('version'); - -/// The current iOS pin from a `Package.swift`'s dd-sdk-ios dependency spec -/// (e.g. `from: "3.0.0"`, `exact: "3.5.0"`, `branch: "develop"`), or null if -/// it has none. Kept separate from [readIosPodspecPin] since a package can -/// carry both a podspec (CocoaPods) and a `Package.swift` (SPM) pinning the -/// same dependency in independently-formatted ways. -String? readSpmPin(String packageSwiftContent) => - iosSpmDependencyPattern.firstMatch(packageSwiftContent)?.namedGroup('spec'); - -/// Pulls the bare version literal out of an SPM dependency spec whose kind -/// pins to a specific version (`exact: "3.12.0"` / `from: "3.0.0"` -> -/// `3.12.0`/`3.0.0`). A spec with nothing to compare (`branch: "develop"`) -/// is returned unchanged, so it never accidentally equals a resolved -/// target and always reads as needing a pin -- used to normalize a -/// `Package.swift` pin into the same shape as [readIosPodspecPin]'s before -/// the two are compared as just another entry in [NativeSdkDelta.pins]. -String spmPinForComparison(String spec) => - _spmVersionLiteralPattern.firstMatch(spec)?.namedGroup('version') ?? spec; -final _spmVersionLiteralPattern = RegExp( - r'^(?:exact|from|upToNextMajor|upToNextMinor):\s*"(?[^"]+)"', -); - -/// The current C++ pin from a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line, or -/// null if it has none. Once pinned by this tooling, `GIT_TAG` holds a -/// commit SHA with the human-meaningful tag kept as a trailing `# ` -/// comment (see [pinCppVersion] in cmake_util.dart) -- that comment is -/// preferred here so the *tag* is what gets compared run-over-run, not an -/// opaque SHA that would never equal a freshly-resolved target tag. -/// -/// Only reads a `GIT_TAG` line while inside the `dd-sdk-cpp` -/// `FetchContent_Declare(...)` block (tracked the same way [pinCppVersion] -/// tracks it) -- a file that also vendors another dependency via its own -/// `FetchContent_Declare` must not have that dependency's `GIT_TAG` read as -/// if it were dd-sdk-cpp's pin. -String? readCppCMakePin(String cmakeListsContent) { - var insideDdSdkCppDeclare = false; - var parenDepth = 0; - - for (final line in cmakeListsContent.split('\n')) { - if (ddSdkCppDeclareStartPattern.hasMatch(line)) { - insideDdSdkCppDeclare = true; - parenDepth = parenBalance(line); - } else if (insideDdSdkCppDeclare) { - parenDepth += parenBalance(line); - } - - if (insideDdSdkCppDeclare) { - final match = _cmakeGitTagPattern.firstMatch(line); - if (match != null) { - return match.namedGroup('comment') ?? match.namedGroup('ref'); - } - } - - if (insideDdSdkCppDeclare && parenDepth <= 0) { - insideDdSdkCppDeclare = false; - } - } - return null; -} /// The native-dependency files found in a package's own directory -- /// resolved by checking what's actually there, not assumed from the @@ -153,7 +80,9 @@ class NativeDependencyFiles { /// pin: an iOS podspec with a Datadog pod dependency, a `Package.swift` /// pinning dd-sdk-ios via SPM, an Android `build.gradle` with a /// `datadog_version`, and/or a `windows/`/`linux/` `CMakeLists.txt` with a -/// dd-sdk-cpp `GIT_TAG`. +/// dd-sdk-cpp `GIT_TAG` inside its `FetchContent_Declare(dd-sdk-cpp ...)` +/// block (a `CMakeLists.txt` whose only `GIT_TAG` belongs to some other +/// vendored dependency doesn't count). NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { File? iosPodspec; File? iosSpmManifest; @@ -188,7 +117,7 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { for (final platformDir in ['windows', 'linux']) { final cmakeFile = File(p.join(packageRoot, platformDir, 'CMakeLists.txt')); if (cmakeFile.existsSync() && - _cmakeGitTagPattern.hasMatch(cmakeFile.readAsStringSync())) { + hasDdSdkCppGitTag(cmakeFile.readAsStringSync())) { cppCMakeLists.add(cmakeFile); } } @@ -201,15 +130,17 @@ NativeDependencyFiles resolveNativeDependencyFiles(String packageRoot) { ); } -/// One file's current pin on a native SDK dependency, and where it came -/// from (e.g. `'podspec'`, `'Package.swift'`, `'windows/CMakeLists.txt'`) -- -/// the label exists purely so a stale pin can be reported back to whoever's -/// reading the plan, not for any comparison logic. -typedef NativeSdkPin = ({String source, String value}); - -/// What's changing (if anything) for one native SDK dependency of a -/// package. [targetVersion] is null when nothing should change -- the -/// patch-branch default, absent an explicit override. +/// What one native SDK dependency of a package resolves to this run. +/// +/// This is a *target*, not a diff: `develop` (and a long-lived pre-release +/// branch) deliberately keeps its manifests on floating constraints (`~> 3`, +/// `branch: "develop"`, `GIT_TAG develop`), and only the release-prep branch's +/// copy is ever pinned. So there's no meaningful "current pin" to subtract +/// from -- the plan just says what to pin to, and `prepare_release.dart` +/// rewrites [files] to match. +/// +/// [targetVersion] is null when nothing should change -- the patch-branch +/// default, absent an explicit override. /// /// [targetSha] is only meaningful for [NativeSdk.cpp]: CMake's /// `FetchContent_Declare` has no field for pinning a tag *and* verifying @@ -221,44 +152,25 @@ class NativeSdkDelta { final String? targetVersion; final String? targetSha; - /// Every file's current pin on this dependency -- one for [NativeSdk. - /// android] (`build.gradle`), up to two for [NativeSdk.ios] (podspec and/ - /// or `Package.swift`), and one per platform for [NativeSdk.cpp] - /// (`windows/CMakeLists.txt`, `linux/CMakeLists.txt`). Outside of a bug, - /// every pin on a dependency should already agree with every other -- - /// they're all pinned to the same target by this same tooling -- so - /// [isChange] just checks that they all still match [targetVersion] - /// rather than tracking each file's staleness independently. - final List pins; - - /// True when a dependency file this package currently ships couldn't be - /// read at [lastReleaseTag] on `release_plan.dart`'s mainline path -- - /// most commonly because the file is new (added since that release) or - /// was renamed/moved, so it has no historical pin to compare against. - /// Treated as a change: silently treating "no historical pin" as "no - /// change" would leave a newly-added or renamed manifest floating. - final bool hasUnknownPin; + /// Every file of this package that pins this dependency and therefore needs + /// rewriting -- one for [NativeSdk.android] (`build.gradle`), up to two for + /// [NativeSdk.ios] (podspec and/or `Package.swift`), and one per platform + /// for [NativeSdk.cpp] (`windows/CMakeLists.txt`, `linux/CMakeLists.txt`). + /// Carried on the plan so the apply step rewrites exactly what discovery + /// found, rather than resolving the file set a second time. + final List files; NativeSdkDelta({ required this.sdk, required this.targetVersion, this.targetSha, - this.pins = const [], - this.hasUnknownPin = false, + this.files = const [], }); - bool get isChange { - if (targetVersion == null) return false; - return hasUnknownPin || pins.any((pin) => pin.value != targetVersion); - } - @override - String toString() { - final current = pins.map((pin) => '${pin.source}=${pin.value}').join(', '); - return isChange - ? '${sdk.name}: $current -> $targetVersion' - : '${sdk.name}: $current (no change)'; - } + String toString() => targetVersion == null + ? '${sdk.name}: no change' + : '${sdk.name}: -> $targetVersion'; } /// The network calls native SDK resolution needs -- bundled so callers diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index e92ae1aac..76e1ca6c8 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -2,12 +2,9 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. -import 'dart:io'; - import 'package:collection/collection.dart'; import 'package:git/git.dart'; import 'package:logging/logging.dart'; -import 'package:path/path.dart' as p; import 'package:version/version.dart'; import 'conventional_commits.dart'; @@ -114,14 +111,8 @@ Future computeReleasePlan( }, resolveCommitSha: (repoSlug, ref) => github.getCommitSha(Logger('native_sdk'), repoSlug, ref), - releaseExists: (repoSlug, version) async { - final release = await github.getReleaseByTagName( - Logger('native_sdk'), - repoSlug, - version, - ); - return release != null; - }, + releaseExists: (repoSlug, version) => + github.releaseExists(Logger('native_sdk'), repoSlug, version), ); final groups = await _resolveGroups(ctx); @@ -149,55 +140,84 @@ Future _computePackagePlan( NativeSdkGateways gateways, { required bool isExplicitlyRequested, }) async { - switch (ctx.trigger) { - case TriggerContext.patch: - case TriggerContext.preRelease: - final nativeSdkDeltas = await _computeNativeSdkDeltas(pkg, ctx, gateways); - return ctx.trigger == TriggerContext.patch - ? await _computePatchPlan(pkg, ctx, gitDir, nativeSdkDeltas) - : await _computePrereleasePlan(pkg, ctx, gitDir, nativeSdkDeltas); - case TriggerContext.mainline: - // Fetched up front (rather than inside _computeNativeSdkDeltas / - // _computeMainlinePlan separately) so both use the exact same tag -- - // it's also what the native SDK comparison below reads its - // "previously released" pin from. - final lastTag = await findLastReleaseTag(gitDir, pkg.name); - final nativeSdkDeltas = await _computeNativeSdkDeltas( - pkg, - ctx, - gateways, - gitDir: gitDir, - lastReleaseTag: lastTag, - ); - return await _computeMainlinePlan( - pkg, - ctx, - gitDir, - nativeSdkDeltas, - lastTag: lastTag, - isExplicitlyRequested: isExplicitlyRequested, - hasNativeSdkChange: nativeSdkDeltas.any((d) => d.isChange), - ); - } -} - -Future _computePatchPlan( - DiscoveredPackage pkg, - RunContext ctx, - GitDir gitDir, - List nativeSdkDeltas, -) async { + // A patch branch is confined to its own release line. Mainline takes the + // last *stable* release as its baseline -- a pre-release line's tags (e.g. + // `4.0.0-beta.3` off `v4`) sort higher but haven't shipped stably. The + // pre-release path wants exactly those, so it constrains neither. final lastTag = await findLastReleaseTag( gitDir, pkg.name, - releaseLine: _releaseLineFromPatchBranch(ctx.currentBranch), + releaseLine: ctx.trigger == TriggerContext.patch + ? _releaseLineFromPatchBranch(ctx.currentBranch) + : null, + stableOnly: ctx.trigger == TriggerContext.mainline, ); final commits = await _conventionalCommitsSince( gitDir, pathspec: pkg.relativePath, - sinceSha: lastTag?.objectSha, + sinceSha: lastTag?.tag.objectSha, ); + final files = resolveNativeDependencyFiles(pkg.absolutePath(ctx.repoRoot)); + + // Whether this package releases at all is decided here, before anything + // touches the network -- resolving native SDK targets for a package that + // turns out to have nothing to ship is pure waste. + // + // Eligibility must come from the package's actual changes, never from + // BUMP_TYPE alone: a targeted override like BUMP_TYPE=major would otherwise + // sweep every discovered package into a major release of the entire repo. + // Patch and pre-release runs release whatever they were pointed at. + if (ctx.trigger == TriggerContext.mainline && + aggregateBumpLevel(commits) == null && + !isExplicitlyRequested && + !_hasForcedNativeUpdate(files, ctx)) { + return null; + } + + final nativeSdkDeltas = await _computeNativeSdkDeltas(files, ctx, gateways); + return switch (ctx.trigger) { + TriggerContext.patch => _computePatchPlan( + pkg, + commits, + lastTag, + nativeSdkDeltas, + ), + TriggerContext.preRelease => _computePrereleasePlan( + pkg, + ctx, + commits, + lastTag, + nativeSdkDeltas, + ), + TriggerContext.mainline => _computeMainlinePlan( + pkg, + ctx, + commits, + lastTag, + nativeSdkDeltas, + ), + }; +} + +/// Whether this run carries an explicit native SDK version override for an SDK +/// [files] shows the package actually depends on. +/// +/// Scoped that way deliberately: an `IOS_SDK_VERSION` on an `--all` run should +/// make the iOS packages eligible, not sweep every pure-Dart package in the +/// repo into the release alongside them. +bool _hasForcedNativeUpdate(NativeDependencyFiles files, RunContext ctx) => + ((files.iosPodspec != null || files.iosSpmManifest != null) && + ctx.iosSdkVersionOverride != null) || + (files.androidGradle != null && ctx.androidSdkVersionOverride != null) || + (files.cppCMakeLists.isNotEmpty && ctx.cppVersionOverride != null); + +PackagePlan _computePatchPlan( + DiscoveredPackage pkg, + List commits, + ReleaseTag? lastTag, + List nativeSdkDeltas, +) { for (final commit in commits) { final bump = commit.bumpType; if (bump == VersionBumpType.major || bump == VersionBumpType.minor) { @@ -209,29 +229,25 @@ Future _computePatchPlan( } } - final newVersion = lastTag == null - ? pkg.version - : Version.parse( - _versionFromTag(lastTag, pkg.name), - ).incrementPatch().toString(); - return PackagePlan( package: pkg, currentVersion: pkg.version, - newVersion: newVersion, + newVersion: lastTag == null + ? pkg.version + : lastTag.version.incrementPatch().toString(), bumpLevel: VersionBumpType.patch, contributingCommits: commits, nativeSdkDeltas: nativeSdkDeltas, ); } -Future _computePrereleasePlan( +PackagePlan _computePrereleasePlan( DiscoveredPackage pkg, RunContext ctx, - GitDir gitDir, + List commits, + ReleaseTag? lastTag, List nativeSdkDeltas, -) async { - final lastTag = await findLastReleaseTag(gitDir, pkg.name); +) { final target = Version.parse(pkg.version); // A prior tag only continues the current prerelease sequence when it's @@ -241,9 +257,7 @@ Future _computePrereleasePlan( // pubspec since bumped to `4.0.0` for this pre-release line) must not be // used as the base, or the new prerelease would sort below that already- // published release. - final tagVersion = lastTag != null - ? Version.parse(_versionFromTag(lastTag, pkg.name)) - : null; + final tagVersion = lastTag?.version; final tagIsOnTargetLine = tagVersion != null && tagVersion.major == target.major && @@ -263,7 +277,7 @@ Future _computePrereleasePlan( // already-published release (`4.0.0-beta.1` < `4.0.0`). throw StateError( 'Package "${pkg.name}" version $target has already been released ' - 'stably as ${lastTag!.tag} -- bump the version in pubspec.yaml ' + 'stably as ${lastTag!.tag.tag} -- bump the version in pubspec.yaml ' 'before starting a new pre-release line.', ); } else if (ctx.prereleaseLabel != null) { @@ -286,65 +300,40 @@ Future _computePrereleasePlan( currentVersion: pkg.version, newVersion: newVersion.toString(), bumpLevel: VersionBumpType.prerelease, + // The bump here is counter-based rather than commit-derived, but the + // commits are still what the changelog is written from. + contributingCommits: commits, nativeSdkDeltas: nativeSdkDeltas, ); } -Future _computeMainlinePlan( +PackagePlan _computeMainlinePlan( DiscoveredPackage pkg, RunContext ctx, - GitDir gitDir, - List nativeSdkDeltas, { - required Tag? lastTag, - required bool isExplicitlyRequested, - required bool hasNativeSdkChange, -}) async { - final commits = await _conventionalCommitsSince( - gitDir, - pathspec: pkg.relativePath, - sinceSha: lastTag?.objectSha, - ); - - var bump = + List commits, + ReleaseTag? lastTag, + List nativeSdkDeltas, +) { + // With nothing auto-detected, this package is here because it was asked for + // by name or a forced native SDK update is driving it -- still worth a + // release, treated as a maintenance patch. + // + // [lastTag] is always a stable version here (see [findLastReleaseTag]'s + // `stableOnly`), so bumping from it is unconditionally right: after a + // long-lived pre-release line merges back, the baseline is still the last + // stable release and the line's own breaking commits are what carry the + // version to the major it was leading up to. + final bump = VersionBumpType.parseOverride(ctx.bumpTypeOverride) ?? - aggregateBumpLevel(commits); - - if (bump == null) { - if (!isExplicitlyRequested && !hasNativeSdkChange) { - // Nothing changed for this package and nobody asked for it by name -- - // --all auto-detection leaves it out of this run entirely. - return null; - } - // Explicitly requested, or only a native SDK bump is driving this - // release: still worth a release, treated as a maintenance patch. - bump = lastTag == null ? null : VersionBumpType.patch; - } - - final tagVersion = lastTag != null - ? Version.parse(_versionFromTag(lastTag, pkg.name)) - : null; - - // A prerelease tag (e.g. `4.0.0-beta.5`) becomes an ancestor of HEAD once - // its branch merges back into mainline, so it can be selected as - // [lastTag] here. Applying a bump on top of it (e.g. incrementPatch -> - // `4.0.1`) would skip the stable release the prerelease line was leading - // up to entirely. Promote it instead: this release publishes that same - // `major.minor.patch` stably, dropping the prerelease suffix, rather than - // bumping past it. - final newVersion = switch (tagVersion) { - null => pkg.version, - final v when v.isPreRelease => Version( - v.major, - v.minor, - v.patch, - ).toString(), - final v => _applyBump(v, bump!).toString(), - }; + aggregateBumpLevel(commits) ?? + (lastTag == null ? null : VersionBumpType.patch); return PackagePlan( package: pkg, currentVersion: pkg.version, - newVersion: newVersion, + newVersion: lastTag == null + ? pkg.version + : _applyBump(lastTag.version, bump!).toString(), bumpLevel: bump, contributingCommits: commits, nativeSdkDeltas: nativeSdkDeltas, @@ -367,129 +356,56 @@ Version _applyBump(Version base, VersionBumpType bump) { } } -/// A tag's name is `{package}/v{version}` -- strip the known prefix rather -/// than trusting [Tag.tag]'s shape blindly. -String _versionFromTag(Tag tag, String packageName) => - tag.tag.substring('$packageName/v'.length); - +/// Resolves what each native SDK this package depends on should be pinned to +/// this run, one [NativeSdkDelta] per SDK it actually ships a manifest for. +/// +/// Purely a *target* resolution -- see [NativeSdkDelta] for why there's no +/// "current pin" to compare against. What each SDK resolves to (latest, +/// an explicit override, or no change on a patch branch) is +/// [resolveNativeSdkTarget]'s call. Future> _computeNativeSdkDeltas( - DiscoveredPackage pkg, + NativeDependencyFiles files, RunContext ctx, - NativeSdkGateways gateways, { - GitDir? gitDir, - Tag? lastReleaseTag, -}) async { - final files = resolveNativeDependencyFiles(pkg.absolutePath(ctx.repoRoot)); - final deltas = []; - - Future content(File file) => _nativeDependencyPinSource( - file, - repoRoot: ctx.repoRoot, - gitDir: gitDir, - lastReleaseTag: lastReleaseTag, - ); - - if (files.iosPodspec != null || files.iosSpmManifest != null) { - final pins = []; - var hasUnknownPin = false; - if (files.iosPodspec != null) { - final fileContent = await content(files.iosPodspec!); - if (fileContent == null) { - hasUnknownPin = true; - } else { - final pin = readIosPodspecPin(fileContent); - if (pin != null) pins.add((source: 'podspec', value: pin)); - } - } - if (files.iosSpmManifest != null) { - final fileContent = await content(files.iosSpmManifest!); - if (fileContent == null) { - hasUnknownPin = true; - } else { - final pin = readSpmPin(fileContent); - if (pin != null) { - pins.add((source: 'Package.swift', value: spmPinForComparison(pin))); - } - } - } - final target = await resolveNativeSdkTarget( - trigger: ctx.trigger, + NativeSdkGateways gateways, +) async { + final perSdkFiles = { + NativeSdk.ios: ( + files: [?files.iosPodspec, ?files.iosSpmManifest], override: ctx.iosSdkVersionOverride, - fetchLatest: () => gateways.fetchLatest(NativeSdk.ios.repoSlug), - releaseExists: (version) => - gateways.releaseExists(NativeSdk.ios.repoSlug, version), - ); - deltas.add( - NativeSdkDelta( - sdk: NativeSdk.ios, - targetVersion: target, - pins: pins, - hasUnknownPin: hasUnknownPin, - ), - ); - } - - if (files.androidGradle != null) { - final fileContent = await content(files.androidGradle!); - final pin = fileContent != null ? readAndroidGradlePin(fileContent) : null; - final target = await resolveNativeSdkTarget( - trigger: ctx.trigger, + ), + NativeSdk.android: ( + files: [?files.androidGradle], override: ctx.androidSdkVersionOverride, - fetchLatest: () => gateways.fetchLatest(NativeSdk.android.repoSlug), - releaseExists: (version) => - gateways.releaseExists(NativeSdk.android.repoSlug, version), - ); - deltas.add( - NativeSdkDelta( - sdk: NativeSdk.android, - targetVersion: target, - pins: [if (pin != null) (source: 'build.gradle', value: pin)], - hasUnknownPin: fileContent == null, - ), - ); - } + ), + NativeSdk.cpp: ( + files: files.cppCMakeLists, + override: ctx.cppVersionOverride, + ), + }; + + final deltas = []; + for (final MapEntry(key: sdk, value: (:files, :override)) + in perSdkFiles.entries) { + if (files.isEmpty) continue; - if (files.cppCMakeLists.isNotEmpty) { - // A package can ship a CMakeLists.txt per platform (windows, linux), - // each pinning dd-sdk-cpp independently -- every one of them needs to - // be checked, not just the first, or a still-stale platform would - // silently be missed (see NativeSdkDelta.pins). - final pins = []; - var hasUnknownPin = false; - for (final file in files.cppCMakeLists) { - final fileContent = await content(file); - if (fileContent == null) { - hasUnknownPin = true; - continue; - } - final pin = readCppCMakePin(fileContent); - if (pin != null) { - pins.add(( - source: p.relative(file.path, from: pkg.absolutePath(ctx.repoRoot)), - value: pin, - )); - } - } final target = await resolveNativeSdkTarget( trigger: ctx.trigger, - override: ctx.cppVersionOverride, - fetchLatest: () => gateways.fetchLatest(NativeSdk.cpp.repoSlug), - releaseExists: (version) => - gateways.releaseExists(NativeSdk.cpp.repoSlug, version), + override: override, + fetchLatest: () => gateways.fetchLatest(sdk.repoSlug), + releaseExists: (version) => gateways.releaseExists(sdk.repoSlug, version), ); - // CMake's FetchContent_Declare has no field for pinning a tag and - // verifying its commit -- the resolved SHA is what actually gets - // written to GIT_TAG (see cmake_util.dart's pinCppVersion). - final targetSha = target != null - ? await gateways.resolveCommitSha(NativeSdk.cpp.repoSlug, target) - : null; + deltas.add( NativeSdkDelta( - sdk: NativeSdk.cpp, + sdk: sdk, targetVersion: target, - targetSha: targetSha, - pins: pins, - hasUnknownPin: hasUnknownPin, + // CMake's FetchContent_Declare has no field for pinning a tag and + // verifying its commit -- the resolved SHA is what actually gets + // written to GIT_TAG (see cmake_util.dart's pinCppVersion). + targetSha: sdk == NativeSdk.cpp && target != null + ? await gateways.resolveCommitSha(sdk.repoSlug, target) + : null, + files: files, ), ); } @@ -497,51 +413,6 @@ Future> _computeNativeSdkDeltas( return deltas; } -/// The content to read a native dependency file's current pin from, for -/// comparison against [NativeSdkDelta.targetVersion]. -/// -/// With no [lastReleaseTag] (patch/pre-release branches, or a package's -/// first-ever mainline release), [file]'s own on-disk content already -/// reflects what was actually pinned last -- release tooling rewrites it in -/// place. On mainline, though, [file] is `develop`'s own copy, which is -/// deliberately left on a floating constraint (`~> 3`, `branch: "develop"`, -/// `GIT_TAG develop`) by that same tooling -- comparing the resolved target -/// against a floating constraint would report a native SDK change on every -/// run. What was actually shipped is whatever got pinned into this same -/// file right before [lastReleaseTag] was cut, so read that historical -/// blob instead. Returns null (nothing to compare) if the file didn't -/// exist yet at that tag. -Future _nativeDependencyPinSource( - File file, { - required String repoRoot, - GitDir? gitDir, - Tag? lastReleaseTag, -}) async { - if (gitDir == null || lastReleaseTag == null) { - return file.readAsStringSync(); - } - final relativePath = p.posix.joinAll( - p.split(p.relative(file.path, from: repoRoot)), - ); - final result = await gitDir.runCommand([ - 'show', - '${lastReleaseTag.tag}:$relativePath', - ], throwOnError: false); - if (result.exitCode != 0) { - // Usually means the file didn't exist yet at lastReleaseTag, but it's - // equally what a moved/renamed file looks like -- log so a real stale - // pin hiding behind a rename isn't dropped completely silently. - Logger('native_sdk').warning( - "⚠️ Couldn't read $relativePath as of ${lastReleaseTag.tag} " - '(git show exit ${result.exitCode}); skipping it for native SDK ' - 'comparison. If this file moved since that release, the comparison ' - 'may miss a stale pin.', - ); - return null; - } - return result.stdout as String; -} - /// [commitMessagesSince]'s raw messages, parsed into [ConventionalCommit]s /// and narrowed to the ones that carry semver weight -- commits that fail /// to parse, or parse but don't bump anything (`chore:`, `docs:`, etc.), diff --git a/tools/releaser/lib/release_validator.dart b/tools/releaser/lib/release_validator.dart index eb91eae0e..7a4df9dba 100644 --- a/tools/releaser/lib/release_validator.dart +++ b/tools/releaser/lib/release_validator.dart @@ -9,9 +9,7 @@ import 'package:path/path.dart' as path; import 'command.dart'; import 'github_cmd_wrapper.dart'; import 'helpers.dart'; -import 'native_sdk.dart'; import 'process_helper.dart'; -import 'trigger_context.dart'; final versionHeadingRegEx = RegExp(r'\s*#'); final changeItemRegEx = RegExp(r'\s*\*'); @@ -107,37 +105,24 @@ class ValidateReleaseCommand extends Command { Logger logger, ) async { final gh = GithubCommandWrapper(args.gitDir.path); - try { - // This legacy CLI has no notion of a patch/pre-release trigger - // context -- it always behaves like `mainline`: default to the - // latest release when none is given, or validate an explicit one. - final resolved = await resolveNativeSdkTarget( - trigger: TriggerContext.mainline, - override: release, - fetchLatest: () async { - logger.fine('🌎 Fetching latest $platform release from github... '); - final latestRelease = await gh.getLatestRelease(logger, repoName); - logger.fine('ℹ️ Latest $platform release is ${latestRelease.name}'); - return latestRelease.tagName; - }, - releaseExists: (version) async { - final ghRelease = await gh.getReleaseByTagName( - logger, - repoName, - version, - ); - return ghRelease != null; - }, - ); - logger.info('ℹ️ Releasing with $platform version $resolved.'); - return resolved; - } on StateError { + // This legacy CLI has no notion of a patch/pre-release trigger context -- + // it always behaves like `mainline`: default to the latest release when + // none is given, or validate an explicit one. + if (release == null) { + logger.fine('🌎 Fetching latest $platform release from github... '); + final latestRelease = await gh.getLatestRelease(logger, repoName); + logger.fine('ℹ️ Latest $platform release is ${latestRelease.name}'); + release = latestRelease.tagName; + } else if (!await gh.releaseExists(logger, repoName, release)) { logger.shout( '❌ Could not find target $platform release $release. Please check the tag name', ); return null; } + + logger.info('ℹ️ Releasing with $platform version $release.'); + return release; } } diff --git a/tools/releaser/test/git_history_test.dart b/tools/releaser/test/git_history_test.dart index c8aafb3a7..5889119e4 100644 --- a/tools/releaser/test/git_history_test.dart +++ b/tools/releaser/test/git_history_test.dart @@ -29,7 +29,7 @@ void main() { final tag = await findLastReleaseTag(gitDir, 'datadog_dio'); expect(tag, isNotNull); - expect(tag!.tag, 'datadog_dio/v2.3.0'); + expect(tag!.tag.tag, 'datadog_dio/v2.3.0'); }); test('returns null when a package has never been tagged', () async { @@ -57,28 +57,37 @@ void main() { ); expect(tag, isNotNull); - expect(tag!.tag, 'datadog_dio/v2.0.0'); + expect(tag!.tag.tag, 'datadog_dio/v2.0.0'); }); test( - 'ignores a higher-versioned tag that is not an ancestor of HEAD -- ' - 'e.g. cut on a long-lived prerelease branch that never merged back', + 'stableOnly ignores a higher-versioned prerelease tag -- e.g. one cut on ' + 'a long-lived pre-release line that has not shipped stably yet', () async { fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.2.0 work'); await fixture.commit('fix: something for 2.2.0'); await fixture.tag('datadog_dio/v2.2.0'); - await fixture.checkoutNewBranch('prerelease-line'); fixture.writeFile('packages/datadog_dio/CHANGES', 'v4.0.0-beta.1 work'); await fixture.commit('feat!: something for 4.0.0-beta.1'); await fixture.tag('datadog_dio/v4.0.0-beta.1'); - await fixture.checkout('main'); final gitDir = await fixture.gitDir; - final tag = await findLastReleaseTag(gitDir, 'datadog_dio'); - expect(tag, isNotNull); - expect(tag!.tag, 'datadog_dio/v2.2.0'); + // Mainline's baseline: the last thing actually shipped stably. + expect( + (await findLastReleaseTag( + gitDir, + 'datadog_dio', + stableOnly: true, + ))?.tag.tag, + 'datadog_dio/v2.2.0', + ); + // The pre-release path wants the opposite -- that beta is its counter. + expect( + (await findLastReleaseTag(gitDir, 'datadog_dio'))?.tag.tag, + 'datadog_dio/v4.0.0-beta.1', + ); }, ); @@ -91,7 +100,7 @@ void main() { final tagSha = (await findLastReleaseTag( await fixture.gitDir, 'datadog_dio', - ))!.objectSha; + ))!.tag.objectSha; fixture.writeFile('packages/datadog_dio/CHANGES', 'unreleased'); await fixture.commit('feat: not yet released'); diff --git a/tools/releaser/test/native_sdk_test.dart b/tools/releaser/test/native_sdk_test.dart index edce2bb78..9982fbc67 100644 --- a/tools/releaser/test/native_sdk_test.dart +++ b/tools/releaser/test/native_sdk_test.dart @@ -49,75 +49,6 @@ let package = Package( '''; void main() { - group('reading current pins', () { - test('readIosPodspecPin finds the shared Datadog pod constraint', () { - expect(readIosPodspecPin(_iosPodspec), '~> 3'); - }); - - test('readIosPodspecPin returns null with no Datadog dependency', () { - expect(readIosPodspecPin("s.dependency 'Flutter'"), isNull); - }); - - test('readAndroidGradlePin finds ext.datadog_version', () { - expect(readAndroidGradlePin(_androidGradle), '3.11.0'); - }); - - test('readAndroidGradlePin returns null with no datadog_version', () { - expect(readAndroidGradlePin('ext.kotlin_version = "2.2.20"'), isNull); - }); - - test('readCppCMakePin finds GIT_TAG regardless of trailing syntax', () { - expect(readCppCMakePin(_windowsCMakeLists), 'develop'); - expect(readCppCMakePin(_linuxCMakeLists), 'develop'); - }); - - test( - 'readCppCMakePin prefers the trailing tag comment over a pinned SHA', - () { - const pinned = ''' -FetchContent_Declare(dd-sdk-cpp - GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git - GIT_TAG a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2) # v1.4.0 -'''; - // The tag, not the opaque SHA -- that's what a freshly-resolved - // target tag needs to compare against to detect "no change". - expect(readCppCMakePin(pinned), 'v1.4.0'); - }, - ); - - test('readCppCMakePin returns null with no GIT_TAG', () { - expect(readCppCMakePin('FetchContent_Declare(something_else)'), isNull); - }); - - test('readCppCMakePin ignores a GIT_TAG belonging to another ' - 'FetchContent_Declare that appears before dd-sdk-cpp', () { - const content = ''' -FetchContent_Declare(some_other_dep - GIT_REPOSITORY https://github.com/example/some_other_dep.git - GIT_TAG v9.9.9) -FetchContent_MakeAvailable(some_other_dep) - -FetchContent_Declare(dd-sdk-cpp - GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git - GIT_TAG develop) -'''; - expect(readCppCMakePin(content), 'develop'); - }); - - test('readSpmPin finds the dd-sdk-ios dependency spec', () { - expect(readSpmPin(_packageSwift), 'from: "3.0.0"'); - }); - - test('readSpmPin returns null with no dd-sdk-ios dependency', () { - expect( - readSpmPin( - '.package(url: "https://github.com/other/pkg.git", from: "1.0.0")', - ), - isNull, - ); - }); - }); - group('resolveNativeDependencyFiles', () { late Directory root; @@ -181,6 +112,20 @@ FetchContent_Declare(dd-sdk-cpp expect(files.isEmpty, isTrue); }); + test('ignores a CMakeLists.txt whose only GIT_TAG belongs to a different ' + 'FetchContent_Declare', () { + write('windows/CMakeLists.txt', ''' +FetchContent_Declare(some_other_dep + GIT_REPOSITORY https://github.com/example/some_other_dep.git + GIT_TAG v9.9.9) +'''); + + final files = resolveNativeDependencyFiles(root.path); + + expect(files.cppCMakeLists, isEmpty); + expect(files.isEmpty, isTrue); + }); + test('ignores a podspec with no Datadog dependency', () { write('ios/some_other_plugin.podspec', "s.dependency 'Flutter'"); @@ -271,118 +216,4 @@ FetchContent_Declare(dd-sdk-cpp expect(target, '3.13.0'); }); }); - - group('NativeSdkDelta.isChange', () { - test('is false when the target matches the current pin', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.android, - pins: [(source: 'build.gradle', value: '3.11.0')], - targetVersion: '3.11.0', - ); - expect(delta.isChange, isFalse); - }); - - test('is false when there is no target (no change)', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.android, - pins: [(source: 'build.gradle', value: '3.11.0')], - targetVersion: null, - ); - expect(delta.isChange, isFalse); - }); - - test('is true when the target differs from the current pin', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.android, - pins: [(source: 'build.gradle', value: '3.11.0')], - targetVersion: '3.12.0', - ); - expect(delta.isChange, isTrue); - }); - - test('is true when the podspec matches but the SPM pin lags behind -- ' - 'both must track the same version', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.ios, - pins: [ - (source: 'podspec', value: '3.12.0'), - (source: 'Package.swift', value: '3.0.0'), - ], - targetVersion: '3.12.0', - ); - expect(delta.isChange, isTrue); - }); - - test('is true when the SPM pin matches but the podspec lags behind', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.ios, - pins: [ - (source: 'podspec', value: '~> 3'), - (source: 'Package.swift', value: '3.12.0'), - ], - targetVersion: '3.12.0', - ); - expect(delta.isChange, isTrue); - }); - - test('is false when both the podspec and SPM pin match the target', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.ios, - pins: [ - (source: 'podspec', value: '3.12.0'), - (source: 'Package.swift', value: '3.12.0'), - ], - targetVersion: '3.12.0', - ); - expect(delta.isChange, isFalse); - }); - - test('a branch-tracking SPM pin always counts as needing a change', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.ios, - pins: [ - (source: 'podspec', value: '3.12.0'), - (source: 'Package.swift', value: 'branch: "develop"'), - ], - targetVersion: '3.12.0', - ); - expect(delta.isChange, isTrue); - }); - - test('is true when the first pin matches but an additional pin ' - '(e.g. a second CMakeLists) lags behind', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.cpp, - pins: [ - (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), - (source: 'linux/CMakeLists.txt', value: 'develop'), - ], - targetVersion: 'v1.4.0', - ); - expect(delta.isChange, isTrue); - }); - - test('is false when the first pin and every additional pin match', () { - final delta = NativeSdkDelta( - sdk: NativeSdk.cpp, - pins: [ - (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), - (source: 'linux/CMakeLists.txt', value: 'v1.4.0'), - ], - targetVersion: 'v1.4.0', - ); - expect(delta.isChange, isFalse); - }); - }); - - group('spmPinForComparison', () { - test('extracts the bare version literal from a version-pinned spec', () { - expect(spmPinForComparison('from: "3.0.0"'), '3.0.0'); - expect(spmPinForComparison('exact: "3.12.0"'), '3.12.0'); - }); - - test('returns a branch-tracking spec unchanged', () { - expect(spmPinForComparison('branch: "develop"'), 'branch: "develop"'); - }); - }); } diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index 845f58c39..360dd01a2 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -2,6 +2,7 @@ // This product includes software developed at Datadog (https://www.datadoghq.com/). // Copyright 2019-Present Datadog, Inc. +import 'package:path/path.dart' as p; import 'package:releaser/native_sdk.dart'; import 'package:releaser/release_plan.dart'; import 'package:test/test.dart'; @@ -115,6 +116,60 @@ void main() { expect(names, contains('datadog_flutter_plugin_ios')); }, ); + + test('a native SDK override does not sweep in a package that ships no ' + 'manifest for that SDK', () async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + await fixture.commit('chore: add podspec fixture'); + + final result = await plan(mainlineCtx(iosSdkVersionOverride: '3.12.0')); + final names = result.packages.map((p) => p.package.name); + + // Pure-Dart: an IOS_SDK_VERSION has nothing to do with it. + expect(names, isNot(contains('lonely_ios'))); + expect(names, isNot(contains('datadog_dio'))); + }); + + test("an override for one SDK doesn't sweep in a package that only depends " + 'on another', () async { + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' + 'datadog_flutter_plugin_ios.podspec', + _iosPodspecWithDatadogDependency, + ); + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' + 'windows/CMakeLists.txt', + _windowsCMakeListsWithGitTag, + ); + await fixture.commit('chore: add native dependency fixtures'); + + final result = await plan( + mainlineCtx(cppVersionOverride: 'v1.4.0'), + resolveCommitSha: (repoSlug, ref) async => + 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2', + ); + final names = result.packages.map((p) => p.package.name); + + expect(names, contains('datadog_flutter_plugin_desktop')); + expect(names, isNot(contains('datadog_flutter_plugin_ios'))); + }); + + test('BUMP_TYPE alone does not sweep an otherwise-unqualifying package ' + 'into --all', () async { + // No qualifying commits, no native SDK change, not explicitly + // requested -- BUMP_TYPE must not be the thing that grants + // eligibility here, or a targeted override would release every + // discovered package. + final result = await plan(mainlineCtx(bumpTypeOverride: 'major')); + final names = result.packages.map((p) => p.package.name); + + expect(names, isNot(contains('lonely_ios'))); + }); }); group('mainline, version computation', () { @@ -220,19 +275,28 @@ void main() { expect(result.packages.single.newVersion, '1.1.1'); }); - test('a merged prerelease tag is promoted to the stable version it was ' - 'leading up to, rather than bumped past it', () async { - // Simulates a long-lived prerelease branch (`v4`) merging back into - // mainline -- its beta tag becomes an ancestor of HEAD and would - // otherwise be picked as lastTag and bumped from, skipping 4.0.0. - await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.3'); + test( + "a pre-release line's tags are not mainline's baseline -- the last " + 'stable release is, and the merged line\'s own commits carry the bump', + () async { + // A long-lived `v4` line ships betas, then merges back into mainline. + await fixture.tag('datadog_flutter_plugin/v3.5.0'); + fixture.writeFile( + 'packages/datadog_flutter_plugin/datadog_flutter_plugin/CHANGES', + 'the v4 work', + ); + await fixture.commit('feat!: the federation rework'); + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.3'); - final result = await plan( - mainlineCtx(requestedPackages: ['datadog_flutter_plugin']), - ); + final result = await plan( + mainlineCtx(requestedPackages: ['datadog_flutter_plugin']), + ); - expect(result.packages.single.newVersion, '4.0.0'); - }); + // 3.5.0 + the breaking commit, not 4.0.0-beta.3 + a patch bump. + expect(result.packages.single.newVersion, '4.0.0'); + expect(result.packages.single.bumpLevel, VersionBumpType.major); + }, + ); }); group('mainline, native SDK deltas', () { @@ -256,9 +320,8 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.pins, [(source: 'podspec', value: '~> 3')]); + expect(delta.sdk, NativeSdk.ios); expect(delta.targetVersion, '3.12.0'); - expect(delta.isChange, isTrue); }); test('with no override, resolves via fetchLatest', () async { @@ -281,50 +344,8 @@ void main() { expect(result.packages.single.nativeSdkDeltas, isEmpty); }); - test('isChange compares against the pin from the last release tag, not ' - "develop's intentionally-floating current pin", () async { - // Simulate what release tooling actually pins into the file right - // before cutting a release. - fixture.writeFile( - 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' - 'datadog_flutter_plugin_ios.podspec', - "Pod::Spec.new do |s|\n s.dependency 'DatadogCore', '3.10.0'\nend\n", - ); - await fixture.commit('chore: release datadog_flutter_plugin_ios 1.0.0'); - await fixture.tag('datadog_flutter_plugin_ios/v1.0.0'); - - // develop moves on, reverting back to its usual floating pin -- - // this must not be mistaken for a native SDK change. - fixture.writeFile( - 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' - 'datadog_flutter_plugin_ios.podspec', - _iosPodspecWithDatadogDependency, - ); - await fixture.commit('chore: back to floating on develop'); - - final unchanged = await plan( - mainlineCtx(), - fetchLatestNativeSdkVersion: (_) async => '3.10.0', - ); - expect( - unchanged.packages.map((p) => p.package.name), - isNot(contains('datadog_flutter_plugin_ios')), - ); - - final changed = await plan( - mainlineCtx(), - fetchLatestNativeSdkVersion: (_) async => '3.13.0', - ); - final delta = changed.packages - .firstWhere((p) => p.package.name == 'datadog_flutter_plugin_ios') - .nativeSdkDeltas - .single; - expect(delta.pins, [(source: 'podspec', value: '3.10.0')]); - expect(delta.isChange, isTrue); - }); - test( - 'a Package.swift alongside the podspec surfaces its own current pin', + 'a Package.swift alongside the podspec is picked up for rewriting', () async { fixture.writeFile( 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' @@ -341,11 +362,11 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.pins, [ - (source: 'podspec', value: '~> 3'), - (source: 'Package.swift', value: '3.0.0'), - ]); expect(delta.targetVersion, '3.12.0'); + expect(delta.files.map((f) => p.basename(f.path)), [ + 'datadog_flutter_plugin_ios.podspec', + 'Package.swift', + ]); }, ); }); @@ -374,9 +395,6 @@ void main() { ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.pins, [ - (source: 'windows/CMakeLists.txt', value: 'develop'), - ]); expect(delta.targetVersion, 'v1.4.0'); expect(delta.targetSha, 'a1b2c3d4e5f6a1b2c3d4e5f6a1b2c3d4e5f6a1b2'); }); @@ -400,26 +418,14 @@ void main() { }, ); - test('a still-stale second CMakeLists (e.g. linux) is not masked by a ' - 'first one (windows) that already matches the target', () async { - // windows/CMakeLists.txt (from setUp, checked first) is rewritten - // to already match the target; linux/CMakeLists.txt is still - // pinned to "develop". - fixture.writeFile( - 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' - 'windows/CMakeLists.txt', - ''' -FetchContent_Declare(dd-sdk-cpp - GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git - GIT_TAG deadbeefdeadbeefdeadbeefdeadbeefdeadbeef) # v1.4.0 -''', - ); + test('every platform CMakeLists is carried for rewriting, not just the ' + 'first', () async { fixture.writeFile( 'packages/datadog_flutter_plugin/datadog_flutter_plugin_desktop/' 'linux/CMakeLists.txt', _windowsCMakeListsWithGitTag, ); - await fixture.commit('chore: update CMakeLists fixtures'); + await fixture.commit('chore: add the linux CMakeLists fixture'); final result = await plan( mainlineCtx( @@ -431,37 +437,10 @@ FetchContent_Declare(dd-sdk-cpp ); final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.pins, [ - (source: 'windows/CMakeLists.txt', value: 'v1.4.0'), - (source: 'linux/CMakeLists.txt', value: 'develop'), + expect(delta.files.map((f) => p.basename(p.dirname(f.path))), [ + 'windows', + 'linux', ]); - expect(delta.isChange, isTrue); - }); - }); - - group('mainline, native SDK deltas with a manifest added after the last ' - 'release', () { - test('a manifest with no historical pin to compare against is treated as ' - 'a change rather than silently skipped', () async { - // Released with no podspec at all -- nothing to compare against. - await fixture.tag('datadog_flutter_plugin_ios/v1.0.0'); - - // The podspec is only added afterwards. - fixture.writeFile( - 'packages/datadog_flutter_plugin/datadog_flutter_plugin_ios/ios/' - 'datadog_flutter_plugin_ios.podspec', - _iosPodspecWithDatadogDependency, - ); - await fixture.commit('chore: add podspec fixture after the release'); - - final result = await plan( - mainlineCtx(requestedPackages: ['datadog_flutter_plugin_ios']), - fetchLatestNativeSdkVersion: (_) async => '3.13.0', - ); - - final delta = result.packages.single.nativeSdkDeltas.single; - expect(delta.pins, isEmpty); - expect(delta.isChange, isTrue); }); }); From 2869929291a16c424b98acb45901d2c94c26f141 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Thu, 27 Aug 2026 09:41:47 -0400 Subject: [PATCH 08/10] tools(releaser): Scope the pre-release tag and support single-line FetchContent An unconstrained lookup returns the package's highest tag overall, so a pre-release run targeting 4.0.0 picks up an unrelated `4.0.1` patch or a concurrent `5.0.0-beta.1`. Neither is on the target line, so the plan falls back to the pubspec version as its base and regenerates `4.0.0-beta.1` -- a tag that already exists, which fails on push. Fix by scoping the release line to one of three options: mainline -- the last stable release patch -- the last release on the branch's release line pre-release -- the last release at the exact declared target version The scope comes from the pubspec version, so nothing new has to be passed in. `releaseLine` generalizes to `versionScope` with a nullable patch component -- patch runs leave it open, pre-release pins it, since "the last release at 4.0.0" and "the last release on the 4.0 line" are different questions. Additionally, `FetchContent_Declare(dd-sdk-cpp ... GIT_TAG develop)` written on one line is valid CMake, but the GIT_TAG pattern was anchored to the start of a line, so neither discovery nor rewriting could see it. Still not handled: a declaration split as `GIT_TAG\n develop` across two lines. transformFile is line-by-line, so that needs a different approach, and no manifest in this repo is written that way. Co-Authored-By: Claude Opus 5 (1M context) --- tools/releaser/lib/cmake_util.dart | 62 ++++++++++++------ tools/releaser/lib/git_history.dart | 21 ++++-- tools/releaser/lib/release_plan.dart | 74 +++++++++++++--------- tools/releaser/test/cmake_util_test.dart | 32 ++++++++++ tools/releaser/test/git_history_test.dart | 24 ++++++- tools/releaser/test/release_plan_test.dart | 14 ++++ 6 files changed, 170 insertions(+), 57 deletions(-) diff --git a/tools/releaser/lib/cmake_util.dart b/tools/releaser/lib/cmake_util.dart index 38127772c..49c0c94bc 100644 --- a/tools/releaser/lib/cmake_util.dart +++ b/tools/releaser/lib/cmake_util.dart @@ -8,9 +8,15 @@ import 'package:logging/logging.dart'; import 'helpers.dart'; -final _gitTagLinePattern = RegExp( - r'^(?\s*GIT_TAG\s+)(?[\w./-]+)(?[^#]*)(?:#.*)?$', -); +/// Matches the `GIT_TAG ` argument anywhere on a line -- not anchored to +/// the line start, since `FetchContent_Declare(dd-sdk-cpp ... GIT_TAG develop)` +/// is valid CMake written on a single line. +final _gitTagPattern = RegExp(r'(?GIT_TAG\s+)(?[\w./-]+)'); + +/// A trailing `# ...` comment, stripped before [_gitTagPattern] is applied so +/// a previous run's `# ` annotation is replaced rather than duplicated -- +/// and so a `#`-commented mention of `GIT_TAG` can't be mistaken for the pin. +final _trailingCommentPattern = RegExp(r'\s*#.*$'); final _ddSdkCppDeclareStartPattern = RegExp( r'FetchContent_Declare\(\s*dd-sdk-cpp\b', @@ -56,11 +62,35 @@ class _DdSdkCppBlockScanner { bool hasDdSdkCppGitTag(String cmakeListsContent) { final scanner = _DdSdkCppBlockScanner(); for (final line in cmakeListsContent.split('\n')) { - if (scanner.accept(line) && _gitTagLinePattern.hasMatch(line)) return true; + if (scanner.accept(line) && + _gitTagPattern.hasMatch( + line.replaceFirst(_trailingCommentPattern, ''), + )) { + return true; + } } return false; } +/// Rewrites [line]'s `GIT_TAG` ref to [targetSha], re-annotated with +/// `# [targetTag]`, leaving everything else on the line as-is -- a trailing +/// `)` closing the `FetchContent_Declare(...` call, a `GIT_REPOSITORY` sharing +/// the line in a single-line declaration, the original indentation. The +/// comment goes at end of line, since a `#` placed before the closing paren +/// would comment it out and break the call. +String _pinGitTagLine(String line, String targetTag, String targetSha) { + final bare = line.replaceFirst(_trailingCommentPattern, ''); + final match = _gitTagPattern.firstMatch(bare); + if (match == null) return line; + + final pinned = bare.replaceRange( + match.start, + match.end, + '${match.namedGroup('prefix')}$targetSha', + ); + return '${pinned.trimRight()} # $targetTag'; +} + /// Rewrites a CMakeLists.txt's dd-sdk-cpp `GIT_TAG` line to pin at /// [targetSha] -- the resolved commit SHA for [targetTag] (e.g. `v1.4.0`), /// kept as a trailing `# ` comment since CMake's `FetchContent_Declare` @@ -68,10 +98,8 @@ bool hasDdSdkCppGitTag(String cmakeListsContent) { /// full commit SHA is used as the actual pin (not the tag) because it's /// immutable, unlike a tag, which can be moved to point elsewhere later. /// -/// Preserves the line's existing whitespace and anything between the ref -/// and end of line -- a trailing `)` closing the `FetchContent_Declare(...` -/// call is common, and the new comment is appended *after* it, since a `#` -/// placed before would comment out the paren too and break the call. +/// Preserves the line's existing whitespace and everything around the ref -- +/// see [_pinGitTagLine]. /// /// Only ever called against a release-prep/patch/pre-release branch's copy /// of the file -- `develop`'s own floating `GIT_TAG develop` is never @@ -90,14 +118,12 @@ Future pinCppVersion( final scanner = _DdSdkCppBlockScanner(); - await transformFile(cmakeListsFile, logger, dryRun, (line) { - if (!scanner.accept(line)) return line; - - final match = _gitTagLinePattern.firstMatch(line); - if (match == null) return line; - - final prefix = match.namedGroup('prefix')!; - final trailing = (match.namedGroup('trailing') ?? '').trimRight(); - return '$prefix$targetSha$trailing # $targetTag'; - }); + await transformFile( + cmakeListsFile, + logger, + dryRun, + (line) => scanner.accept(line) + ? _pinGitTagLine(line, targetTag, targetSha) + : line, + ); } diff --git a/tools/releaser/lib/git_history.dart b/tools/releaser/lib/git_history.dart index f45fc3e9d..1308ac234 100644 --- a/tools/releaser/lib/git_history.dart +++ b/tools/releaser/lib/git_history.dart @@ -22,9 +22,14 @@ typedef ReleaseTag = ({Tag tag, Version version}); /// ancient tags. The two callers that need to exclude something exclude it by /// what the tag *is*, not by where it sits in the graph: /// -/// [releaseLine], when given, restricts the search to tags whose major/minor -/// matches -- a patch branch's own release line, so a tag mainline has since -/// cut for a newer major/minor can't be picked up instead. +/// [versionScope], when given, restricts the search to tags at that +/// major/minor -- and, when its `patch` is non-null, that exact +/// major.minor.patch. A patch branch scopes to its own release line, so a tag +/// mainline has since cut for a newer major/minor can't be picked up instead. +/// A pre-release run scopes to the full version its pubspec declares as the +/// target, since "the last release at 4.0.0" is the only tag that can continue +/// its counter -- an unrelated higher tag (a `4.0.1` patch, a concurrent +/// `5.0.0-beta.1`) would otherwise be selected and mask the real one. /// /// [stableOnly] drops pre-release versions. Mainline passes this: a /// `4.0.0-beta.3` tag cut off a long-lived pre-release line sorts above the @@ -34,7 +39,7 @@ typedef ReleaseTag = ({Tag tag, Version version}); Future findLastReleaseTag( GitDir gitDir, String packageName, { - (int major, int minor)? releaseLine, + ({int major, int minor, int? patch})? versionScope, bool stableOnly = false, }) async { final prefix = '$packageName/v'; @@ -55,9 +60,11 @@ Future findLastReleaseTag( .where((pair) => !stableOnly || !pair.version.isPreRelease) .where( (pair) => - releaseLine == null || - (pair.version.major == releaseLine.$1 && - pair.version.minor == releaseLine.$2), + versionScope == null || + (pair.version.major == versionScope.major && + pair.version.minor == versionScope.minor && + (versionScope.patch == null || + pair.version.patch == versionScope.patch)), ) .toList() ..sort((a, b) => a.version.compareTo(b.version)); diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index 76e1ca6c8..46ff22454 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -140,16 +140,21 @@ Future _computePackagePlan( NativeSdkGateways gateways, { required bool isExplicitlyRequested, }) async { - // A patch branch is confined to its own release line. Mainline takes the - // last *stable* release as its baseline -- a pre-release line's tags (e.g. - // `4.0.0-beta.3` off `v4`) sort higher but haven't shipped stably. The - // pre-release path wants exactly those, so it constrains neither. + // Each trigger asks a different question of the tag history, and each asks + // it in the query rather than filtering afterwards: + // mainline -- the last *stable* release. A pre-release line's tags + // (`4.0.0-beta.3` off `v4`) sort higher but haven't shipped. + // patch -- the last release on the branch's own release line. + // pre-release -- the last release at the exact version pubspec declares as + // the target, the only tag that can continue its counter. final lastTag = await findLastReleaseTag( gitDir, pkg.name, - releaseLine: ctx.trigger == TriggerContext.patch - ? _releaseLineFromPatchBranch(ctx.currentBranch) - : null, + versionScope: switch (ctx.trigger) { + TriggerContext.mainline => null, + TriggerContext.patch => _versionScopeFromPatchBranch(ctx.currentBranch), + TriggerContext.preRelease => _versionScopeFromTarget(pkg.version), + }, stableOnly: ctx.trigger == TriggerContext.mainline, ); final commits = await _conventionalCommitsSince( @@ -250,34 +255,26 @@ PackagePlan _computePrereleasePlan( ) { final target = Version.parse(pkg.version); - // A prior tag only continues the current prerelease sequence when it's - // for the exact version pubspec.yaml is declaring as the target -- e.g. - // a `4.0.0-beta.1` tag continues towards a pubspec of `4.0.0`. A tag for - // an older, already-published line (say the last stable `3.2.0`, with - // pubspec since bumped to `4.0.0` for this pre-release line) must not be - // used as the base, or the new prerelease would sort below that already- - // published release. - final tagVersion = lastTag?.version; - final tagIsOnTargetLine = - tagVersion != null && - tagVersion.major == target.major && - tagVersion.minor == target.minor && - tagVersion.patch == target.patch; - final base = tagIsOnTargetLine ? tagVersion : target; + // [lastTag] is already scoped to [target]'s exact major.minor.patch (see + // _versionScopeFromTarget), so any tag found here is by construction one + // that continues this pre-release line -- either an earlier counter for it + // or the stable release it was leading up to. With none, the pubspec's own + // declared version is the base. + final base = lastTag?.version ?? target; final Version newVersion; if (base.isPreRelease && (ctx.prereleaseLabel == null || base.preRelease.first == ctx.prereleaseLabel)) { newVersion = base.incrementPreRelease(); - } else if (tagIsOnTargetLine && !tagVersion.isPreRelease) { + } else if (lastTag != null && !base.isPreRelease) { // [lastTag] is a *stable* tag for this exact target version (e.g. the // target line was already released as 4.0.0 and pubspec.yaml hasn't // been bumped since) -- a new pre-release here would sort below that // already-published release (`4.0.0-beta.1` < `4.0.0`). throw StateError( 'Package "${pkg.name}" version $target has already been released ' - 'stably as ${lastTag!.tag.tag} -- bump the version in pubspec.yaml ' + 'stably as ${lastTag.tag.tag} -- bump the version in pubspec.yaml ' 'before starting a new pre-release line.', ); } else if (ctx.prereleaseLabel != null) { @@ -444,18 +441,35 @@ final _patchBranchPattern = RegExp( String? _packageNameFromPatchBranch(String branch) => _patchBranchPattern.firstMatch(branch)?.namedGroup('package'); -/// Extracts the `{major}.{minor}` release line from a -/// `release/{package}/v{major}.{minor}.x` patch-branch name. Only called -/// once [_resolveGroups] has already validated the branch matches the -/// convention, so a non-match here would be a bug in that validation. -(int major, int minor) _releaseLineFromPatchBranch(String branch) { +/// The `{major}.{minor}` release line from a +/// `release/{package}/v{major}.{minor}.x` patch-branch name, as a scope for +/// [findLastReleaseTag] -- `patch` is left open, since any patch level on that +/// line is a valid predecessor. Only called once [_resolveGroups] has already +/// validated the branch matches the convention, so a non-match here would be a +/// bug in that validation. +({int major, int minor, int? patch}) _versionScopeFromPatchBranch( + String branch, +) { final match = _patchBranchPattern.firstMatch(branch)!; return ( - int.parse(match.namedGroup('major')!), - int.parse(match.namedGroup('minor')!), + major: int.parse(match.namedGroup('major')!), + minor: int.parse(match.namedGroup('minor')!), + patch: null, ); } +/// The exact `{major}.{minor}.{patch}` a pre-release run is working towards, +/// from the package's declared pubspec version, as a scope for +/// [findLastReleaseTag]. +/// +/// Deliberately drops any pre-release suffix the pubspec itself carries: a +/// pubspec sitting at `4.0.0-beta.1` mid-line is still targeting `4.0.0`, and +/// its own earlier betas are exactly the tags that must be found. +({int major, int minor, int? patch}) _versionScopeFromTarget(String version) { + final target = Version.parse(version); + return (major: target.major, minor: target.minor, patch: target.patch); +} + Future> _resolveGroups(RunContext ctx) async { final allGroups = await discoverPackages(ctx.repoRoot); diff --git a/tools/releaser/test/cmake_util_test.dart b/tools/releaser/test/cmake_util_test.dart index 72d95c639..bc0d0d15e 100644 --- a/tools/releaser/test/cmake_util_test.dart +++ b/tools/releaser/test/cmake_util_test.dart @@ -108,6 +108,38 @@ FetchContent_MakeAvailable(some_other_dep) expect(contents, contains('GIT_TAG v9.9.9)')); }); + test('handles a declaration written on a single line', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString( + 'FetchContent_Declare(dd-sdk-cpp ' + 'GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git ' + 'GIT_TAG develop)\n', + ); + + await pinCppVersion(file, 'v1.4.0', _sha, logger, false); + + expect( + (await file.readAsString()).trim(), + 'FetchContent_Declare(dd-sdk-cpp ' + 'GIT_REPOSITORY https://github.com/DataDog/dd-sdk-cpp.git ' + 'GIT_TAG $_sha) # v1.4.0', + ); + }); + + test('re-pinning replaces the previous annotation rather than stacking ' + 'another one', () async { + final file = File(p.join(root.path, 'CMakeLists.txt')); + await file.writeAsString( + 'FetchContent_Declare(dd-sdk-cpp\n GIT_TAG $_sha) # v1.4.0\n', + ); + + await pinCppVersion(file, 'v1.5.0', _sha, logger, false); + + final contents = await file.readAsString(); + expect(contents, contains('GIT_TAG $_sha) # v1.5.0')); + expect(contents, isNot(contains('v1.4.0'))); + }); + test('leaves everything else in the file untouched', () async { final file = File(p.join(root.path, 'CMakeLists.txt')); await file.writeAsString(''' diff --git a/tools/releaser/test/git_history_test.dart b/tools/releaser/test/git_history_test.dart index 5889119e4..891918d1a 100644 --- a/tools/releaser/test/git_history_test.dart +++ b/tools/releaser/test/git_history_test.dart @@ -38,7 +38,7 @@ void main() { expect(tag, isNull); }); - test('releaseLine restricts the search to that major/minor, ignoring a ' + test('versionScope restricts the search to that major/minor, ignoring a ' 'newer tag from a different line', () async { fixture.writeFile('packages/datadog_dio/CHANGES', 'v2.0.0 work'); await fixture.commit('fix: something for 2.0.0'); @@ -53,7 +53,7 @@ void main() { final tag = await findLastReleaseTag( gitDir, 'datadog_dio', - releaseLine: (2, 0), + versionScope: (major: 2, minor: 0, patch: null), ); expect(tag, isNotNull); @@ -91,6 +91,26 @@ void main() { }, ); + test('a versionScope with a patch restricts to that exact version -- the ' + 'pre-release path, where only tags at the declared target can continue ' + 'its counter', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'beta work'); + await fixture.commit('feat: 4.0.0-beta.1'); + await fixture.tag('datadog_dio/v4.0.0-beta.1'); + // A patch off an older line, and a concurrent v5 effort -- both sort + // above the beta and would mask it without the patch component. + await fixture.tag('datadog_dio/v4.0.1'); + await fixture.tag('datadog_dio/v5.0.0-beta.1'); + + final tag = await findLastReleaseTag( + await fixture.gitDir, + 'datadog_dio', + versionScope: (major: 4, minor: 0, patch: 0), + ); + + expect(tag?.tag.tag, 'datadog_dio/v4.0.0-beta.1'); + }); + test( 'commitMessagesSince only returns commits after the given sha', () async { diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index 360dd01a2..5949b9309 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -590,6 +590,20 @@ void main() { expect(result.packages.single.newVersion, '4.0.0-beta.1'); }); + test('an unrelated higher tag does not mask the target line -- the counter ' + 'continues from the tag at the declared target version', () async { + // pubspec.version is 4.0.0. A patch off an older line and a + // concurrent v5 effort both sort above this line's own beta; + // selecting either would restart at beta.1 and collide. + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.1'); + await fixture.tag('datadog_flutter_plugin/v4.0.1'); + await fixture.tag('datadog_flutter_plugin/v5.0.0-beta.1'); + + final result = await plan(preReleaseCtx(prereleaseLabel: 'beta')); + + expect(result.packages.single.newVersion, '4.0.0-beta.2'); + }); + test('rejects starting a new pre-release once the target version has ' 'already been released stably', () async { // pubspec.version is 4.0.0, and it's already been released stably From b383158735bee70122d1062bd31aa5af86508a73 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Thu, 27 Aug 2026 10:08:59 -0400 Subject: [PATCH 09/10] Throw if BUMP_TYPE will be ignored. --- tools/releaser/lib/release_plan.dart | 34 ++++++++++++++++ tools/releaser/test/release_plan_test.dart | 45 ++++++++++++++++++++++ 2 files changed, 79 insertions(+) diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index 46ff22454..e1e229d06 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -115,6 +115,8 @@ Future computeReleasePlan( github.releaseExists(Logger('native_sdk'), repoSlug, version), ); + _validateTriggerInputs(ctx); + final groups = await _resolveGroups(ctx); final selected = _selectPackages(groups, ctx); @@ -133,6 +135,38 @@ Future computeReleasePlan( return ReleasePlan(trigger: ctx.trigger, packages: plans); } +/// Rejects per-run overrides that the trigger the run is happening under has +/// no way to honour, rather than accepting and silently ignoring them. +/// +/// Only the mainline path derives a bump level at all: a patch branch forces +/// `patch` by definition, and a pre-release branch's bump comes from the +/// prerelease counter. A `BUMP_TYPE` on either would read as "this release is +/// a major" and quietly not be. +void _validateTriggerInputs(RunContext ctx) { + final bumpType = ctx.bumpTypeOverride; + if (bumpType == null || bumpType.isEmpty) return; + + switch (ctx.trigger) { + case TriggerContext.mainline: + // Parsed (and rejected if unrecognized) where it's applied. + return; + case TriggerContext.patch: + throw StateError( + 'BUMP_TYPE="$bumpType" does not apply on a patch branch -- a patch ' + 'release always increments the patch level of its release line, and ' + 'a commit that would justify anything more is rejected outright. ' + 'Clear BUMP_TYPE, or release from develop instead.', + ); + case TriggerContext.preRelease: + throw StateError( + 'BUMP_TYPE="$bumpType" does not apply on a pre-release branch -- the ' + 'version comes from the prerelease counter against the target ' + 'declared in pubspec.yaml. Use PRERELEASE_LABEL to start a new label, ' + 'or bump pubspec.yaml to move to a new target version.', + ); + } +} + Future _computePackagePlan( DiscoveredPackage pkg, RunContext ctx, diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index 5949b9309..cad6989e4 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -451,6 +451,28 @@ void main() { currentBranch: branch, ); + test('BUMP_TYPE is rejected rather than silently ignored', () async { + // A patch branch always increments the patch level, so an override + // here would read as "this release is a major" and quietly not be. + await expectLater( + plan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.patch, + currentBranch: 'release/datadog_dio/v1.1.x', + bumpTypeOverride: 'major', + ), + ), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('does not apply on a patch branch'), + ), + ), + ); + }); + test('resolves the single named package with no grouping', () async { final result = await plan(patchCtx('release/datadog_dio/v1.1.x')); expect(result.packages, hasLength(1)); @@ -542,6 +564,29 @@ void main() { prereleaseLabel: prereleaseLabel, ); + test('BUMP_TYPE is rejected rather than silently ignored', () async { + // The version here comes from the prerelease counter, not a bump level. + await expectLater( + plan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.preRelease, + currentBranch: 'v4', + requestedPackages: ['datadog_flutter_plugin'], + prereleaseLabel: 'beta', + bumpTypeOverride: 'minor', + ), + ), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('does not apply on a pre-release branch'), + ), + ), + ); + }); + test('the first prerelease for a base version requires a label', () async { await expectLater(plan(preReleaseCtx()), throwsStateError); }); From 96a8c45bf7237ab337580b8f647fcb46c02c9602 Mon Sep 17 00:00:00 2001 From: Jeff Ward Date: Thu, 27 Aug 2026 15:30:19 -0400 Subject: [PATCH 10/10] More review fixes - Reject overriding the bump_type without an explicit packages list - Be more tolerant of spacing differences in native SDK files - Prevent backwords moves of pre-releases. --- tools/releaser/lib/cocoapod_util.dart | 33 +++++-- tools/releaser/lib/gradle_util.dart | 53 ++++++++-- tools/releaser/lib/native_sdk.dart | 13 ++- tools/releaser/lib/release_plan.dart | 65 ++++++++++-- tools/releaser/lib/spm_util.dart | 39 ++++++-- tools/releaser/test/cocoapod_util_test.dart | 48 +++++++++ tools/releaser/test/gradle_util_test.dart | 52 ++++++++++ tools/releaser/test/release_plan_test.dart | 104 ++++++++++++++++++-- tools/releaser/test/spm_util_test.dart | 59 +++++++++++ 9 files changed, 420 insertions(+), 46 deletions(-) create mode 100644 tools/releaser/test/cocoapod_util_test.dart create mode 100644 tools/releaser/test/gradle_util_test.dart create mode 100644 tools/releaser/test/spm_util_test.dart diff --git a/tools/releaser/lib/cocoapod_util.dart b/tools/releaser/lib/cocoapod_util.dart index 5c15cff68..3164b5bf5 100644 --- a/tools/releaser/lib/cocoapod_util.dart +++ b/tools/releaser/lib/cocoapod_util.dart @@ -12,6 +12,25 @@ import 'package_list.dart'; final overridesStartPattern = RegExp(r'\s+# Datadog Pod Overrides'); final overridesEndPattern = RegExp(r'\s+# End Datadog Pod Overrides'); +/// Rewrites a podspec's `s.dependency 'Datadog...'` constraint to [version], +/// or returns [line] unchanged if it isn't one. +/// +/// Replaces only the matched range rather than reconstructing the line. +/// [iosPodspecDependencyPattern] tolerates any spacing around the comma, so +/// rebuilding a canonical ` s.dependency 'X', 'Y'` reformats whatever +/// spacing and indentation the podspec actually used and drops anything +/// trailing the constraint. +String pinIosPodspecDependencyLine(String line, String version) { + final match = iosPodspecDependencyPattern.firstMatch(line); + if (match == null) return line; + + return line.replaceRange( + match.start, + match.end, + "${match.namedGroup('prefix')}$version'", + ); +} + class PinCocoapodsVersionCommand extends Command { @override Future run(CommandArguments args, Logger logger) async { @@ -81,14 +100,12 @@ class PinCocoapodsVersionCommand extends Command { } logger.info('ℹ️ Setting the iOS Pod Dependency to ${args.iOSRelease}'); - await transformFile(file, logger, args.dryRun, (element) { - final match = iosPodspecDependencyPattern.firstMatch(element); - if (match != null) { - element = - " s.dependency '${match.namedGroup('dependency')}', '${args.iOSRelease}'"; - } - return element; - }); + await transformFile( + file, + logger, + args.dryRun, + (element) => pinIosPodspecDependencyLine(element, args.iOSRelease!), + ); return true; } diff --git a/tools/releaser/lib/gradle_util.dart b/tools/releaser/lib/gradle_util.dart index 2fac8dfa1..bf283132c 100644 --- a/tools/releaser/lib/gradle_util.dart +++ b/tools/releaser/lib/gradle_util.dart @@ -8,6 +8,25 @@ import 'helpers.dart'; import 'native_sdk.dart'; import 'package_list.dart'; +/// Rewrites a `build.gradle`'s `ext.datadog_version` assignment to [version], +/// or returns [line] unchanged if it isn't that assignment. +/// +/// Rebuilt from the match rather than from a literal `ext.datadog_version = +/// "..."`: [androidGradleVersionPattern] tolerates any spacing around the `=`, +/// so a literal would match a line and then silently replace nothing in it, +/// leaving the constraint unpinned with no error. Replacing the matched range +/// also keeps the line's own spacing and indentation. +String pinAndroidGradleVersionLine(String line, String version) { + final match = androidGradleVersionPattern.firstMatch(line); + if (match == null) return line; + + return line.replaceRange( + match.start, + match.end, + '${match.namedGroup('prefix')}$version"', + ); +} + class UpdateGradleFilesCommand extends Command { @override Future run(CommandArguments args, Logger logger) async { @@ -19,6 +38,28 @@ class UpdateGradleFilesCommand extends Command { } Future _updateGradleFiles(CommandArguments args, Logger logger) async { + // Resolved by ValidateReleaseCommand -- but only for packages that pass + // hasNativeDependency(), which is a stale two-name list. Releasing + // anything else skips that validation entirely and leaves this null while + // still returning success, so it has to be guarded here rather than + // interpolated: this loop walks a hardcoded gradleList and rewrites any + // datadog_flutter_plugin path regardless of what's actually being + // released, so an unguarded null wrote a literal + // `ext.datadog_version = "null"` into a package that need not even be part + // of the release -- and the next CommitChangesCommand shipped it. + // + // Leaving the pin floating is the safe failure: it's what develop already + // carries, and it shows up in the release diff as "nothing changed" + // instead of as a corrupted constraint. + final androidRelease = args.androidRelease; + if (androidRelease == null) { + logger.warning( + '⚠️ No Android SDK version was resolved for this release -- leaving ' + 'ext.datadog_version alone. Pass --android-version if this release is ' + 'meant to move the Android SDK pin.', + ); + } + for (var filePath in gradleList) { final file = File(path.join(args.gitDir.path, filePath)); if (!file.existsSync()) { @@ -33,15 +74,9 @@ class UpdateGradleFilesCommand extends Command { bool writeMavenBlock = true; await transformFile(file, logger, args.dryRun, (line) { // For the datadog_flutter_plugin, use a tighter constraint - if (file.path.contains('datadog_flutter_plugin')) { - final versionMatch = androidGradleVersionPattern.firstMatch(line); - if (versionMatch != null) { - final oldVersion = versionMatch.namedGroup('version'); - line = line.replaceFirst( - '$androidGradleVersionPrefix = "$oldVersion"', - '$androidGradleVersionPrefix = "${args.androidRelease}"', - ); - } + if (androidRelease != null && + file.path.contains('datadog_flutter_plugin')) { + line = pinAndroidGradleVersionLine(line, androidRelease); } // Remove requests for external gradle files diff --git a/tools/releaser/lib/native_sdk.dart b/tools/releaser/lib/native_sdk.dart index fd73f27ce..e88e7f9ae 100644 --- a/tools/releaser/lib/native_sdk.dart +++ b/tools/releaser/lib/native_sdk.dart @@ -24,8 +24,12 @@ enum NativeSdk { /// public so `cocoapod_util.dart`'s pin-rewriting step and this file's /// discovery share one definition of the line shape instead of maintaining /// two separate regexes for it. +/// [prefix] spans everything up to and including the constraint's opening +/// quote, so a rewrite can rebuild the line from the match instead of +/// reconstructing it -- see [pinIosPodspecDependencyLine]. final iosPodspecDependencyPattern = RegExp( - r"s\.dependency\s+'(?Datadog\w*)'\s*,\s*'(?[^']+)'", + r"(?s\.dependency\s+'(?Datadog\w*)'\s*,\s*')" + r"(?[^']+)'", ); /// The `ext.datadog_version = "..."` line prefix in a `build.gradle`, and @@ -36,8 +40,13 @@ final iosPodspecDependencyPattern = RegExp( /// Discovery below uses the same pattern to decide whether a `build.gradle` /// is an Android native-dependency file at all. const androidGradleVersionPrefix = 'ext.datadog_version'; + +/// [prefix] spans everything up to and including the opening quote, so a +/// rewrite can rebuild the assignment from the match rather than from a +/// literal -- reproducing a literal `x = "y"` would silently fail to replace +/// a line the pattern happily matched with different spacing. final androidGradleVersionPattern = RegExp( - '$androidGradleVersionPrefix\\s*=\\s*"(?[^"]+)"', + '(?$androidGradleVersionPrefix\\s*=\\s*")(?[^"]+)"', ); /// Matches a `Package.swift`'s dd-sdk-ios dependency line (e.g. diff --git a/tools/releaser/lib/release_plan.dart b/tools/releaser/lib/release_plan.dart index e1e229d06..b4aea9d3a 100644 --- a/tools/releaser/lib/release_plan.dart +++ b/tools/releaser/lib/release_plan.dart @@ -135,20 +135,37 @@ Future computeReleasePlan( return ReleasePlan(trigger: ctx.trigger, packages: plans); } -/// Rejects per-run overrides that the trigger the run is happening under has -/// no way to honour, rather than accepting and silently ignoring them. +/// Rejects per-run overrides the run has no coherent way to honour, rather +/// than accepting and silently reinterpreting them. /// /// Only the mainline path derives a bump level at all: a patch branch forces /// `patch` by definition, and a pre-release branch's bump comes from the /// prerelease counter. A `BUMP_TYPE` on either would read as "this release is /// a major" and quietly not be. +/// +/// And even on mainline it requires an explicit `PACKAGES`. "Override the +/// computed bump" is only a meaningful instruction about packages the caller +/// named: combined with `--all` it silently re-levels whatever happened to +/// qualify this run, so a single `fix:` typo ships as a major. Nobody typing +/// `BUMP_TYPE=major` means "and also major-release everything else that has a +/// commit", so that combination is an error rather than something to make +/// safe. void _validateTriggerInputs(RunContext ctx) { final bumpType = ctx.bumpTypeOverride; if (bumpType == null || bumpType.isEmpty) return; switch (ctx.trigger) { case TriggerContext.mainline: - // Parsed (and rejected if unrecognized) where it's applied. + if (ctx.requestedPackages.isEmpty) { + throw StateError( + 'BUMP_TYPE="$bumpType" requires an explicit PACKAGES list -- it ' + 'applies uniformly to every package in the run, so on an --all run ' + 'it would re-level whichever packages happened to qualify, turning ' + 'an unrelated fix into a $bumpType release. Name the packages this ' + 'bump is for, or clear BUMP_TYPE and let the commits decide.', + ); + } + // Otherwise parsed (and rejected if unrecognized) where it's applied. return; case TriggerContext.patch: throw StateError( @@ -202,11 +219,21 @@ Future _computePackagePlan( // touches the network -- resolving native SDK targets for a package that // turns out to have nothing to ship is pure waste. // - // Eligibility must come from the package's actual changes, never from - // BUMP_TYPE alone: a targeted override like BUMP_TYPE=major would otherwise - // sweep every discovered package into a major release of the entire repo. - // Patch and pre-release runs release whatever they were pointed at. - if (ctx.trigger == TriggerContext.mainline && + // Eligibility comes from the package's actual changes. (BUMP_TYPE can't + // reach here on its own -- _validateTriggerInputs requires an explicit + // PACKAGES alongside it, which makes every package in such a run + // isExplicitlyRequested.) + // + // This applies to pre-release `--all` runs too, not just mainline. Without + // it, every untouched package is handed a fresh `-beta.1`, and -- worse -- + // omitting PRERELEASE_LABEL to continue an existing counter aborts the whole + // plan on the first package that has never been part of the pre-release + // line. A package's first-ever alpha still qualifies: with no tag at the + // target version its commit range is the package's whole history. + // + // A patch run is exempt: its single package comes from the branch name, and + // a patch branch exists precisely because something needs shipping from it. + if (ctx.trigger != TriggerContext.patch && aggregateBumpLevel(commits) == null && !isExplicitlyRequested && !_hasForcedNativeUpdate(files, ctx)) { @@ -326,6 +353,28 @@ PackagePlan _computePrereleasePlan( ); } + // Whatever the branches above decided, a release has to move forward. + // + // Asserted as an invariant rather than enumerated as another case, because + // the ways to go backwards outnumber the ways to go forwards: labels are + // compared lexically by semver, so `beta` after `rc.1` restarts at + // `beta.1` -- already published, and below the latest release. Worse, it + // doesn't self-correct: `rc.1` stays the highest tag, so every subsequent + // run proposes that same `beta.1` again. + // + // A forward label change is still fine (`beta.3` -> `rc.1`); only a + // non-increasing result is rejected. + if (lastTag != null && newVersion <= lastTag.version) { + throw StateError( + 'Pre-release $newVersion for "${pkg.name}" would not move forward from ' + '${lastTag.tag.tag}. Pre-release labels are ordered lexically ' + '(alpha < beta < rc), so a label earlier than the one already shipped ' + 'restarts below it. Continue with a label that sorts after ' + '"${lastTag.version.preRelease.first}", or bump pubspec.yaml to start a ' + 'new target version.', + ); + } + return PackagePlan( package: pkg, currentVersion: pkg.version, diff --git a/tools/releaser/lib/spm_util.dart b/tools/releaser/lib/spm_util.dart index c984057da..ef073204a 100644 --- a/tools/releaser/lib/spm_util.dart +++ b/tools/releaser/lib/spm_util.dart @@ -11,7 +11,29 @@ import 'command.dart'; import 'helpers.dart'; import 'native_sdk.dart'; -const datadogIosRepo = 'https://github.com/Datadog/dd-sdk-ios.git'; +/// Rewrites a `Package.swift`'s dd-sdk-ios dependency to [versionString] (a +/// full SPM spec such as `exact: "3.13.0"`), or returns [line] unchanged if it +/// isn't that dependency. +/// +/// Keeps the file's own URL rather than comparing against one canonical +/// spelling: [iosSpmDependencyPattern] already guarantees this is a dd-sdk-ios +/// dependency, and `DataDog` vs `Datadog` casing genuinely varies across this +/// repo's manifests. Gating on an exact match meant discovery would report +/// such a manifest while the rewrite silently skipped it. +/// +/// Replacing only the matched range preserves the line's indentation and +/// anything after the call -- a trailing comma between array elements, most +/// commonly -- instead of reconstructing them. +String pinSpmDependencyLine(String line, String versionString) { + final match = iosSpmDependencyPattern.firstMatch(line); + if (match == null) return line; + + return line.replaceRange( + match.start, + match.end, + '.package(url: "${match.namedGroup('url')}", $versionString)', + ); +} class PinSwiftPackageVersion extends Command { @override @@ -64,15 +86,12 @@ class PinSwiftPackageVersion extends Command { } logger.info('ℹ️ Setting the iOS Pod Dependency to $versionString'); - await transformFile(file, logger, dryRun, (line) { - final match = iosSpmDependencyPattern.firstMatch(line); - if (match != null && match.namedGroup('url') == datadogIosRepo) { - final needsComma = line.trimRight().endsWith(','); - line = - ' .package(url: "$datadogIosRepo", $versionString)${needsComma ? ',' : ''}'; - } - return line; - }); + await transformFile( + file, + logger, + dryRun, + (line) => pinSpmDependencyLine(line, versionString), + ); return true; } diff --git a/tools/releaser/test/cocoapod_util_test.dart b/tools/releaser/test/cocoapod_util_test.dart new file mode 100644 index 000000000..d9d6e5b0e --- /dev/null +++ b/tools/releaser/test/cocoapod_util_test.dart @@ -0,0 +1,48 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:releaser/cocoapod_util.dart'; +import 'package:test/test.dart'; + +void main() { + test('pins the constraint, preserving indentation', () { + expect( + pinIosPodspecDependencyLine( + " s.dependency 'DatadogCore', '~> 3'", + '3.13.0', + ), + " s.dependency 'DatadogCore', '3.13.0'", + ); + }); + + test('keeps the podspec\'s own spacing around the comma', () { + // datadog_inappwebview_tracking writes it this way. The old pattern + // required a single space and never matched the line at all; now that it + // does, the rewrite must not reformat what it matched. + expect( + pinIosPodspecDependencyLine( + " s.dependency 'DatadogCore', '~> 3.0'", + '3.13.0', + ), + " s.dependency 'DatadogCore', '3.13.0'", + ); + }); + + test('leaves anything after the constraint alone', () { + expect( + pinIosPodspecDependencyLine( + " s.dependency 'DatadogCore', '~> 3' # floating on develop", + '3.13.0', + ), + " s.dependency 'DatadogCore', '3.13.0' # floating on develop", + ); + }); + + test('leaves a non-Datadog dependency untouched', () { + const line = " s.dependency 'Flutter'"; + expect(pinIosPodspecDependencyLine(line, '3.13.0'), line); + const other = " s.dependency 'DictionaryCoder', '1.2.0'"; + expect(pinIosPodspecDependencyLine(other, '3.13.0'), other); + }); +} diff --git a/tools/releaser/test/gradle_util_test.dart b/tools/releaser/test/gradle_util_test.dart new file mode 100644 index 000000000..265f73537 --- /dev/null +++ b/tools/releaser/test/gradle_util_test.dart @@ -0,0 +1,52 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:releaser/gradle_util.dart'; +import 'package:test/test.dart'; + +void main() { + test('pins the version, preserving indentation', () { + expect( + pinAndroidGradleVersionLine(' ext.datadog_version = "3+"', '3.11.0'), + ' ext.datadog_version = "3.11.0"', + ); + }); + + test('pins regardless of the spacing around the assignment', () { + // The pattern tolerates any spacing, so the rewrite has to as well -- + // rebuilding a literal `ext.datadog_version = "..."` matched these lines + // and then silently replaced nothing in them. + expect( + pinAndroidGradleVersionLine( + ' ext.datadog_version = "3+"', + '3.11.0', + ), + ' ext.datadog_version = "3.11.0"', + ); + expect( + pinAndroidGradleVersionLine(' ext.datadog_version="3+"', '3.11.0'), + ' ext.datadog_version="3.11.0"', + ); + }); + + test('leaves anything after the assignment alone', () { + expect( + pinAndroidGradleVersionLine( + ' ext.datadog_version = "3+" // floating on develop', + '3.11.0', + ), + ' ext.datadog_version = "3.11.0" // floating on develop', + ); + }); + + test('leaves an unrelated line untouched', () { + expect( + pinAndroidGradleVersionLine( + ' ext.kotlin_version = "2.2.20"', + '3.11.0', + ), + ' ext.kotlin_version = "2.2.20"', + ); + }); +} diff --git a/tools/releaser/test/release_plan_test.dart b/tools/releaser/test/release_plan_test.dart index cad6989e4..72ce44984 100644 --- a/tools/releaser/test/release_plan_test.dart +++ b/tools/releaser/test/release_plan_test.dart @@ -159,16 +159,41 @@ void main() { expect(names, isNot(contains('datadog_flutter_plugin_ios'))); }); - test('BUMP_TYPE alone does not sweep an otherwise-unqualifying package ' - 'into --all', () async { - // No qualifying commits, no native SDK change, not explicitly - // requested -- BUMP_TYPE must not be the thing that grants - // eligibility here, or a targeted override would release every - // discovered package. - final result = await plan(mainlineCtx(bumpTypeOverride: 'major')); - final names = result.packages.map((p) => p.package.name); + test('BUMP_TYPE without an explicit PACKAGES list is rejected', () async { + // "Override the computed bump" only means something about packages the + // caller named. On an --all run it would re-level whatever qualified -- + // a lone `fix:` typo shipping as a major. + fixture.writeFile('packages/datadog_dio/CHANGES', 'a typo fix'); + await fixture.commit('fix: correct a typo'); - expect(names, isNot(contains('lonely_ios'))); + await expectLater( + plan(mainlineCtx(bumpTypeOverride: 'major')), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('requires an explicit PACKAGES list'), + ), + ), + ); + }); + + test('BUMP_TYPE applies to the named packages only', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'a typo fix'); + await fixture.commit('fix: correct a typo'); + await fixture.tag('datadog_dio/v2.3.0'); + fixture.writeFile('packages/datadog_dio/CHANGES', 'another'); + await fixture.commit('fix: correct another typo'); + + final result = await plan( + mainlineCtx( + requestedPackages: ['datadog_dio'], + bumpTypeOverride: 'major', + ), + ); + + expect(result.packages.single.package.name, 'datadog_dio'); + expect(result.packages.single.newVersion, '3.0.0'); }); }); @@ -587,6 +612,47 @@ void main() { ); }); + test('--all excludes packages with nothing to ship', () async { + fixture.writeFile('packages/datadog_dio/CHANGES', 'a real feature'); + await fixture.commit('feat: add a real feature to dio'); + + final result = await plan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.preRelease, + currentBranch: 'v4', + prereleaseLabel: 'beta', + ), + ); + final names = result.packages.map((p) => p.package.name); + + expect(names, contains('datadog_dio')); + // Untouched -- handing it a fresh beta would publish a release nobody + // asked for. + expect(names, isNot(contains('lonely_ios'))); + }); + + test("--all without a label doesn't abort on a package that was never part " + 'of the pre-release line', () async { + // Omitting PRERELEASE_LABEL to continue an existing counter is a + // documented workflow; an untouched package with no tag at the target + // version used to throw and take the whole plan down with it. + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.1'); + + final result = await plan( + RunContext( + repoRoot: fixture.root.path, + trigger: TriggerContext.preRelease, + currentBranch: 'v4', + ), + ); + + expect( + result.packages.map((p) => p.package.name), + isNot(contains('lonely_ios')), + ); + }); + test('the first prerelease for a base version requires a label', () async { await expectLater(plan(preReleaseCtx()), throwsStateError); }); @@ -616,6 +682,26 @@ void main() { }, ); + test('a label that would move the version backward is rejected', () async { + // Labels are ordered lexically, so `beta` after `rc.1` restarts at + // `beta.1` -- already published, and below the latest release. It + // also never self-corrects: `rc.1` stays the highest tag, so every + // later run proposes that same `beta.1` again. + await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.1'); + await fixture.tag('datadog_flutter_plugin/v4.0.0-rc.1'); + + await expectLater( + plan(preReleaseCtx(prereleaseLabel: 'beta')), + throwsA( + isA().having( + (e) => e.message, + 'message', + contains('would not move forward from'), + ), + ), + ); + }); + test('switching to a new label restarts the counter at .1', () async { await fixture.tag('datadog_flutter_plugin/v4.0.0-beta.3'); diff --git a/tools/releaser/test/spm_util_test.dart b/tools/releaser/test/spm_util_test.dart new file mode 100644 index 000000000..b7d247292 --- /dev/null +++ b/tools/releaser/test/spm_util_test.dart @@ -0,0 +1,59 @@ +// Unless explicitly stated otherwise all files in this repository are licensed under the Apache License Version 2.0. +// This product includes software developed at Datadog (https://www.datadoghq.com/). +// Copyright 2019-Present Datadog, Inc. + +import 'package:releaser/spm_util.dart'; +import 'package:test/test.dart'; + +const _pin = 'exact: "3.13.0"'; + +void main() { + test('pins a from: constraint, preserving indentation', () { + expect( + pinSpmDependencyLine( + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: "3.0.0")', + _pin, + ), + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", $_pin)', + ); + }); + + test('pins a branch-tracking dependency', () { + expect( + pinSpmDependencyLine( + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", branch: "develop")', + _pin, + ), + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", $_pin)', + ); + }); + + test('keeps a trailing comma between array elements', () { + expect( + pinSpmDependencyLine( + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", from: "3.0.0"),', + _pin, + ), + ' .package(url: "https://github.com/Datadog/dd-sdk-ios.git", $_pin),', + ); + }); + + test('keeps the manifest\'s own URL rather than one canonical spelling', () { + // `DataDog` and `Datadog` both appear across this repo's manifests. + // Gating the rewrite on an exact match let discovery report a manifest + // that the rewrite then silently skipped. + expect( + pinSpmDependencyLine( + ' .package(url: "https://github.com/DataDog/dd-sdk-ios.git", from: "3.0.0")', + _pin, + ), + ' .package(url: "https://github.com/DataDog/dd-sdk-ios.git", $_pin)', + ); + }); + + test('leaves a non-Datadog dependency untouched', () { + const line = + ' .package(url: "https://github.com/other/pkg.git", from: "1.0.0")'; + expect(pinSpmDependencyLine(line, _pin), line); + }); +}