From 223ae5d3acff1fd97d4dd0f5ea66b1bcf8b04c61 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 16 Aug 2023 15:24:51 -0700 Subject: [PATCH] Add support for LOCAL_ENGINE_HOST to Linux/Mac/Win builds. (#132579) Partial work towards https://github.com/flutter/flutter/issues/132245. I also couldn't help myself to do a very minor refactor and add some comments to `LocalEngineInfo` because I was getting confused myself implementing it. --- .../flutter_tools/lib/src/android/gradle.dart | 20 ++--- packages/flutter_tools/lib/src/artifacts.dart | 82 ++++++++++++------- .../flutter_tools/lib/src/build_info.dart | 4 +- .../lib/src/ios/xcode_build_settings.dart | 7 +- .../lib/src/linux/build_linux.dart | 8 +- .../lib/src/windows/build_windows.dart | 8 +- .../test/general.shard/artifacts_test.dart | 35 +++++++- .../test/general.shard/build_info_test.dart | 17 +++- 8 files changed, 127 insertions(+), 54 deletions(-) diff --git a/packages/flutter_tools/lib/src/android/gradle.dart b/packages/flutter_tools/lib/src/android/gradle.dart index 3a456dc48f..6341cc173d 100644 --- a/packages/flutter_tools/lib/src/android/gradle.dart +++ b/packages/flutter_tools/lib/src/android/gradle.dart @@ -390,20 +390,20 @@ class AndroidGradleBuilder implements AndroidBuilder { final LocalEngineInfo? localEngineInfo = _artifacts.localEngineInfo; if (localEngineInfo != null) { final Directory localEngineRepo = _getLocalEngineRepo( - engineOutPath: localEngineInfo.engineOutPath, + engineOutPath: localEngineInfo.targetOutPath, androidBuildInfo: androidBuildInfo, fileSystem: _fileSystem, ); _logger.printTrace( - 'Using local engine: ${localEngineInfo.engineOutPath}\n' + 'Using local engine: ${localEngineInfo.targetOutPath}\n' 'Local Maven repo: ${localEngineRepo.path}' ); 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('-Plocal-engine-out=${localEngineInfo.targetOutPath}'); + command.add('-Plocal-engine-host-out=${localEngineInfo.hostOutPath}'); command.add('-Ptarget-platform=${_getTargetPlatformByLocalEnginePath( - localEngineInfo.engineOutPath)}'); + localEngineInfo.targetOutPath)}'); } else if (androidBuildInfo.targetArchs.isNotEmpty) { final String targetPlatforms = androidBuildInfo .targetArchs @@ -716,18 +716,18 @@ class AndroidGradleBuilder implements AndroidBuilder { final LocalEngineInfo? localEngineInfo = _artifacts.localEngineInfo; if (localEngineInfo != null) { final Directory localEngineRepo = _getLocalEngineRepo( - engineOutPath: localEngineInfo.engineOutPath, + engineOutPath: localEngineInfo.targetOutPath, androidBuildInfo: androidBuildInfo, fileSystem: _fileSystem, ); _logger.printTrace( - 'Using local engine: ${localEngineInfo.engineOutPath}\n' + 'Using local engine: ${localEngineInfo.targetOutPath}\n' 'Local Maven repo: ${localEngineRepo.path}' ); 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('-Plocal-engine-out=${localEngineInfo.targetOutPath}'); + command.add('-Plocal-engine-host-out=${localEngineInfo.hostOutPath}'); // Copy the local engine repo in the output directory. try { @@ -742,7 +742,7 @@ class AndroidGradleBuilder implements AndroidBuilder { ); } command.add('-Ptarget-platform=${_getTargetPlatformByLocalEnginePath( - localEngineInfo.engineOutPath)}'); + localEngineInfo.targetOutPath)}'); } else if (androidBuildInfo.targetArchs.isNotEmpty) { final String targetPlatforms = androidBuildInfo.targetArchs .map((AndroidArch e) => e.platformName).join(','); diff --git a/packages/flutter_tools/lib/src/artifacts.dart b/packages/flutter_tools/lib/src/artifacts.dart index 97e6e1da9f..8775db6fec 100644 --- a/packages/flutter_tools/lib/src/artifacts.dart +++ b/packages/flutter_tools/lib/src/artifacts.dart @@ -284,17 +284,40 @@ class EngineBuildPaths { final String? webSdk; } -/// Information about a local engine build +/// Information about a local engine build (i.e. `--local-engine[-host]=...`). +/// +/// See https://github.com/flutter/flutter/wiki/The-flutter-tool#using-a-locally-built-engine-with-the-flutter-tool +/// for more information about local engine builds. class LocalEngineInfo { + /// Creates a reference to a local engine build. + /// + /// The [targetOutPath] and [hostOutPath] are assumed to be resolvable + /// paths to the built engine artifacts for the target (device) and host + /// (build) platforms, respectively. const LocalEngineInfo({ - required this.engineOutPath, - required this.engineHostOutPath, - required this.localEngineName, + required this.targetOutPath, + required this.hostOutPath, }); - final String engineOutPath; - final String engineHostOutPath; - final String localEngineName; + /// The path to the engine artifacts for the target (device) platform. + /// + /// For example, if the target platform is Android debug, this would be a path + /// like `/path/to/engine/src/out/android_debug_unopt`. To retrieve just the + /// name (platform), see [localTargetName]. + final String targetOutPath; + + /// The path to the engine artifacts for the host (build) platform. + /// + /// For example, if the host platform is debug, this would be a path like + /// `/path/to/engine/src/out/host_debug_unopt`. To retrieve just the name + /// (platform), see [localHostName]. + final String hostOutPath; + + /// The name of the target (device) platform, i.e. `android_debug_unopt`. + String get localTargetName => globals.fs.path.basename(targetOutPath); + + /// The name of the host (build) platform, e.g. `host_debug_unopt`. + String get localHostName => globals.fs.path.basename(hostOutPath); } // Manages the engine artifacts of Flutter. @@ -824,9 +847,8 @@ class CachedLocalEngineArtifacts implements Artifacts { }) : _fileSystem = fileSystem, localEngineInfo = LocalEngineInfo( - engineOutPath: engineOutPath, - engineHostOutPath: _hostEngineOutPath, - localEngineName: fileSystem.path.basename(engineOutPath) + targetOutPath: engineOutPath, + hostOutPath: _hostEngineOutPath, ), _cache = cache, _processManager = processManager, @@ -937,28 +959,28 @@ class CachedLocalEngineArtifacts implements Artifacts { return _flutterTesterPath(platform!); case Artifact.isolateSnapshotData: case Artifact.vmSnapshotData: - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'gen', 'flutter', 'lib', 'snapshot', artifactFileName); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'gen', 'flutter', 'lib', 'snapshot', artifactFileName); case Artifact.icuData: case Artifact.flutterXcframework: case Artifact.flutterMacOSFramework: - return _fileSystem.path.join(localEngineInfo.engineOutPath, artifactFileName); + return _fileSystem.path.join(localEngineInfo.targetOutPath, artifactFileName); case Artifact.platformKernelDill: if (platform == TargetPlatform.fuchsia_x64 || platform == TargetPlatform.fuchsia_arm64) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter_runner_patched_sdk', artifactFileName); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter_runner_patched_sdk', artifactFileName); } return _fileSystem.path.join(_getFlutterPatchedSdkPath(mode), artifactFileName); case Artifact.platformLibrariesJson: return _fileSystem.path.join(_getFlutterPatchedSdkPath(mode), 'lib', artifactFileName); case Artifact.flutterFramework: return _getIosEngineArtifactPath( - localEngineInfo.engineOutPath, environmentType, _fileSystem, _platform); + localEngineInfo.targetOutPath, environmentType, _fileSystem, _platform); case Artifact.flutterPatchedSdkPath: // When using local engine always use [BuildMode.debug] regardless of // what was specified in [mode] argument because local engine will // have only one flutter_patched_sdk in standard location, that // is happen to be what debug(non-release) mode is using. if (platform == TargetPlatform.fuchsia_x64 || platform == TargetPlatform.fuchsia_arm64) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter_runner_patched_sdk'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter_runner_patched_sdk'); } return _getFlutterPatchedSdkPath(BuildMode.debug); case Artifact.skyEnginePath: @@ -967,11 +989,11 @@ class CachedLocalEngineArtifacts implements Artifacts { final String hostPlatform = getNameForHostPlatform(getCurrentHostPlatform()); final String modeName = mode!.isRelease ? 'release' : mode.toString(); final String dartBinaries = 'dart_binaries-$modeName-$hostPlatform'; - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'host_bundle', dartBinaries, 'kernel_compiler.dart.snapshot'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'host_bundle', dartBinaries, 'kernel_compiler.dart.snapshot'); case Artifact.fuchsiaFlutterRunner: final String jitOrAot = mode!.isJit ? '_jit' : '_aot'; final String productOrNo = mode.isRelease ? '_product' : ''; - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter$jitOrAot${productOrNo}_runner-0.far'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter$jitOrAot${productOrNo}_runner-0.far'); case Artifact.fontSubset: return _fileSystem.path.join(_hostEngineOutPath, artifactFileName); case Artifact.constFinder: @@ -999,11 +1021,11 @@ class CachedLocalEngineArtifacts implements Artifacts { @override String getEngineType(TargetPlatform platform, [ BuildMode? mode ]) { - return _fileSystem.path.basename(localEngineInfo.engineOutPath); + return _fileSystem.path.basename(localEngineInfo.targetOutPath); } String _getFlutterPatchedSdkPath(BuildMode? buildMode) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, + return _fileSystem.path.join(localEngineInfo.targetOutPath, buildMode == BuildMode.release ? 'flutter_patched_sdk_product' : 'flutter_patched_sdk'); } @@ -1053,14 +1075,14 @@ class CachedLocalEngineArtifacts implements Artifacts { } String _getFlutterWebSdkPath() { - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter_web_sdk'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter_web_sdk'); } String _genSnapshotPath() { const List clangDirs = ['.', 'clang_x64', 'clang_x86', 'clang_i386', 'clang_arm64']; final String genSnapshotName = _artifactToFileName(Artifact.genSnapshot, _platform)!; for (final String clangDir in clangDirs) { - final String genSnapshotPath = _fileSystem.path.join(localEngineInfo.engineOutPath, clangDir, genSnapshotName); + final String genSnapshotPath = _fileSystem.path.join(localEngineInfo.targetOutPath, clangDir, genSnapshotName); if (_processManager.canRun(genSnapshotPath)) { return genSnapshotPath; } @@ -1070,11 +1092,11 @@ class CachedLocalEngineArtifacts implements Artifacts { String _flutterTesterPath(TargetPlatform platform) { if (_platform.isLinux) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, _artifactToFileName(Artifact.flutterTester, _platform)); + return _fileSystem.path.join(localEngineInfo.targetOutPath, _artifactToFileName(Artifact.flutterTester, _platform)); } else if (_platform.isMacOS) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter_tester'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter_tester'); } else if (_platform.isWindows) { - return _fileSystem.path.join(localEngineInfo.engineOutPath, 'flutter_tester.exe'); + return _fileSystem.path.join(localEngineInfo.targetOutPath, 'flutter_tester.exe'); } throw Exception('Unsupported platform $platform.'); } @@ -1381,11 +1403,13 @@ class _TestArtifacts implements Artifacts { class _TestLocalEngine extends _TestArtifacts { _TestLocalEngine( - String engineOutPath, String engineHostOutPath, super.fileSystem) - : localEngineInfo = LocalEngineInfo( - engineOutPath: engineOutPath, - engineHostOutPath: engineHostOutPath, - localEngineName: fileSystem.path.basename(engineOutPath)); + String engineOutPath, + String engineHostOutPath, + super.fileSystem, + ) : localEngineInfo = LocalEngineInfo( + targetOutPath: engineOutPath, + hostOutPath: engineHostOutPath, + ); @override bool get isLocalEngine => true; diff --git a/packages/flutter_tools/lib/src/build_info.dart b/packages/flutter_tools/lib/src/build_info.dart index 053197c8f1..5dcc2948b1 100644 --- a/packages/flutter_tools/lib/src/build_info.dart +++ b/packages/flutter_tools/lib/src/build_info.dart @@ -647,7 +647,7 @@ List defaultIOSArchsForEnvironment( // Handle single-arch local engines. final LocalEngineInfo? localEngineInfo = artifacts.localEngineInfo; if (localEngineInfo != null) { - final String localEngineName = localEngineInfo.localEngineName; + final String localEngineName = localEngineInfo.localTargetName; if (localEngineName.contains('_arm64')) { return [ DarwinArch.arm64 ]; } @@ -670,7 +670,7 @@ List defaultMacOSArchsForEnvironment(Artifacts artifacts) { // Handle single-arch local engines. final LocalEngineInfo? localEngineInfo = artifacts.localEngineInfo; if (localEngineInfo != null) { - if (localEngineInfo.localEngineName.contains('_arm64')) { + if (localEngineInfo.localTargetName.contains('_arm64')) { return [ DarwinArch.arm64 ]; } return [ DarwinArch.x86_64 ]; diff --git a/packages/flutter_tools/lib/src/ios/xcode_build_settings.dart b/packages/flutter_tools/lib/src/ios/xcode_build_settings.dart index 0e7c42b9f3..df53b38695 100644 --- a/packages/flutter_tools/lib/src/ios/xcode_build_settings.dart +++ b/packages/flutter_tools/lib/src/ios/xcode_build_settings.dart @@ -181,12 +181,15 @@ Future> _xcodeBuildSettingsLines({ final LocalEngineInfo? localEngineInfo = globals.artifacts?.localEngineInfo; if (localEngineInfo != null) { - final String engineOutPath = localEngineInfo.engineOutPath; + final String engineOutPath = localEngineInfo.targetOutPath; xcodeBuildSettings.add('FLUTTER_ENGINE=${globals.fs.path.dirname(globals.fs.path.dirname(engineOutPath))}'); - final String localEngineName = localEngineInfo.localEngineName; + final String localEngineName = localEngineInfo.localTargetName; xcodeBuildSettings.add('LOCAL_ENGINE=$localEngineName'); + final String localEngineHostName = localEngineInfo.localHostName; + xcodeBuildSettings.add('LOCAL_ENGINE_HOST=$localEngineHostName'); + // Tell Xcode not to build universal binaries for local engines, which are // single-architecture. // diff --git a/packages/flutter_tools/lib/src/linux/build_linux.dart b/packages/flutter_tools/lib/src/linux/build_linux.dart index 81a8d775dd..410416aa63 100644 --- a/packages/flutter_tools/lib/src/linux/build_linux.dart +++ b/packages/flutter_tools/lib/src/linux/build_linux.dart @@ -56,9 +56,11 @@ Future buildLinux( environmentConfig['FLUTTER_TARGET'] = target; final LocalEngineInfo? localEngineInfo = globals.artifacts?.localEngineInfo; if (localEngineInfo != null) { - final String engineOutPath = localEngineInfo.engineOutPath; - environmentConfig['FLUTTER_ENGINE'] = globals.fs.path.dirname(globals.fs.path.dirname(engineOutPath)); - environmentConfig['LOCAL_ENGINE'] = localEngineInfo.localEngineName; + final String targetOutPath = localEngineInfo.targetOutPath; + // $ENGINE/src/out/foo_bar_baz -> $ENGINE/src + environmentConfig['FLUTTER_ENGINE'] = globals.fs.path.dirname(globals.fs.path.dirname(targetOutPath)); + environmentConfig['LOCAL_ENGINE'] = localEngineInfo.localTargetName; + environmentConfig['LOCAL_ENGINE_HOST'] = localEngineInfo.localHostName; } writeGeneratedCmakeConfig(Cache.flutterRoot!, linuxProject, buildInfo, environmentConfig, logger); diff --git a/packages/flutter_tools/lib/src/windows/build_windows.dart b/packages/flutter_tools/lib/src/windows/build_windows.dart index c63b1dc3bc..a665856d0c 100644 --- a/packages/flutter_tools/lib/src/windows/build_windows.dart +++ b/packages/flutter_tools/lib/src/windows/build_windows.dart @@ -247,9 +247,11 @@ void _writeGeneratedFlutterConfig( }; final LocalEngineInfo? localEngineInfo = globals.artifacts?.localEngineInfo; if (localEngineInfo != null) { - final String engineOutPath = localEngineInfo.engineOutPath; - environment['FLUTTER_ENGINE'] = globals.fs.path.dirname(globals.fs.path.dirname(engineOutPath)); - environment['LOCAL_ENGINE'] = localEngineInfo.localEngineName; + final String targetOutPath = localEngineInfo.targetOutPath; + // Get the engine source root $ENGINE/src/out/foo_bar_baz -> $ENGINE/src + environment['FLUTTER_ENGINE'] = globals.fs.path.dirname(globals.fs.path.dirname(targetOutPath)); + environment['LOCAL_ENGINE'] = localEngineInfo.localTargetName; + environment['LOCAL_ENGINE_HOST'] = localEngineInfo.localHostName; } writeGeneratedCmakeConfig(Cache.flutterRoot!, windowsProject, buildInfo, environment, globals.logger); } diff --git a/packages/flutter_tools/test/general.shard/artifacts_test.dart b/packages/flutter_tools/test/general.shard/artifacts_test.dart index b6afa09955..920980a23b 100644 --- a/packages/flutter_tools/test/general.shard/artifacts_test.dart +++ b/packages/flutter_tools/test/general.shard/artifacts_test.dart @@ -11,7 +11,7 @@ import 'package:flutter_tools/src/build_info.dart'; import 'package:flutter_tools/src/cache.dart'; import '../src/common.dart'; -import '../src/fake_process_manager.dart'; +import '../src/context.dart'; import '../src/fakes.dart'; void main() { @@ -545,4 +545,37 @@ void main() { ); }); }); + + group('LocalEngineInfo', () { + late FileSystem fileSystem; + late LocalEngineInfo localEngineInfo; + + setUp(() { + fileSystem = MemoryFileSystem.test(); + }); + + testUsingContext('determines the target device name from the path', () { + localEngineInfo = LocalEngineInfo( + targetOutPath: fileSystem.path.join(fileSystem.currentDirectory.path, 'out', 'android_debug_unopt'), + hostOutPath: fileSystem.path.join(fileSystem.currentDirectory.path, 'out', 'host_debug_unopt'), + ); + + expect(localEngineInfo.localTargetName, 'android_debug_unopt'); + }, overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); + + testUsingContext('determines the target device name from the path when using a custom engine path', () { + localEngineInfo = LocalEngineInfo( + targetOutPath: fileSystem.path.join(fileSystem.currentDirectory.path, 'out', 'android_debug_unopt'), + hostOutPath: fileSystem.path.join(fileSystem.currentDirectory.path, 'out', 'host_debug_unopt'), + ); + + expect(localEngineInfo.localHostName, 'host_debug_unopt'); + }, overrides: { + FileSystem: () => fileSystem, + ProcessManager: () => FakeProcessManager.any(), + }); + }); } 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 eef8e9c10f..929ce64cd4 100644 --- a/packages/flutter_tools/test/general.shard/build_info_test.dart +++ b/packages/flutter_tools/test/general.shard/build_info_test.dart @@ -2,7 +2,10 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +import 'package:file/memory.dart'; + import 'package:flutter_tools/src/artifacts.dart'; +import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/logger.dart'; import 'package:flutter_tools/src/build_info.dart'; @@ -101,7 +104,7 @@ void main() { expect(getNameForTargetPlatform(TargetPlatform.android), isNot(contains('ios'))); }); - testWithoutContext('defaultIOSArchsForEnvironment', () { + testUsingContext('defaultIOSArchsForEnvironment', () { expect(defaultIOSArchsForEnvironment( EnvironmentType.physical, Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'ios_debug_unopt'), @@ -124,9 +127,12 @@ void main() { expect(defaultIOSArchsForEnvironment( EnvironmentType.simulator, Artifacts.test(), ), [ DarwinArch.x86_64, DarwinArch.arm64 ]); - }); + }, overrides: { + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + }); - testWithoutContext('defaultMacOSArchsForEnvironment', () { + testUsingContext('defaultMacOSArchsForEnvironment', () { expect(defaultMacOSArchsForEnvironment( Artifacts.testLocalEngine(localEngineHost: 'host_debug_unopt', localEngine: 'host_debug_unopt'), ).single, DarwinArch.x86_64); @@ -138,7 +144,10 @@ void main() { expect(defaultMacOSArchsForEnvironment( Artifacts.test(), ), [ DarwinArch.x86_64, DarwinArch.arm64 ]); - }); + }, overrides: { + FileSystem: () => MemoryFileSystem.test(), + ProcessManager: () => FakeProcessManager.any(), + }); testWithoutContext('getIOSArchForName on Darwin arches', () { expect(getIOSArchForName('armv7'), DarwinArch.armv7);