From 65ccd95f68c8ef954da51d6b981582d0f44fd2bd Mon Sep 17 00:00:00 2001 From: Bryan Oltman Date: Mon, 11 Mar 2024 16:11:31 -0400 Subject: [PATCH] fix: only build patch artifacts once if release version is specified and flutter version is not current (#1785) --- .../commands/patch/patch_android_command.dart | 37 +++++--- .../src/commands/patch/patch_ios_command.dart | 87 +++++++++++++------ .../patch/patch_android_command_test.dart | 41 ++++++++- .../patch/patch_ios_command_test.dart | 59 +++++++++++++ 4 files changed, 187 insertions(+), 37 deletions(-) diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart index 976ff3ac..4e1f1f66 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_android_command.dart @@ -112,19 +112,11 @@ If this option is not provided, the version number will be determined from the p final shorebirdYaml = shorebirdEnv.getShorebirdYaml()!; final appId = shorebirdYaml.getAppId(flavor: flavor); final app = await codePushClientWrapper.getApp(appId: appId); - final originalFlutterRevision = shorebirdEnv.flutterRevision; - final buildProgress = logger.progress('Building patch'); - try { - await buildAppBundle(flavor: flavor, target: target); - buildProgress.complete(); - } on ProcessException catch (error) { - buildProgress.fail('Failed to build: ${error.message}'); - return ExitCode.software.code; - } + + var hasBuiltWithLatestFlutter = false; final projectRoot = shorebirdEnv.getShorebirdProjectRoot()!; - final bundleDirPath = p.join( projectRoot.path, 'build', @@ -143,6 +135,17 @@ If this option is not provided, the version number will be determined from the p releaseVersion = argReleaseVersion; } else { logger.detail('No release version provided. Determining from bundle.'); + final buildProgress = logger.progress('Building patch'); + try { + await buildAppBundle(flavor: flavor, target: target); + buildProgress.complete(); + } on ProcessException catch (error) { + buildProgress.fail('Failed to build: ${error.message}'); + return ExitCode.software.code; + } + + hasBuiltWithLatestFlutter = true; + final detectReleaseVersionProgress = logger.progress( 'Detecting release version', ); @@ -200,6 +203,20 @@ Current Flutter Revision: $originalFlutterRevision await shorebirdFlutter.useRevision(revision: originalFlutterRevision); flutterVersionProgress.complete(); } + } else if (!hasBuiltWithLatestFlutter) { + // If we haven't already built the patch with the latest version of + // Flutter (i.e., if the release version was provided as an argument and + // we didn't need to build the patch to determine the release version), + // build it now. + final buildProgress = logger.progress('Building patch'); + try { + await buildAppBundle(flavor: flavor, target: target); + buildProgress.complete(); + } on ProcessException catch (error) { + buildProgress.fail('Failed to build: ${error.message}'); + return ExitCode.software.code; + } + hasBuiltWithLatestFlutter = true; } final releaseArtifacts = await codePushClientWrapper.getReleaseArtifacts( diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart index 4b00c8a0..7393c4c4 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_ios_command.dart @@ -121,30 +121,11 @@ If this option is not provided, the version number will be determined from the p const releasePlatform = ReleasePlatform.ios; final flavor = results.findOption('flavor', argParser: argParser); final target = results.findOption('target', argParser: argParser); - final shorebirdYaml = shorebirdEnv.getShorebirdYaml()!; final appId = shorebirdYaml.getAppId(flavor: flavor); final app = await codePushClientWrapper.getApp(appId: appId); + var hasBuiltWithLatestFlutter = false; - try { - await _buildPatch(flavor: flavor, target: target); - } catch (_) { - return ExitCode.software.code; - } - - final archivePath = getXcarchiveDirectory()?.path; - if (archivePath == null) { - logger.err('Unable to find .xcarchive directory'); - return ExitCode.software.code; - } - - final plistFile = File(p.join(archivePath, 'Info.plist')); - if (!plistFile.existsSync()) { - logger.err('No Info.plist file found at ${plistFile.path}.'); - return ExitCode.software.code; - } - - final plist = Plist(file: plistFile); final String releaseVersion; final argReleaseVersion = results['release-version'] as String?; if (argReleaseVersion != null) { @@ -153,15 +134,19 @@ If this option is not provided, the version number will be determined from the p } else { logger.detail('No release version provided. Determining from archive.'); try { - releaseVersion = plist.versionNumber; - } catch (error) { - logger.err( - 'Failed to determine release version from ${plistFile.path}: $error', - ); + await _buildPatch(flavor: flavor, target: target); + } catch (_) { return ExitCode.software.code; } + hasBuiltWithLatestFlutter = true; - logger.info('Detected release version $releaseVersion'); + try { + releaseVersion = _readVersionFromPlist(); + logger.info('Detected release version $releaseVersion'); + } on _ReadVersionException catch (error) { + logger.err(error.message); + return ExitCode.software.code; + } } final release = await codePushClientWrapper.getRelease( @@ -207,6 +192,23 @@ Current Flutter Revision: $currentFlutterRevision await shorebirdFlutter.useRevision(revision: currentFlutterRevision); flutterVersionProgress.complete(); } + } else if (!hasBuiltWithLatestFlutter) { + // If we haven't already built the patch with the latest version of + // Flutter (i.e., if the release version was provided as an argument and + // we didn't need to build the patch to determine the release version), + // build it now. + try { + await _buildPatch(flavor: flavor, target: target); + } catch (_) { + return ExitCode.software.code; + } + hasBuiltWithLatestFlutter = true; + } + + final archivePath = getXcarchiveDirectory()?.path; + if (archivePath == null) { + logger.err('Unable to find .xcarchive directory'); + return ExitCode.software.code; } final releaseArtifact = await codePushClientWrapper.getReleaseArtifact( @@ -404,6 +406,29 @@ ${summary.join('\n')} 'out.vmcode', ); + String _readVersionFromPlist() { + final archivePath = getXcarchiveDirectory()?.path; + if (archivePath == null) { + throw _ReadVersionException('Unable to find .xcarchive directory'); + } + + final plistFile = File(p.join(archivePath, 'Info.plist')); + if (!plistFile.existsSync()) { + throw _ReadVersionException( + 'No Info.plist file found at ${plistFile.path}.', + ); + } + + final plist = Plist(file: plistFile); + try { + return plist.versionNumber; + } catch (error) { + throw _ReadVersionException( + 'Failed to determine release version from ${plistFile.path}: $error', + ); + } + } + Future _buildPatch({ required String? flavor, required String? target, @@ -474,3 +499,13 @@ ${summary.join('\n')} return ExitCode.success.code; } } + +/// {@template _ReadVersionException} +/// Exception thrown when the release version cannot be determined. +/// {@endtemplate} +class _ReadVersionException implements Exception { + /// {@macro _ReadVersionException} + _ReadVersionException(this.message); + + final String message; +} diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart index 427c34ee..52df1892 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_android_command_test.dart @@ -512,7 +512,7 @@ Please re-run the release command for this version or create a new release.'''), when(() => shorebirdEnv.flutterRevision).thenReturn(otherRevision); when( () => shorebirdFlutter.useRevision(revision: any(named: 'revision')), - ).thenAnswer((invocation) async { + ).thenAnswer((_) async { // Cause builds to fail after switching flutter versions. when(() => flutterBuildProcessResult.exitCode).thenReturn(1); when(() => flutterBuildProcessResult.stderr).thenReturn('oops'); @@ -539,6 +539,45 @@ Please re-run the release command for this version or create a new release.'''), verifyNever(() => bundletool.getVersionCode(any())); verifyNever(() => logger.progress('Detecting release version')); }); + + test('exits with code 70 if build fails', () async { + when(() => flutterBuildProcessResult.exitCode).thenReturn(1); + when(() => flutterBuildProcessResult.stderr).thenReturn('oops'); + + setUpProjectRoot(); + setUpProjectRootArtifacts(); + final exitCode = await runWithOverrides(command.run); + expect(exitCode, ExitCode.software.code); + }); + + test('only builds once if release uses different flutter revision', + () async { + const otherRevision = 'other-revision'; + when(() => shorebirdEnv.flutterRevision).thenReturn(otherRevision); + + setUpProjectRoot(); + setUpProjectRootArtifacts(); + final exitCode = await runWithOverrides(command.run); + expect(exitCode, ExitCode.success.code); + + verify( + () => shorebirdFlutter.useRevision(revision: release.flutterRevision), + ).called(1); + verify( + () => shorebirdProcess.run( + 'flutter', + [ + 'build', + 'appbundle', + '--release', + ], + runInShell: any(named: 'runInShell'), + ), + ).called(1); + verify( + () => shorebirdFlutter.useRevision(revision: otherRevision), + ).called(1); + }); }); group('when release-version option is not provided', () { diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart index b50152c4..754c16f5 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_ios_command_test.dart @@ -805,6 +805,65 @@ Please re-run the release command for this version or create a new release.'''), ), ).called(1); }); + + test('exits with code 70 if xcarchive is not found', () async { + setUpProjectRoot(); + setUpProjectRootArtifacts(); + Directory( + p.join(projectRoot.path, 'build'), + ).deleteSync(recursive: true); + + final exitCode = await runWithOverrides(command.run); + + expect(exitCode, equals(ExitCode.software.code)); + verify( + () => logger.err( + any(that: contains('Unable to find .xcarchive directory')), + ), + ).called(1); + }); + + test('exits with code 70 if build fails', () async { + when(() => flutterBuildProcessResult.exitCode).thenReturn(1); + when(() => flutterBuildProcessResult.stderr).thenReturn('oops'); + + setUpProjectRoot(); + setUpProjectRootArtifacts(); + final exitCode = await runWithOverrides(command.run); + expect(exitCode, ExitCode.software.code); + }); + + test('only builds once if release uses different flutter revision', + () async { + const otherRevision = 'other-revision'; + when(() => shorebirdEnv.flutterRevision).thenReturn(otherRevision); + + setUpProjectRoot(); + setUpProjectRootArtifacts(); + final exitCode = await runWithOverrides(command.run); + expect(exitCode, ExitCode.success.code); + + verify( + () => shorebirdFlutter.useRevision( + revision: preLinkerRelease.flutterRevision), + ).called(1); + verify( + () => shorebirdProcess.run( + 'flutter', + any( + that: containsAll([ + 'build', + 'ipa', + '--release', + ]), + ), + runInShell: any(named: 'runInShell'), + ), + ).called(1); + verify( + () => shorebirdFlutter.useRevision(revision: otherRevision), + ).called(1); + }); }); group('when release-version option is not provided', () {