Skip to content

Simplify build-trace gate; remove dynamic subcommand probe #3709

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Draft
eseidel wants to merge 2 commits into
base: main
Choose a base branch
from
+110 −199
Draft

Simplify build-trace gate; remove dynamic subcommand probe #3709

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
43c99e3
fix(cli): gate build tracing off for unknown Flutter versions
eseidel Apr 21, 2026
838924a
Revert "fix: only pass --shorebird-trace to build commands that suppo…
eseidel Apr 21, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
71 changes: 22 additions & 49 deletions packages/shorebird_cli/lib/src/artifact_builder/artifact_builder.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -103,44 +103,6 @@ fails when using the same flutter version, please file an issue:
${link(uri: Uri.parse('https://github.com/shorebirdtech/shorebird/issues/new'))}
''';

/// Cache of `flutter build <command>` help output checks for
/// `--shorebird-trace` support. Populated lazily by
/// [_supportsTraceFlag].
final _traceSupport = <String, bool>{};

/// Returns whether `flutter build <command>` accepts `--shorebird-trace`.
///
/// Probes the command's help output and caches the result per [command]
/// so that subsequent calls for the same command are free.
/// Returns `false` if the help check fails for any reason.
Future<bool> _supportsTraceFlag(String command) async {
if (_traceSupport.containsKey(command)) return _traceSupport[command]!;

try {
final result = await process.run(
'flutter',
['build', command, '-h'],
runInShell: false,
);
final supported = result.stdout.toString().contains('--shorebird-trace');
_traceSupport[command] = supported;
return supported;
} on Exception {
_traceSupport[command] = false;
return false;
}
}

/// Returns the `--shorebird-trace` argument if the current
/// [BuildTraceSession] has a trace file and the given [command]
/// supports it, or an empty list otherwise.
Future<List<String>> _traceArgs(String command) async {
final traceFile = buildTraceSession.traceFile;
if (traceFile == null) return const [];
if (!await _supportsTraceFlag(command)) return const [];
return ['--shorebird-trace=${traceFile.path}'];
}

/// Builds an aab using `flutter build appbundle`. Runs `flutter pub get` with
/// the system installation of Flutter to reset
/// `.dart_tool/package_config.json` after the build completes or fails.
Expand All @@ -154,14 +116,15 @@ ${link(uri: Uri.parse('https://github.com/shorebirdtech/shorebird/issues/new'))}
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final targetPlatformArgs = targetPlatforms?.targetPlatformArg;
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'appbundle',
'--release',
if (flavor != null) '--flavor=$flavor',
if (target != null) '--target=$target',
if (targetPlatformArgs != null) '--target-platform=$targetPlatformArgs',
...await _traceArgs('appbundle'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -221,6 +184,7 @@ Reason: Exited with code $exitCode.''',
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final targetPlatformArgs = targetPlatforms?.targetPlatformArg;
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'apk',
Expand All @@ -233,7 +197,7 @@ Reason: Exited with code $exitCode.''',
// coverage:ignore-start
if (splitPerAbi) '--split-per-abi',
// coverage:ignore-end
...await _traceArgs('apk'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -290,14 +254,15 @@ Reason: Exited with code $exitCode.''',
return _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final targetPlatformArgs = targetPlatforms?.targetPlatformArg;
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'aar',
'--no-debug',
'--no-profile',
'--build-number=$buildNumber',
if (targetPlatformArgs != null) '--target-platform=$targetPlatformArgs',
...await _traceArgs('aar'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -334,12 +299,13 @@ Reason: Exited with code $exitCode.''',
}) async {
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'linux',
'--release',
if (target != null) '--target=$target',
...await _traceArgs('linux'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -387,14 +353,15 @@ Reason: Exited with code $exitCode.''',
String? appDillPath;
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'macos',
'--release',
if (flavor != null) '--flavor=$flavor',
if (target != null) '--target=$target',
if (!codesign) '--no-codesign',
...await _traceArgs('macos'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];
final buildStart = clock.now();
Expand Down Expand Up @@ -454,14 +421,15 @@ Reason: Exited with code $exitCode.''',
String? appDillPath;
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'ipa',
'--release',
if (flavor != null) '--flavor=$flavor',
if (target != null) '--target=$target',
if (!codesign) '--no-codesign',
...await _traceArgs('ipa'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -517,12 +485,13 @@ Reason: Exited with code $exitCode.''',
String? appDillPath;
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'ios-framework',
'--no-debug',
'--no-profile',
...await _traceArgs('ios-framework'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down Expand Up @@ -582,10 +551,13 @@ Reason: Exited with code $exitCode.''',
final flutterVersion = await shorebirdFlutter.resolveFlutterVersion(
revision,
);
// Treat an unknown version (e.g. a pinned dev revision) as new enough,
// matching the pattern used for other version-gated features.
// When resolveFlutterVersion returns null (dev revision not on any
// flutter_release/* branch), the gate falls through to the allowlist.
// That's intentional: a dev pin predating the tracing PR would otherwise
// get `--shorebird-trace` passed to a flutter build that doesn't know
// the flag, and fail hard.
final supportsTrace = buildTraceSupportConstraint.isSatisfiedBy(
version: flutterVersion ?? buildTraceSupportConstraint.minVersion,
version: flutterVersion,
revision: revision,
);
if (!supportsTrace) {
Expand Down Expand Up @@ -793,12 +765,13 @@ Either run `flutter pub get` manually, or follow the steps in ${cannotRunInVSCod
}) async {
await _runShorebirdBuildCommand(() async {
const executable = 'flutter';
final traceFile = buildTraceSession.traceFile;
final arguments = [
'build',
'windows',
'--release',
if (target != null) '--target=$target',
...await _traceArgs('windows'),
if (traceFile != null) '--shorebird-trace=${traceFile.path}',
...args,
];

Expand Down
23 changes: 18 additions & 5 deletions packages/shorebird_cli/lib/src/flutter_version_constraints.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,15 @@ class FlutterSupportConstraint {
final Set<String> allowedRevisions;

/// Whether the given [version]/[revision] pair satisfies this constraint.
bool isSatisfiedBy({required Version version, required String revision}) =>
version >= minVersion || allowedRevisions.contains(revision);
///
/// A null [version] (e.g. a Shorebird-fork dev revision that isn't on any
/// `flutter_release/*` branch and so has no parseable upstream version)
/// can only satisfy the constraint via [allowedRevisions]. Gating on
/// `version ?? minVersion` would treat every unknown revision as new
/// enough, silently opting pre-feature dev pins into feature behavior.
bool isSatisfiedBy({required Version? version, required String revision}) =>
(version != null && version >= minVersion) ||
allowedRevisions.contains(revision);
}

/// Flutter support for `flutter build --shorebird-trace=<path>` for emitting
Expand All @@ -76,9 +83,15 @@ class FlutterSupportConstraint {
final buildTraceSupportConstraint = FlutterSupportConstraint(
minVersion: Version(3, 41, 7),
allowedRevisions: {
// Current Shorebird Flutter pin (bin/internal/flutter.version). Can
// be removed once a flutter_release/3.41.7 branch ships with this
// (or a later tracing-enabled) commit at its tip.
// Shorebird-fork Flutter pins that ship the tracing feature but
// still report upstream version 3.41.6, so `resolveFlutterVersion`
// can't satisfy the floor via the version path.
//
// Add a new entry here every time `bin/internal/flutter.version`
// is bumped to a revision that carries the tracing PRs; entries
// stop mattering once a `flutter_release/3.41.7` branch ships
// them and the floor can be satisfied directly.
'3b10eecea184bb381f1045a878eeff36548ed11e',
'c2c56ab2d5483bdf86152725342f55ca6faed946',
},
);
Loading