From 27f6e7385d7c6fc078c867d60e5abe1acf050dae Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Mon, 27 Jan 2025 16:59:06 -0500 Subject: [PATCH] fix: tighten Flutter revisions for macOS releasing and patching (#2808) --- cspell.config.yaml | 2 + .../lib/src/commands/patch/macos_patcher.dart | 7 +- .../shorebird_cli/lib/src/platform/apple.dart | 12 +- .../commands/patch/macos_patcher_test.dart | 115 +++++++++++++----- 4 files changed, 102 insertions(+), 34 deletions(-) diff --git a/cspell.config.yaml b/cspell.config.yaml index b3e91598..ad4be560 100644 --- a/cspell.config.yaml +++ b/cspell.config.yaml @@ -68,6 +68,7 @@ words: - nserror - Oltman - orri # Arm64 instruction, Or Register with Immediate + - patchability - pana - pkcs - podfile @@ -107,6 +108,7 @@ words: - unmockable - Unpatchable - unsets + - unskipped - upvote - usbmuxd - USERPROFILE diff --git a/packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart b/packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart index 1ada3791..3bf9bd7b 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/macos_patcher.dart @@ -143,8 +143,11 @@ This may indicate that the patch contains native changes, which cannot be applie shorebirdFlutter.getVersion(), ).wait; - if ((flutterVersion ?? minimumSupportedMacosFlutterVersion) < - minimumSupportedMacosFlutterVersion) { + // TODO(bryanoltman): revert this to checking the flutter version once + // the minimum supported version is updated. + if (!patchableMacosFlutterRevisions.contains( + shorebirdEnv.flutterRevision, + )) { logger.err( ''' macOS patches are not supported with Flutter versions older than $minimumSupportedMacosFlutterVersion. diff --git a/packages/shorebird_cli/lib/src/platform/apple.dart b/packages/shorebird_cli/lib/src/platform/apple.dart index e03c5938..756b3f57 100644 --- a/packages/shorebird_cli/lib/src/platform/apple.dart +++ b/packages/shorebird_cli/lib/src/platform/apple.dart @@ -99,7 +99,17 @@ Please report issues at https://github.com/shorebirdtech/shorebird/issues/new final minimumSupportedIosFlutterVersion = Version(3, 22, 2); /// The minimum allowed Flutter version for creating macOS releases. -final minimumSupportedMacosFlutterVersion = Version(3, 27, 0); +final minimumSupportedMacosFlutterVersion = Version(3, 27, 3); + +/// Flutter revisions that support macOS patching. +/// This is a temporary workaround. Some revisions of 3.27.3 (those included +/// here) support our current (unlinked) method of patching macOS releases, +/// while the rest do not. +// TODO(bryanoltman): remove this when we can bump the minimum macOS version to +// 3.27.4 or higher. +const patchableMacosFlutterRevisions = { + '5c1dcc19ebcee3565c65262dd95970186e4d81cc', +}; /// A reference to a [Apple] instance. final appleRef = create(Apple.new); diff --git a/packages/shorebird_cli/test/src/commands/patch/macos_patcher_test.dart b/packages/shorebird_cli/test/src/commands/patch/macos_patcher_test.dart index fd3a26ed..08b05995 100644 --- a/packages/shorebird_cli/test/src/commands/patch/macos_patcher_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/macos_patcher_test.dart @@ -150,6 +150,9 @@ void main() { when( () => shorebirdEnv.getShorebirdProjectRoot(), ).thenReturn(projectRoot); + when( + () => shorebirdEnv.flutterRevision, + ).thenReturn('5c1dcc19ebcee3565c65262dd95970186e4d81cc'); appDirectory = Directory( p.join( @@ -508,42 +511,92 @@ This may indicate that the patch contains native changes, which cannot be applie ).thenAnswer((_) async => Version(3, 27, 3)); }); - group('when specified flutter version is less than minimum', () { - setUp(() { - when( - () => shorebirdValidator.validatePreconditions( - checkUserIsAuthenticated: any( - named: 'checkUserIsAuthenticated', + group( + 'when specified flutter version is less than minimum', + () { + setUp(() { + when( + () => shorebirdValidator.validatePreconditions( + checkUserIsAuthenticated: any( + named: 'checkUserIsAuthenticated', + ), + checkShorebirdInitialized: any( + named: 'checkShorebirdInitialized', + ), + validators: any(named: 'validators'), + supportedOperatingSystems: any( + named: 'supportedOperatingSystems', + ), ), - checkShorebirdInitialized: any( - named: 'checkShorebirdInitialized', - ), - validators: any(named: 'validators'), - supportedOperatingSystems: any( - named: 'supportedOperatingSystems', - ), - ), - ).thenAnswer((_) async {}); - when( - () => shorebirdFlutter.getVersion(), - ).thenAnswer((_) async => Version(3, 0, 0)); - }); + ).thenAnswer((_) async {}); + when( + () => shorebirdFlutter.getVersion(), + ).thenAnswer((_) async => Version(3, 0, 0)); + }); - test('logs error and exits with code 70', () async { - await expectLater( - () => runWithOverrides(patcher.buildPatchArtifact), - exitsWithCode(ExitCode.software), - ); + test('logs error and exits with code 70', () async { + await expectLater( + () => runWithOverrides(patcher.buildPatchArtifact), + exitsWithCode(ExitCode.software), + ); - verify( - () => logger.err( - ''' + verify( + () => logger.err( + ''' macOS patches are not supported with Flutter versions older than $minimumSupportedMacosFlutterVersion. For more information see: ${supportedFlutterVersionsUrl.toLink()}''', - ), - ).called(1); - }); - }); + ), + ).called(1); + }); + }, + skip: + '''Skipping while we use the flutter revision when checking for patchability''', + ); + + // TODO(bryanoltman): this test can be removed (and the above test + // can be unskipped) when we can increase the + // minimumSupportedMacosVersion to 3.27.4 or higher. + group( + 'when release flutter version is not in the allowlist', + () { + setUp(() { + when( + () => shorebirdValidator.validatePreconditions( + checkUserIsAuthenticated: any( + named: 'checkUserIsAuthenticated', + ), + checkShorebirdInitialized: any( + named: 'checkShorebirdInitialized', + ), + validators: any(named: 'validators'), + supportedOperatingSystems: any( + named: 'supportedOperatingSystems', + ), + ), + ).thenAnswer((_) async {}); + when( + () => shorebirdEnv.flutterRevision, + ).thenReturn('not-a-supported-revision'); + }); + + test('logs error and exits with code 70', () async { + await expectLater( + () => runWithOverrides(patcher.buildPatchArtifact), + exitsWithCode(ExitCode.software), + ); + + verify( + () => logger.err( + ''' +macOS patches are not supported with Flutter versions older than $minimumSupportedMacosFlutterVersion. +For more information see: ${supportedFlutterVersionsUrl.toLink()}''', + ), + ).called(1); + }); + }, + skip: + '''Skipping while we use the flutter revision when checking for patchability''', + ); group('when build fails with ProcessException', () { setUp(() {