diff --git a/packages/shorebird_cli/lib/src/cache.dart b/packages/shorebird_cli/lib/src/cache.dart index 609a7fa26..829011cf4 100644 --- a/packages/shorebird_cli/lib/src/cache.dart +++ b/packages/shorebird_cli/lib/src/cache.dart @@ -1,6 +1,7 @@ import 'dart:io' hide Platform; import 'package:http/http.dart' as http; +import 'package:meta/meta.dart'; import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; @@ -49,15 +50,17 @@ class Cache { registerArtifact(AotToolsExeArtifact(cache: this, platform: platform)); } - void registerArtifact(CachedArtifact artifact) => _artifacts.add(artifact); + void registerArtifact(CachedArtifact artifact) => artifacts.add(artifact); Future updateAll() async { - for (final artifact in _artifacts) { + for (final artifact in artifacts) { if (await artifact.isUpToDate()) { continue; } + final progress = logger.progress('Downloading ${artifact.name} artifact'); await artifact.update(); + progress.complete('Downloaded ${artifact.name} artifact'); } } @@ -98,7 +101,8 @@ class Cache { ); } - final List _artifacts = []; + @visibleForTesting + final List artifacts = []; String get storageBaseUrl => 'https://storage.googleapis.com'; @@ -207,6 +211,14 @@ class AotToolsDillArtifact extends CachedArtifact { /// For a few revisions in Dec 2023, we distributed aot-tools as an executable. /// Should be removed sometime after June 2024. +/// +/// The change to use a .dill file was made in +/// https://github.com/shorebirdtech/_build_engine/commit/babbc37d93e7a2f36e62787e47eee5a3b5458901 +/// The Flutter versions that use this are: +/// - 3.13.9 (a3d5f7c614aa1cc4d6cb1506e74fd1c81678e68e) +/// - 3.16.3 (b9b23902966504a9778f4c07e3a3487fa84dcb2a) +/// - 3.16.4 (7e92b034c5dddb727cf5e802c23cddd39b325a7f) +/// - 3.16.5 (4e8a7c746ae6f10951f3e676f10b82b21d7300a5) class AotToolsExeArtifact extends CachedArtifact { AotToolsExeArtifact({required super.cache, required super.platform}); diff --git a/packages/shorebird_cli/lib/src/command_runner.dart b/packages/shorebird_cli/lib/src/command_runner.dart index 2825734db..7507e2403 100644 --- a/packages/shorebird_cli/lib/src/command_runner.dart +++ b/packages/shorebird_cli/lib/src/command_runner.dart @@ -5,6 +5,7 @@ import 'package:args/command_runner.dart'; import 'package:cli_completion/cli_completion.dart'; import 'package:mason_logger/mason_logger.dart'; import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; import 'package:shorebird_cli/src/engine_config.dart'; import 'package:shorebird_cli/src/logger.dart'; @@ -206,6 +207,13 @@ Run ${lightCyan.wrap('shorebird upgrade')} to upgrade.'''); exitCode = ExitCode.success.code; } else { try { + // Most commands need shorebird artifacts, so we ensure the cache + // is up to date for all commands (even ones which don't need it). We + // could make a Command subclass and move the cache onto that and + // only update in that case, but it didn't seem worth it at the time. + // Ensure all cached artifacts are up-to-date before running the + // command. + await cache.updateAll(); exitCode = await super.runCommand(topLevelResults); } catch (error, stackTrace) { logger diff --git a/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart b/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart index 809710e98..47fc483b9 100644 --- a/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart +++ b/packages/shorebird_cli/lib/src/commands/patch/patch_aar_command.dart @@ -9,7 +9,6 @@ import 'package:path/path.dart' as p; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/artifact_manager.dart'; -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/commands/patch/patch.dart'; @@ -99,8 +98,6 @@ of the Android app that is using this module.''', final allowAssetDiffs = results['allow-asset-diffs'] == true; final allowNativeDiffs = results['allow-native-diffs'] == true; - await cache.updateAll(); - if (shorebirdEnv.androidPackageName == null) { logger.err('Could not find androidPackage in pubspec.yaml.'); return ExitCode.config.code; 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 1408d8099..a42f7005f 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 @@ -7,7 +7,6 @@ import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_differ.dart'; import 'package:shorebird_cli/src/artifact_manager.dart'; -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; @@ -108,8 +107,6 @@ If this option is not provided, the version number will be determined from the p final dryRun = results['dry-run'] == true; final isStaging = results['staging'] == true; - await cache.updateAll(); - const releasePlatform = ReleasePlatform.android; final flavor = results.findOption('flavor', argParser: argParser); final target = results.findOption('target', argParser: argParser); diff --git a/packages/shorebird_cli/lib/src/executables/aot_tools.dart b/packages/shorebird_cli/lib/src/executables/aot_tools.dart index cd910e661..db75824ab 100644 --- a/packages/shorebird_cli/lib/src/executables/aot_tools.dart +++ b/packages/shorebird_cli/lib/src/executables/aot_tools.dart @@ -6,7 +6,6 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:path/path.dart' as p; import 'package:pub_semver/pub_semver.dart'; import 'package:scoped/scoped.dart'; -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/engine_config.dart'; import 'package:shorebird_cli/src/extensions/version.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; @@ -86,8 +85,6 @@ class AotTools { List command, { String? workingDirectory, }) async { - await cache.updateAll(); - // This will be a path to either a kernel (.dill) file or a Dart script if // we're running with a local engine. final artifactPath = shorebirdArtifacts.getArtifactPath( diff --git a/packages/shorebird_cli/lib/src/executables/bundletool.dart b/packages/shorebird_cli/lib/src/executables/bundletool.dart index 8b65f267e..55b74305c 100644 --- a/packages/shorebird_cli/lib/src/executables/bundletool.dart +++ b/packages/shorebird_cli/lib/src/executables/bundletool.dart @@ -16,7 +16,6 @@ class Bundletool { static const jar = 'bundletool.jar'; Future _exec(List command) async { - await cache.updateAll(); final bundletool = p.join(cache.getArtifactDirectory(jar).path, jar); final javaHome = java.home; final javaExecutable = java.executable ?? 'java'; diff --git a/packages/shorebird_cli/lib/src/shorebird_release_version_mixin.dart b/packages/shorebird_cli/lib/src/shorebird_release_version_mixin.dart index d8e4695ab..864794ab9 100644 --- a/packages/shorebird_cli/lib/src/shorebird_release_version_mixin.dart +++ b/packages/shorebird_cli/lib/src/shorebird_release_version_mixin.dart @@ -1,4 +1,3 @@ -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/command.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; @@ -9,8 +8,6 @@ mixin ShorebirdReleaseVersionMixin on ShorebirdCommand { Future extractReleaseVersionFromAppBundle( String appBundlePath, ) async { - await cache.updateAll(); - final results = await Future.wait([ bundletool.getVersionName(appBundlePath), bundletool.getVersionCode(appBundlePath), diff --git a/packages/shorebird_cli/test/src/cache_test.dart b/packages/shorebird_cli/test/src/cache_test.dart index 1802aae9a..53c41fc7a 100644 --- a/packages/shorebird_cli/test/src/cache_test.dart +++ b/packages/shorebird_cli/test/src/cache_test.dart @@ -28,6 +28,7 @@ void main() { late http.Client httpClient; late Logger logger; late Platform platform; + late Progress progress; late Process chmodProcess; late ShorebirdEnv shorebirdEnv; late ShorebirdProcess shorebirdProcess; @@ -72,6 +73,7 @@ void main() { httpClient = MockHttpClient(); logger = MockLogger(); platform = MockPlatform(); + progress = MockProgress(); chmodProcess = MockProcess(); shorebirdEnv = MockShorebirdEnv(); shorebirdProcess = MockShorebirdProcess(); @@ -86,6 +88,7 @@ void main() { (invocation.namedArguments[#outputDirectory] as Directory) .createSync(recursive: true); }); + when(() => logger.progress(any())).thenReturn(progress); when( () => shorebirdEnv.shorebirdEngineRevision, ).thenReturn(shorebirdEngineRevision); @@ -179,6 +182,41 @@ void main() { }); group('updateAll', () { + group('progress', () { + group('when all artifacts are up-to-date', () { + setUp(() { + runWithOverrides(() { + for (final artifact in cache.artifacts) { + artifact.location.createSync(recursive: true); + } + }); + }); + + test('does not print progress', () async { + await runWithOverrides(cache.updateAll); + verifyNever(() => logger.progress(any())); + }); + }); + + group('when artifacts require updates', () { + test('prints progress', () async { + await runWithOverrides(cache.updateAll); + + // We expect one fewer call to progress.complete than the number of + // artifacts. This is because [AotToolsDillArtifact] and + // [AotToolsExeArtifact] share a location. These artifacts will + // never both be present for the same engine revision. We + // temporarily used an precompiled executable for aot-tools before + // switching to a .dill file. This can be removed once we no longer + // need to support the precompiled executable case. See the + // [AotToolsExeArtifact] definition for more details. + final expectedUpdateCount = cache.artifacts.length - 1; + verify(() => logger.progress(any())).called(expectedUpdateCount); + verify(() => progress.complete(any())).called(expectedUpdateCount); + }); + }); + }); + group('patch', () { test('throws CacheUpdateFailure if a SocketException is thrown', () async { diff --git a/packages/shorebird_cli/test/src/command_runner_test.dart b/packages/shorebird_cli/test/src/command_runner_test.dart index a16013b82..1bfe89aa1 100644 --- a/packages/shorebird_cli/test/src/command_runner_test.dart +++ b/packages/shorebird_cli/test/src/command_runner_test.dart @@ -3,6 +3,7 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; +import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/command_runner.dart'; import 'package:shorebird_cli/src/logger.dart' hide logger; import 'package:shorebird_cli/src/platform.dart'; @@ -20,6 +21,7 @@ void main() { const flutterRevision = 'test-flutter-revision'; const flutterVersion = '1.2.3'; + late Cache cache; late Logger logger; late Platform platform; late ShorebirdEnv shorebirdEnv; @@ -31,6 +33,7 @@ void main() { return runScoped( body, values: { + cacheRef.overrideWith(() => cache), loggerRef.overrideWith(() => logger), platformRef.overrideWith(() => platform), shorebirdEnvRef.overrideWith(() => shorebirdEnv), @@ -41,11 +44,13 @@ void main() { } setUp(() { + cache = MockCache(); logger = MockLogger(); platform = MockPlatform(); shorebirdEnv = MockShorebirdEnv(); shorebirdFlutter = MockShorebirdFlutter(); shorebirdVersion = MockShorebirdVersion(); + when(() => cache.updateAll()).thenAnswer((_) async {}); when(() => logger.level).thenReturn(Level.info); when( () => shorebirdEnv.shorebirdEngineRevision, @@ -272,6 +277,16 @@ Run ${lightCyan.wrap('shorebird upgrade')} to upgrade.'''), }); group('on command failure', () { + test('updates cache artifacts', () async { + // This will fail due to the release android command missing scoped + // dependencies. Because the cache update should happen before the + // command runs, we verify that behavior here. + await runWithOverrides( + () => commandRunner.run(['release', 'android']), + ); + verify(() => cache.updateAll()).called(1); + }); + test('logs a stack trace using detail', () async { // This will fail due to the release android command missing scoped // dependencies. diff --git a/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart b/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart index d2ca605af..3f5b40c34 100644 --- a/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/patch/patch_aar_command_test.dart @@ -10,7 +10,6 @@ import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/archive_analysis/archive_analysis.dart'; import 'package:shorebird_cli/src/artifact_manager.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; -import 'package:shorebird_cli/src/cache.dart' show Cache, cacheRef; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; import 'package:shorebird_cli/src/config/config.dart'; @@ -102,7 +101,6 @@ void main() { late ShorebirdProcessResult flutterBuildProcessResult; late ShorebirdProcessResult flutterPubGetProcessResult; late http.Client httpClient; - late Cache cache; late ShorebirdEnv shorebirdEnv; late ShorebirdFlutter shorebirdFlutter; late ShorebirdProcess shorebirdProcess; @@ -115,7 +113,6 @@ void main() { values: { artifactManagerRef.overrideWith(() => artifactManager), authRef.overrideWith(() => auth), - cacheRef.overrideWith(() => cache), codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), engineConfigRef.overrideWith(() => const EngineConfig.empty()), httpClientRef.overrideWith(() => httpClient), @@ -190,7 +187,6 @@ void main() { flutterBuildProcessResult = MockProcessResult(); flutterPubGetProcessResult = MockProcessResult(); httpClient = MockHttpClient(); - cache = MockCache(); shorebirdEnv = MockShorebirdEnv(); shorebirdFlutter = MockShorebirdFlutter(); shorebirdProcess = MockShorebirdProcess(); @@ -318,10 +314,6 @@ void main() { metadata: any(named: 'metadata'), ), ).thenAnswer((_) async {}); - when(() => cache.updateAll()).thenAnswer((_) async => {}); - when( - () => cache.getArtifactDirectory(any()), - ).thenReturn(Directory.systemTemp.createTempSync()); when( () => shorebirdFlutter.getVersionAndRevision(), ).thenAnswer((_) async => flutterVersionAndRevision); diff --git a/packages/shorebird_cli/test/src/commands/release/release_android_command_test.dart b/packages/shorebird_cli/test/src/commands/release/release_android_command_test.dart index 8a6017930..490df53df 100644 --- a/packages/shorebird_cli/test/src/commands/release/release_android_command_test.dart +++ b/packages/shorebird_cli/test/src/commands/release/release_android_command_test.dart @@ -8,7 +8,6 @@ import 'package:path/path.dart' as p; import 'package:platform/platform.dart'; import 'package:scoped/scoped.dart'; import 'package:shorebird_cli/src/auth/auth.dart'; -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/code_push_client_wrapper.dart'; import 'package:shorebird_cli/src/commands/commands.dart'; import 'package:shorebird_cli/src/config/config.dart'; @@ -73,7 +72,6 @@ void main() { late Doctor doctor; late Platform platform; late Auth auth; - late Cache cache; late Java java; late Logger logger; late OperatingSystemInterface operatingSystemInterface; @@ -93,7 +91,6 @@ void main() { values: { authRef.overrideWith(() => auth), bundletoolRef.overrideWith(() => bundletool), - cacheRef.overrideWith(() => cache), codePushClientWrapperRef.overrideWith(() => codePushClientWrapper), doctorRef.overrideWith(() => doctor), engineConfigRef.overrideWith(() => const EngineConfig.empty()), @@ -127,7 +124,6 @@ void main() { shorebirdRoot = Directory.systemTemp.createTempSync(); projectRoot = Directory.systemTemp.createTempSync(); auth = MockAuth(); - cache = MockCache(); java = MockJava(); progress = MockProgress(); logger = MockLogger(); @@ -191,10 +187,6 @@ void main() { when(() => argResults.wasParsed(any())).thenReturn(true); when(() => auth.isAuthenticated).thenReturn(true); when(() => auth.client).thenReturn(httpClient); - when(() => cache.updateAll()).thenAnswer((_) async => {}); - when( - () => cache.getArtifactDirectory(any()), - ).thenReturn(Directory.systemTemp.createTempSync()); when(() => logger.progress(any())).thenReturn(progress); when(() => logger.confirm(any())).thenReturn(true); when( diff --git a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart index cdb343f3e..7a7c5e312 100644 --- a/packages/shorebird_cli/test/src/executables/aot_tools_test.dart +++ b/packages/shorebird_cli/test/src/executables/aot_tools_test.dart @@ -4,7 +4,6 @@ import 'package:mason_logger/mason_logger.dart'; import 'package:mocktail/mocktail.dart'; import 'package:path/path.dart' as p; import 'package:scoped/scoped.dart'; -import 'package:shorebird_cli/src/cache.dart'; import 'package:shorebird_cli/src/executables/executables.dart'; import 'package:shorebird_cli/src/shorebird_artifacts.dart'; import 'package:shorebird_cli/src/shorebird_env.dart'; @@ -15,7 +14,6 @@ import '../mocks.dart'; void main() { group(AotTools, () { - late Cache cache; late ShorebirdArtifacts shorebirdArtifacts; late ShorebirdProcess process; late ShorebirdEnv shorebirdEnv; @@ -27,7 +25,6 @@ void main() { return runScoped( body, values: { - cacheRef.overrideWith(() => cache), processRef.overrideWith(() => process), shorebirdArtifactsRef.overrideWith(() => shorebirdArtifacts), shorebirdEnvRef.overrideWith(() => shorebirdEnv), @@ -36,7 +33,6 @@ void main() { } setUp(() { - cache = MockCache(); process = MockShorebirdProcess(); shorebirdArtifacts = MockShorebirdArtifacts(); shorebirdEnv = MockShorebirdEnv(); @@ -44,7 +40,6 @@ void main() { workingDirectory = Directory('aot-tools test'); aotTools = AotTools(); - when(() => cache.updateAll()).thenAnswer((_) async {}); when(() => shorebirdEnv.dartBinaryFile).thenReturn(dartBinaryFile); when( () => shorebirdArtifacts.getArtifactPath( diff --git a/packages/shorebird_cli/test/src/executables/bundletool_test.dart b/packages/shorebird_cli/test/src/executables/bundletool_test.dart index 7152b8002..90ad24315 100644 --- a/packages/shorebird_cli/test/src/executables/bundletool_test.dart +++ b/packages/shorebird_cli/test/src/executables/bundletool_test.dart @@ -45,7 +45,6 @@ void main() { bundletool = Bundletool(); when(() => androidSdk.path).thenReturn(androidSdkPath); - when(() => cache.updateAll()).thenAnswer((_) async {}); when( () => cache.getArtifactDirectory(any()), ).thenReturn(workingDirectory); diff --git a/third_party/flutter/bin/internal/shared.sh b/third_party/flutter/bin/internal/shared.sh index f7235293a..0e41d3025 100755 --- a/third_party/flutter/bin/internal/shared.sh +++ b/third_party/flutter/bin/internal/shared.sh @@ -164,8 +164,12 @@ function upgrade_shorebird () ( mv "$SNAPSHOT_PATH" "$SNAPSHOT_PATH_OLD" fi - # Compile... - $DART_PATH --verbosity=error --disable-dart-dev --snapshot="$SNAPSHOT_PATH" --snapshot-kind="app-jit" --packages="$SHOREBIRD_CLI_DIR/.dart_tool/package_config.json" --no-enable-mirrors "$SCRIPT_PATH" > /dev/null + # Compile our snapshot. + # We invoke `$SNAPSHOT_PATH completion` to trigger the "completion" command, which + # avoids executing as much of our code as possible. We do this because running + # the script here (instead of from the compiled snapshot) invalidates a lot of + # assumptions we make about the cwd in the shorebird_cli tool. + $DART_PATH --verbosity=error --disable-dart-dev --snapshot="$SNAPSHOT_PATH" --snapshot-kind="app-jit" --packages="$SHOREBIRD_CLI_DIR/.dart_tool/package_config.json" --no-enable-mirrors "$SCRIPT_PATH" completion > /dev/null echo "$compilekey" > "$STAMP_PATH" # Delete any temporary snapshot path.