From 6579844528dc1b487f0bf61c8cbd86e8d4d78d19 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Tue, 15 Aug 2023 07:28:18 -0700 Subject: [PATCH] Update `flutter_tools` internals related to Gradle/XCode to set `--local-engine-host`. (#132346) Partial work towards https://github.com/flutter/flutter/issues/132245. I made a minor refactor to test-only code because it was too confusing to have 2 optional parameters that are technically required together, but otherwise all other changes *should* be pass throughs. That being said, I can't say I totally understand the Gradle stuff so I could use a hand double checking that. --- .../gradle/src/main/groovy/flutter.groovy | 14 +++++++ .../flutter_tools/lib/src/android/gradle.dart | 2 + packages/flutter_tools/lib/src/artifacts.dart | 39 +++++++++++++------ packages/flutter_tools/lib/src/ios/mac.dart | 1 + .../hermetic/assemble_test.dart | 2 +- .../android/android_gradle_builder_test.dart | 24 ++++++++---- .../general.shard/android/gradle_test.dart | 2 +- .../test/general.shard/build_info_test.dart | 10 ++--- .../general.shard/ios/xcodeproj_test.dart | 12 +++--- 9 files changed, 73 insertions(+), 33 deletions(-) diff --git a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy index 95a750e6ef..657ef144bf 100644 --- a/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy +++ b/packages/flutter_tools/gradle/src/main/groovy/flutter.groovy @@ -164,6 +164,7 @@ class FlutterPlugin implements Plugin { private File flutterRoot private File flutterExecutable private String localEngine + private String localEngineHost private String localEngineSrcPath private Properties localProperties private String engineVersion @@ -324,6 +325,13 @@ class FlutterPlugin implements Plugin { } localEngine = engineOut.name localEngineSrcPath = engineOut.parentFile.parent + + String engineHostOutPath = project.property('local-engine-host-out') + File engineHostOut = project.file(engineHostOutPath) + if (!engineHostOut.isDirectory()) { + throw new GradleException('local-engine-host-out must point to a local engine host build') + } + localEngineHostOut = engineHostOut.name } project.android.buildTypes.all this.&addFlutterDependencies } @@ -1027,6 +1035,7 @@ class FlutterPlugin implements Plugin { flutterExecutable this.flutterExecutable buildMode variantBuildMode localEngine this.localEngine + localEngineHost this.localEngineHost localEngineSrcPath this.localEngineSrcPath targetPath getFlutterTarget() verbose this.isVerbose() @@ -1235,6 +1244,8 @@ abstract class BaseFlutterTask extends DefaultTask { @Optional @Input String localEngine @Optional @Input + String localEngineHost + @Optional @Input String localEngineSrcPath @Optional @Input Boolean fastStart @@ -1313,6 +1324,9 @@ abstract class BaseFlutterTask extends DefaultTask { args "--local-engine", localEngine args "--local-engine-src-path", localEngineSrcPath } + if (localEngineHost != null) { + args "--local-engine-host", localEngineHost + } if (verbose) { args "--verbose" } else { diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index d90431dd49..3a456dc48f 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -401,6 +401,7 @@ class AndroidGradleBuilder implements AndroidBuilder { command.add('-Plocal-engine-repo=${localEngineRepo.path}'); command.add('-Plocal-engine-build-mode=${buildInfo.modeName}'); command.add('-Plocal-engine-out=${localEngineInfo.engineOutPath}'); + command.add('-Plocal-engine-host-out=${localEngineInfo.engineHostOutPath}'); command.add('-Ptarget-platform=${_getTargetPlatformByLocalEnginePath( localEngineInfo.engineOutPath)}'); } else if (androidBuildInfo.targetArchs.isNotEmpty) { @@ -726,6 +727,7 @@ class AndroidGradleBuilder implements AndroidBuilder { command.add('-Plocal-engine-repo=${localEngineRepo.path}'); command.add('-Plocal-engine-build-mode=${buildInfo.modeName}'); command.add('-Plocal-engine-out=${localEngineInfo.engineOutPath}'); + command.add('-Plocal-engine-host-out=${localEngineInfo.engineHostOutPath}'); // Copy the local engine repo in the output directory. try { diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart index f226d139d2..97e6e1da9f 100644 --- a/packages/flutter_tools/lib/src/artifacts.dart +++ b/packages/flutter_tools/lib/src/artifacts.dart @@ -3,6 +3,7 @@ // found in the LICENSE file. import 'package:file/memory.dart'; +import 'package:meta/meta.dart'; import 'package:process/process.dart'; import 'base/common.dart'; @@ -287,10 +288,12 @@ class EngineBuildPaths { class LocalEngineInfo { const LocalEngineInfo({ required this.engineOutPath, + required this.engineHostOutPath, required this.localEngineName, }); final String engineOutPath; + final String engineHostOutPath; final String localEngineName; } @@ -302,12 +305,23 @@ abstract class Artifacts { /// If a [fileSystem] is not provided, creates a new [MemoryFileSystem] instance. /// /// Creates a [LocalEngineArtifacts] if `localEngine` is non-null - factory Artifacts.test({String? localEngine, FileSystem? fileSystem}) { - fileSystem ??= MemoryFileSystem.test(); - if (localEngine != null) { - return _TestLocalEngine(localEngine, fileSystem); - } - return _TestArtifacts(fileSystem); + @visibleForTesting + factory Artifacts.test({FileSystem? fileSystem}) { + return _TestArtifacts(fileSystem ?? MemoryFileSystem.test()); + } + + /// A test-specific implementation of artifacts that returns stable paths for + /// all artifacts, and uses a local engine. + /// + /// If a [fileSystem] is not provided, creates a new [MemoryFileSystem] instance. + @visibleForTesting + factory Artifacts.testLocalEngine({ + required String localEngine, + required String localEngineHost, + FileSystem? fileSystem, + }) { + return _TestLocalEngine( + localEngine, localEngineHost, fileSystem ?? MemoryFileSystem.test()); } static Artifacts getLocalEngine(EngineBuildPaths engineBuildPaths) { @@ -811,6 +825,7 @@ class CachedLocalEngineArtifacts implements Artifacts { localEngineInfo = LocalEngineInfo( engineOutPath: engineOutPath, + engineHostOutPath: _hostEngineOutPath, localEngineName: fileSystem.path.basename(engineOutPath) ), _cache = cache, @@ -1365,12 +1380,12 @@ class _TestArtifacts implements Artifacts { } class _TestLocalEngine extends _TestArtifacts { - _TestLocalEngine(String engineOutPath, super.fileSystem) : - localEngineInfo = - LocalEngineInfo( - engineOutPath: engineOutPath, - localEngineName: fileSystem.path.basename(engineOutPath) - ); + _TestLocalEngine( + String engineOutPath, String engineHostOutPath, super.fileSystem) + : localEngineInfo = LocalEngineInfo( + engineOutPath: engineOutPath, + engineHostOutPath: engineHostOutPath, + localEngineName: fileSystem.path.basename(engineOutPath)); @override bool get isLocalEngine => true; diff --git a/packages/flutter_tools/lib/src/ios/mac.dart b/packages/flutter_tools/lib/src/ios/mac.dart index 34c487e7ff..f51d436b97 100644 --- a/packages/flutter_tools/lib/src/ios/mac.dart +++ b/packages/flutter_tools/lib/src/ios/mac.dart @@ -72,6 +72,7 @@ class IMobileDevice { /// Create an [IMobileDevice] for testing. factory IMobileDevice.test({ required ProcessManager processManager }) { return IMobileDevice( + // ignore: invalid_use_of_visible_for_testing_member artifacts: Artifacts.test(), cache: Cache.test(processManager: processManager), processManager: processManager, diff --git a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart index 09f2c56444..4e302df9b8 100644 --- a/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart +++ b/packages/flutter_tools/test/commands.shard/hermetic/assemble_test.dart @@ -192,7 +192,7 @@ void main() { )); await commandRunner.run(['assemble', '-o Output', 'debug_macos_bundle_flutter_assets']); }, overrides: { - Artifacts: () => Artifacts.test(localEngine: 'out/host_release'), + Artifacts: () => Artifacts.testLocalEngine(localEngine: 'out/host_release', localEngineHost: 'out/host_release'), Cache: () => Cache.test(processManager: FakeProcessManager.any()), FileSystem: () => MemoryFileSystem.test(), ProcessManager: () => FakeProcessManager.any(), diff --git a/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart b/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart index b989d371f4..217a6911b0 100644 --- a/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart +++ b/packages/flutter_tools/test/general.shard/android/android_gradle_builder_test.dart @@ -1187,7 +1187,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_arm'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_arm', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1200,6 +1200,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_arm', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-arm', '-Ptarget=lib/main.dart', '-Pbase-application-name=io.flutter.app.FlutterApplication', @@ -1266,7 +1267,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_arm64'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_arm64', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1279,6 +1280,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_arm64', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-arm64', '-Ptarget=lib/main.dart', '-Pbase-application-name=io.flutter.app.FlutterApplication', @@ -1345,7 +1347,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_x86'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_x86', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1358,6 +1360,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_x86', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-x86', '-Ptarget=lib/main.dart', '-Pbase-application-name=io.flutter.app.FlutterApplication', @@ -1424,7 +1427,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_x64'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_x64', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1437,6 +1440,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_x64', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-x64', '-Ptarget=lib/main.dart', '-Pbase-application-name=io.flutter.app.FlutterApplication', @@ -1565,7 +1569,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_arm'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_arm', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1586,6 +1590,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_arm', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-arm', 'assembleAarRelease', ], @@ -1653,7 +1658,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_arm64'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_arm64', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1674,6 +1679,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_arm64', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-arm64', 'assembleAarRelease', ], @@ -1741,7 +1747,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_x86'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_x86', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1762,6 +1768,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_x86', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-x86', 'assembleAarRelease', ], @@ -1829,7 +1836,7 @@ unknown crash logger: logger, processManager: processManager, fileSystem: fileSystem, - artifacts: Artifacts.test(localEngine: 'out/android_x64'), + artifacts: Artifacts.testLocalEngine(localEngine: 'out/android_x64', localEngineHost: 'out/host_release'), usage: testUsage, gradleUtils: FakeGradleUtils(), platform: FakePlatform(), @@ -1850,6 +1857,7 @@ unknown crash '-Plocal-engine-repo=/.tmp_rand0/flutter_tool_local_engine_repo.rand0', '-Plocal-engine-build-mode=release', '-Plocal-engine-out=out/android_x64', + '-Plocal-engine-host-out=out/host_release', '-Ptarget-platform=android-x64', 'assembleAarRelease', ], diff --git a/packages/flutter_tools/test/general.shard/android/gradle_test.dart b/packages/flutter_tools/test/general.shard/android/gradle_test.dart index 5b32f92799..493d76cb38 100644 --- a/packages/flutter_tools/test/general.shard/android/gradle_test.dart +++ b/packages/flutter_tools/test/general.shard/android/gradle_test.dart @@ -211,7 +211,7 @@ void main() { setUp(() { fs = MemoryFileSystem.test(); - localEngineArtifacts = Artifacts.test(localEngine: 'out/android_arm'); + localEngineArtifacts = Artifacts.testLocalEngine(localEngine: 'out/android_arm', localEngineHost: 'out/host_release'); }); void testUsingAndroidContext(String description, dynamic Function() testMethod) { diff --git a/packages/flutter_tools/test/general.shard/build_info_test.dart b/packages/flutter_tools/test/general.shard/build_info_test.dart index af04ee243a..eef8e9c10f 100644 --- a/packages/flutter_tools/test/general.shard/build_info_test.dart +++ b/packages/flutter_tools/test/general.shard/build_info_test.dart @@ -104,17 +104,17 @@ void main() { testWithoutContext('defaultIOSArchsForEnvironment', () { expect(defaultIOSArchsForEnvironment( EnvironmentType.physical, - Artifacts.test(localEngine: 'ios_debug_unopt'), + Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'ios_debug_unopt'), ).single, DarwinArch.arm64); expect(defaultIOSArchsForEnvironment( EnvironmentType.simulator, - Artifacts.test(localEngine: 'ios_debug_sim_unopt'), + Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'ios_debug_sim_unopt'), ).single, DarwinArch.x86_64); expect(defaultIOSArchsForEnvironment( EnvironmentType.simulator, - Artifacts.test(localEngine: 'ios_debug_sim_unopt_arm64'), + Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'ios_debug_sim_unopt_arm64'), ).single, DarwinArch.arm64); expect(defaultIOSArchsForEnvironment( @@ -128,11 +128,11 @@ void main() { testWithoutContext('defaultMacOSArchsForEnvironment', () { expect(defaultMacOSArchsForEnvironment( - Artifacts.test(localEngine: 'host_debug_unopt'), + Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'host_debug_unopt'), ).single, DarwinArch.x86_64); expect(defaultMacOSArchsForEnvironment( - Artifacts.test(localEngine: 'host_debug_unopt_arm64'), + Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'host_debug_unopt_arm64'), ).single, DarwinArch.arm64); expect(defaultMacOSArchsForEnvironment( diff --git a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart index 3dfe9157d5..4dacb94625 100644 --- a/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart +++ b/packages/flutter_tools/test/general.shard/ios/xcodeproj_test.dart @@ -754,7 +754,7 @@ Information about project "Runner": setUp(() { fs = MemoryFileSystem.test(); - localIosArtifacts = Artifacts.test(localEngine: 'out/ios_profile_arm64'); + localIosArtifacts = Artifacts.testLocalEngine(localEngine: 'out/ios_profile_arm64', localEngineHost: 'out/host_release'); macOS = FakePlatform(operatingSystem: 'macos'); fs.file(xcodebuild).createSync(recursive: true); }); @@ -938,7 +938,7 @@ Build settings for action build and target plugin2: } testUsingOsxContext('exits when armv7 local engine is set', () async { - localIosArtifacts = Artifacts.test(localEngine: 'out/ios_profile_arm'); + localIosArtifacts = Artifacts.testLocalEngine(localEngine: 'out/ios_profile_arm', localEngineHost: 'out/host_release'); const BuildInfo buildInfo = BuildInfo.debug; final FlutterProject project = FlutterProject.fromDirectoryTest(fs.directory('path/to/project')); await expectLater(() => @@ -971,7 +971,7 @@ Build settings for action build and target plugin2: final String buildPhaseScriptContents = buildPhaseScript.readAsStringSync(); expect(buildPhaseScriptContents.contains('export "ARCHS=arm64"'), isTrue); }, overrides: { - Artifacts: () => Artifacts.test(localEngine: 'out/host_profile_arm64'), + Artifacts: () => Artifacts.testLocalEngine(localEngine: 'out/host_profile_arm64', localEngineHost: 'out/host_release'), Platform: () => macOS, FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(), @@ -998,7 +998,7 @@ Build settings for action build and target plugin2: final String buildPhaseScriptContents = buildPhaseScript.readAsStringSync(); expect(buildPhaseScriptContents.contains('export "ARCHS=x86_64"'), isTrue); }, overrides: { - Artifacts: () => Artifacts.test(localEngine: 'out/host_profile'), + Artifacts: () => Artifacts.testLocalEngine(localEngine: 'out/host_profile', localEngineHost: 'out/host_release'), Platform: () => macOS, FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(), @@ -1083,7 +1083,7 @@ Build settings for action build and target plugin2: final String buildPhaseScriptContents = buildPhaseScript.readAsStringSync(); expect(buildPhaseScriptContents.contains('ARCHS=x86_64'), isTrue); }, overrides: { - Artifacts: () => Artifacts.test(localEngine: 'out/ios_debug_sim_unopt'), + Artifacts: () => Artifacts.testLocalEngine(localEngine: 'out/ios_debug_sim_unopt', localEngineHost: 'out/host_debug_unopt'), Platform: () => macOS, FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(), @@ -1109,7 +1109,7 @@ Build settings for action build and target plugin2: final String buildPhaseScriptContents = buildPhaseScript.readAsStringSync(); expect(buildPhaseScriptContents.contains('ARCHS=arm64'), isTrue); }, overrides: { - Artifacts: () => Artifacts.test(localEngine: 'out/ios_debug_sim_arm64'), + Artifacts: () => Artifacts.testLocalEngine(localEngine: 'out/ios_debug_sim_arm64', localEngineHost: 'out/host_debug_unopt'), Platform: () => macOS, FileSystem: () => fs, ProcessManager: () => FakeProcessManager.any(),