From 082ae838bd60d55d21190d21fcd49599bc6f52b8 Mon Sep 17 00:00:00 2001 From: Jenn Magder Date: Fri, 7 Feb 2020 15:28:45 -0800 Subject: [PATCH] Fall back to global cache and platform if null when injected into constructor (#50370) --- .../build_ios_framework_module_test.dart | 50 ++++++ dev/devicelab/lib/framework/utils.dart | 7 + .../flutter_tools/lib/src/commands/build.dart | 3 - .../lib/src/commands/build_ios_framework.dart | 27 ++- .../hermetic/build_ios_framework_test.dart | 158 +++++++++++------- 5 files changed, 168 insertions(+), 77 deletions(-) diff --git a/dev/devicelab/bin/tasks/build_ios_framework_module_test.dart b/dev/devicelab/bin/tasks/build_ios_framework_module_test.dart index 051e191f5a..db2928126f 100644 --- a/dev/devicelab/bin/tasks/build_ios_framework_module_test.dart +++ b/dev/devicelab/bin/tasks/build_ios_framework_module_test.dart @@ -298,6 +298,56 @@ Future main() async { } } + // This builds all build modes' frameworks by default + section('Build podspec'); + + const String cocoapodsOutputDirectoryName = 'flutter-frameworks-cocoapods'; + + await inDirectory(projectDir, () async { + await flutter( + 'build', + options: [ + 'ios-framework', + '--cocoapods', + '--force', // Allow podspec creation on master. + '--output=$cocoapodsOutputDirectoryName' + ], + ); + }); + + final String cocoapodsOutputPath = path.join(projectDir.path, cocoapodsOutputDirectoryName); + for (final String mode in ['Debug', 'Profile', 'Release']) { + checkFileExists(path.join( + cocoapodsOutputPath, + mode, + 'Flutter.podspec', + )); + + checkDirectoryExists(path.join( + cocoapodsOutputPath, + mode, + 'App.framework', + )); + + checkDirectoryExists(path.join( + cocoapodsOutputPath, + mode, + 'FlutterPluginRegistrant.framework', + )); + + checkDirectoryExists(path.join( + cocoapodsOutputPath, + mode, + 'device_info.framework', + )); + + checkDirectoryExists(path.join( + cocoapodsOutputPath, + mode, + 'package_info.framework', + )); + } + return TaskResult.success(null); } on TaskResult catch (taskResult) { return taskResult; diff --git a/dev/devicelab/lib/framework/utils.dart b/dev/devicelab/lib/framework/utils.dart index 58dec5c761..b2169377e1 100644 --- a/dev/devicelab/lib/framework/utils.dart +++ b/dev/devicelab/lib/framework/utils.dart @@ -641,6 +641,13 @@ void checkFileNotExists(String file) { } } +/// Checks that the directory exists, otherwise throws a [FileSystemException]. +void checkDirectoryExists(String directory) { + if (!exists(Directory(directory))) { + throw FileSystemException('Expected directory to exist.', directory); + } +} + /// Check that `collection` contains all entries in `values`. void checkCollectionContains(Iterable values, Iterable collection) { for (final T value in values) { diff --git a/packages/flutter_tools/lib/src/commands/build.dart b/packages/flutter_tools/lib/src/commands/build.dart index 3af077b703..26c43f6893 100644 --- a/packages/flutter_tools/lib/src/commands/build.dart +++ b/packages/flutter_tools/lib/src/commands/build.dart @@ -9,7 +9,6 @@ import '../bundle.dart'; import '../commands/build_linux.dart'; import '../commands/build_macos.dart'; import '../commands/build_windows.dart'; -import '../globals.dart' as globals; import '../runner/flutter_command.dart'; import 'build_aar.dart'; import 'build_aot.dart'; @@ -31,8 +30,6 @@ class BuildCommand extends FlutterCommand { addSubcommand(BuildIOSFrameworkCommand( aotBuilder: AotBuilder(), bundleBuilder: BundleBuilder(), - cache: globals.cache, - platform: globals.platform, )); addSubcommand(BuildBundleCommand(verboseHelp: verboseHelp)); addSubcommand(BuildWebCommand()); diff --git a/packages/flutter_tools/lib/src/commands/build_ios_framework.dart b/packages/flutter_tools/lib/src/commands/build_ios_framework.dart index 84faa31bf3..cb673bae91 100644 --- a/packages/flutter_tools/lib/src/commands/build_ios_framework.dart +++ b/packages/flutter_tools/lib/src/commands/build_ios_framework.dart @@ -37,13 +37,13 @@ class BuildIOSFrameworkCommand extends BuildSubCommand { FlutterVersion flutterVersion, // Instantiating FlutterVersion kicks off networking, so delay until it's needed, but allow test injection. @required AotBuilder aotBuilder, @required BundleBuilder bundleBuilder, - @required Cache cache, - @required Platform platform + Cache cache, + Platform platform }) : _flutterVersion = flutterVersion, _aotBuilder = aotBuilder, _bundleBuilder = bundleBuilder, - _cache = cache, - _platform = platform { + _injectedCache = cache, + _injectedPlatform = platform { addTreeShakeIconsFlag(); usesTargetOption(); usesFlavorOption(); @@ -84,13 +84,22 @@ class BuildIOSFrameworkCommand extends BuildSubCommand { abbr: 'o', valueHelp: 'path/to/directory/', help: 'Location to write the frameworks.', + ) + ..addFlag('force', + abbr: 'f', + help: 'Force Flutter.podspec creation on the master channel. For testing only.', + hide: true ); } final AotBuilder _aotBuilder; final BundleBuilder _bundleBuilder; - final Cache _cache; - final Platform _platform; + + Cache get _cache => _injectedCache ?? globals.cache; + final Cache _injectedCache; + + Platform get _platform => _injectedPlatform ?? globals.platform; + final Platform _injectedPlatform; FlutterVersion _flutterVersion; @@ -180,7 +189,7 @@ class BuildIOSFrameworkCommand extends BuildSubCommand { if (boolArg('cocoapods')) { // FlutterVersion.instance kicks off git processing which can sometimes fail, so don't try it until needed. _flutterVersion ??= globals.flutterVersion; - produceFlutterPodspec(mode, modeDirectory); + produceFlutterPodspec(mode, modeDirectory, force: boolArg('force')); } else { // Copy Flutter.framework. await _produceFlutterFramework(mode, modeDirectory); @@ -218,11 +227,11 @@ class BuildIOSFrameworkCommand extends BuildSubCommand { /// Create podspec that will download and unzip remote engine assets so host apps can leverage CocoaPods /// vendored framework caching. @visibleForTesting - void produceFlutterPodspec(BuildMode mode, Directory modeDirectory) { + void produceFlutterPodspec(BuildMode mode, Directory modeDirectory, { bool force = false }) { final Status status = globals.logger.startProgress(' ├─Creating Flutter.podspec...', timeout: timeoutConfiguration.fastOperation); try { final GitTagVersion gitTagVersion = _flutterVersion.gitTagVersion; - if (gitTagVersion.x == null || gitTagVersion.y == null || gitTagVersion.z == null || gitTagVersion.commits != 0) { + if (!force && (gitTagVersion.x == null || gitTagVersion.y == null || gitTagVersion.z == null || gitTagVersion.commits != 0)) { throwToolExit( '--cocoapods is only supported on the dev, beta, or stable channels. Detected version is ${_flutterVersion.frameworkVersion}'); } diff --git a/packages/flutter_tools/test/commands.shard/hermetic/build_ios_framework_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/build_ios_framework_test.dart index abf22a4951..8fa3b3becd 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/build_ios_framework_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/build_ios_framework_test.dart @@ -134,7 +134,6 @@ void main() { when(mockGitTagVersion.y).thenReturn(13); when(mockGitTagVersion.z).thenReturn(11); when(mockGitTagVersion.hotfix).thenReturn(13); - when(mockGitTagVersion.commits).thenReturn(0); when(mockFlutterVersion.frameworkVersion).thenReturn(frameworkVersion); @@ -143,78 +142,107 @@ void main() { ..writeAsStringSync(licenseText); }); - testUsingContext('contains license and version', () async { - final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( - aotBuilder: MockAotBuilder(), - bundleBuilder: MockBundleBuilder(), - platform: fakePlatform, - flutterVersion: mockFlutterVersion, - cache: mockCache - ); - command.produceFlutterPodspec(BuildMode.debug, outputDirectory); + group('on master channel', () { + setUp(() { + when(mockGitTagVersion.commits).thenReturn(100); + }); - final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); - final String podspecContents = expectedPodspec.readAsStringSync(); - expect(podspecContents, contains('\'1.13.1113\'')); - expect(podspecContents, contains('# $frameworkVersion')); - expect(podspecContents, contains(licenseText)); - }, overrides: { - FileSystem: () => memoryFileSystem, - ProcessManager: () => FakeProcessManager.any(), + testUsingContext('created when forced', () async { + final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( + aotBuilder: MockAotBuilder(), + bundleBuilder: MockBundleBuilder(), + platform: fakePlatform, + flutterVersion: mockFlutterVersion, + cache: mockCache + ); + command.produceFlutterPodspec(BuildMode.debug, outputDirectory, force: true); + + final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); + expect(expectedPodspec.existsSync(), isTrue); + }, overrides: { + FileSystem: () => memoryFileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); }); - testUsingContext('debug URL', () async { - final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( - aotBuilder: MockAotBuilder(), - bundleBuilder: MockBundleBuilder(), - platform: fakePlatform, - flutterVersion: mockFlutterVersion, - cache: mockCache - ); - command.produceFlutterPodspec(BuildMode.debug, outputDirectory); + group('not on master channel', () { + setUp(() { + when(mockGitTagVersion.commits).thenReturn(0); + }); - final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); - final String podspecContents = expectedPodspec.readAsStringSync(); - expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios/artifacts.zip\'')); - }, overrides: { - FileSystem: () => memoryFileSystem, - ProcessManager: () => FakeProcessManager.any(), - }); + testUsingContext('contains license and version', () async { + final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( + aotBuilder: MockAotBuilder(), + bundleBuilder: MockBundleBuilder(), + platform: fakePlatform, + flutterVersion: mockFlutterVersion, + cache: mockCache + ); + command.produceFlutterPodspec(BuildMode.debug, outputDirectory); - testUsingContext('profile URL', () async { - final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( - aotBuilder: MockAotBuilder(), - bundleBuilder: MockBundleBuilder(), - platform: fakePlatform, - flutterVersion: mockFlutterVersion, - cache: mockCache - ); - command.produceFlutterPodspec(BuildMode.profile, outputDirectory); + final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); + final String podspecContents = expectedPodspec.readAsStringSync(); + expect(podspecContents, contains('\'1.13.1113\'')); + expect(podspecContents, contains('# $frameworkVersion')); + expect(podspecContents, contains(licenseText)); + }, overrides: { + FileSystem: () => memoryFileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); - final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); - final String podspecContents = expectedPodspec.readAsStringSync(); - expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios-profile/artifacts.zip\'')); - }, overrides: { - FileSystem: () => memoryFileSystem, - ProcessManager: () => FakeProcessManager.any(), - }); + testUsingContext('debug URL', () async { + final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( + aotBuilder: MockAotBuilder(), + bundleBuilder: MockBundleBuilder(), + platform: fakePlatform, + flutterVersion: mockFlutterVersion, + cache: mockCache + ); + command.produceFlutterPodspec(BuildMode.debug, outputDirectory); - testUsingContext('release URL', () async { - final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( - aotBuilder: MockAotBuilder(), - bundleBuilder: MockBundleBuilder(), - platform: fakePlatform, - flutterVersion: mockFlutterVersion, - cache: mockCache - ); - command.produceFlutterPodspec(BuildMode.release, outputDirectory); + final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); + final String podspecContents = expectedPodspec.readAsStringSync(); + expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios/artifacts.zip\'')); + }, overrides: { + FileSystem: () => memoryFileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); - final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); - final String podspecContents = expectedPodspec.readAsStringSync(); - expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios-release/artifacts.zip\'')); - }, overrides: { - FileSystem: () => memoryFileSystem, - ProcessManager: () => FakeProcessManager.any(), + testUsingContext('profile URL', () async { + final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( + aotBuilder: MockAotBuilder(), + bundleBuilder: MockBundleBuilder(), + platform: fakePlatform, + flutterVersion: mockFlutterVersion, + cache: mockCache + ); + command.produceFlutterPodspec(BuildMode.profile, outputDirectory); + + final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); + final String podspecContents = expectedPodspec.readAsStringSync(); + expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios-profile/artifacts.zip\'')); + }, overrides: { + FileSystem: () => memoryFileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); + + testUsingContext('release URL', () async { + final BuildIOSFrameworkCommand command = BuildIOSFrameworkCommand( + aotBuilder: MockAotBuilder(), + bundleBuilder: MockBundleBuilder(), + platform: fakePlatform, + flutterVersion: mockFlutterVersion, + cache: mockCache + ); + command.produceFlutterPodspec(BuildMode.release, outputDirectory); + + final File expectedPodspec = outputDirectory.childFile('Flutter.podspec'); + final String podspecContents = expectedPodspec.readAsStringSync(); + expect(podspecContents, contains('\'$storageBaseUrl/flutter_infra/flutter/$engineRevision/ios-release/artifacts.zip\'')); + }, overrides: { + FileSystem: () => memoryFileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); }); }); });