Unverified Commit a021015e authored by Matan Lurey's avatar Matan Lurey Committed by GitHub

Always require `--local-engine-host` if `--local-engine` (#133003)

Closes #132245.
parent 97e434f8
...@@ -308,12 +308,10 @@ class UserMessages { ...@@ -308,12 +308,10 @@ class UserMessages {
"you have compiled the engine in that directory, which should produce an 'out' directory"; "you have compiled the engine in that directory, which should produce an 'out' directory";
String get runnerLocalEngineOrWebSdkRequired => String get runnerLocalEngineOrWebSdkRequired =>
'You must specify --local-engine or --local-web-sdk if you are using a locally built engine or web sdk.'; 'You must specify --local-engine or --local-web-sdk if you are using a locally built engine or web sdk.';
// TODO(matanlurey): Make this an error, https://github.com/flutter/flutter/issues/132245. String get runnerLocalEngineRequiresHostEngine =>
String runnerLocalEngineRequiresHostEngine({bool warning = false}) => 'You are using a locally built engine (--local-engine) but have not specified --local-engine-host.\n'
'${warning ? 'Warning! ' : ''}You are using a locally built engine (--local-engine) but have not specified --local-engine-host.\n'
'You may be building with a different engine than the one you are running with. ' 'You may be building with a different engine than the one you are running with. '
'See https://github.com/flutter/flutter/issues/132245 for details (in the future this will become ' 'See https://github.com/flutter/flutter/issues/132245 for details.';
'an error).';
String runnerNoEngineBuild(String engineBuildPath) => String runnerNoEngineBuild(String engineBuildPath) =>
'No Flutter engine build found at $engineBuildPath.'; 'No Flutter engine build found at $engineBuildPath.';
String runnerNoWebSdk(String webSdkPath) => String runnerNoWebSdk(String webSdkPath) =>
......
...@@ -276,11 +276,6 @@ Future<T> runInContext<T>( ...@@ -276,11 +276,6 @@ Future<T> runInContext<T>(
platform: globals.platform, platform: globals.platform,
fileSystem: globals.fs, fileSystem: globals.fs,
flutterRoot: Cache.flutterRoot!, flutterRoot: Cache.flutterRoot!,
// TODO(matanlurey): https://github.com/flutter/flutter/issues/132245.
// Even though we *think* this feature is stable, we'd like to verify
// that automated tests are all providing `--local-engine-host` before
// enforcing it for all users.
treatMissingLocalEngineHostAsFatal: runningOnBot,
), ),
Logger: () => globals.platform.isWindows Logger: () => globals.platform.isWindows
? WindowsStdoutLogger( ? WindowsStdoutLogger(
......
...@@ -33,20 +33,17 @@ class LocalEngineLocator { ...@@ -33,20 +33,17 @@ class LocalEngineLocator {
required FileSystem fileSystem, required FileSystem fileSystem,
required String flutterRoot, required String flutterRoot,
required UserMessages userMessages, required UserMessages userMessages,
bool treatMissingLocalEngineHostAsFatal = false,
}) : _platform = platform, }) : _platform = platform,
_logger = logger, _logger = logger,
_fileSystem = fileSystem, _fileSystem = fileSystem,
_flutterRoot = flutterRoot, _flutterRoot = flutterRoot,
_userMessages = userMessages, _userMessages = userMessages;
_treatMissingLocalEngineHostAsFatal = treatMissingLocalEngineHostAsFatal;
final Platform _platform; final Platform _platform;
final Logger _logger; final Logger _logger;
final FileSystem _fileSystem; final FileSystem _fileSystem;
final String _flutterRoot; final String _flutterRoot;
final UserMessages _userMessages; final UserMessages _userMessages;
final bool _treatMissingLocalEngineHostAsFatal;
/// Returns the engine build path of a local engine if one is located, otherwise `null`. /// Returns the engine build path of a local engine if one is located, otherwise `null`.
Future<EngineBuildPaths?> findEnginePath({ Future<EngineBuildPaths?> findEnginePath({
...@@ -170,26 +167,6 @@ class LocalEngineLocator { ...@@ -170,26 +167,6 @@ class LocalEngineLocator {
return engineSourcePath; return engineSourcePath;
} }
// Determine the host engine directory associated with the local engine:
// Strip '_sim' since there are no host simulator builds.
String _getHostEngineBasename(String localEngineBasename) {
if (localEngineBasename.startsWith('web_') ||
localEngineBasename.startsWith('wasm_') ||
localEngineBasename.startsWith('host_')) {
// Don't modify the web or host local engine's basename.
return localEngineBasename;
}
String tmpBasename = localEngineBasename.replaceFirst('_sim', '');
tmpBasename = tmpBasename.substring(tmpBasename.indexOf('_') + 1);
// Strip suffix for various archs.
const List<String> suffixes = <String>['_arm', '_arm64', '_x86', '_x64'];
for (final String suffix in suffixes) {
tmpBasename = tmpBasename.replaceFirst(RegExp('$suffix\$'), '');
}
return 'host_$tmpBasename';
}
EngineBuildPaths _findEngineBuildPath({ EngineBuildPaths _findEngineBuildPath({
required String engineSourcePath, required String engineSourcePath,
String? localEngine, String? localEngine,
...@@ -209,16 +186,10 @@ class LocalEngineLocator { ...@@ -209,16 +186,10 @@ class LocalEngineLocator {
} }
if (localHostEngine == null) { if (localHostEngine == null) {
// TODO(matanlurey): https://github.com/flutter/flutter/issues/132245, always throwToolExit. throwToolExit(_userMessages.runnerLocalEngineRequiresHostEngine);
if (_treatMissingLocalEngineHostAsFatal) {
throwToolExit(_userMessages.runnerLocalEngineRequiresHostEngine());
}
_logger.printStatus(_userMessages.runnerLocalEngineRequiresHostEngine(warning: true));
} }
final String basename = localHostEngine ?? _fileSystem.path.basename(engineBuildPath);
final String hostBasename = _getHostEngineBasename(basename);
engineHostBuildPath = _fileSystem.path.normalize( engineHostBuildPath = _fileSystem.path.normalize(
_fileSystem.path.join(_fileSystem.path.dirname(engineBuildPath), hostBasename), _fileSystem.path.join(_fileSystem.path.dirname(engineBuildPath), localHostEngine),
); );
if (!_fileSystem.isDirectorySync(engineHostBuildPath)) { if (!_fileSystem.isDirectorySync(engineHostBuildPath)) {
throwToolExit(_userMessages.runnerNoEngineBuild(engineHostBuildPath), exitCode: 2); throwToolExit(_userMessages.runnerNoEngineBuild(engineHostBuildPath), exitCode: 2);
......
...@@ -43,7 +43,7 @@ void main() { ...@@ -43,7 +43,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: 'ios_debug'), await localEngineLocator.findEnginePath(localEngine: 'ios_debug', localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug', hostEngine: '/arbitrary/engine/src/out/host_debug',
targetEngine: '/arbitrary/engine/src/out/ios_debug', targetEngine: '/arbitrary/engine/src/out/ios_debug',
...@@ -58,7 +58,7 @@ void main() { ...@@ -58,7 +58,7 @@ void main() {
.writeAsStringSync('sky_engine:file:///symlink/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/'); .writeAsStringSync('sky_engine:file:///symlink/src/out/ios_debug/gen/dart-pkg/sky_engine/lib/');
expect( expect(
await localEngineLocator.findEnginePath(localEngine: 'ios_debug'), await localEngineLocator.findEnginePath(localEngine: 'ios_debug', localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/symlink/src/out/host_debug', hostEngine: '/symlink/src/out/host_debug',
targetEngine: '/symlink/src/out/ios_debug', targetEngine: '/symlink/src/out/ios_debug',
...@@ -84,7 +84,7 @@ void main() { ...@@ -84,7 +84,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(engineSourcePath: '$kArbitraryEngineRoot/src', localEngine: 'ios_debug'), await localEngineLocator.findEnginePath(engineSourcePath: '$kArbitraryEngineRoot/src', localEngine: 'ios_debug', localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug', hostEngine: '/arbitrary/engine/src/out/host_debug',
targetEngine: '/arbitrary/engine/src/out/ios_debug', targetEngine: '/arbitrary/engine/src/out/ios_debug',
...@@ -119,7 +119,7 @@ void main() { ...@@ -119,7 +119,7 @@ void main() {
expect(logger.traceText, contains('Local engine source at /arbitrary/engine/src')); expect(logger.traceText, contains('Local engine source at /arbitrary/engine/src'));
}); });
testWithoutContext('works but produces a warning if --local-engine is specified but not --local-engine-host', () async { testWithoutContext('fails if --local-engine-host is omitted', () async {
final FileSystem fileSystem = MemoryFileSystem.test(); final FileSystem fileSystem = MemoryFileSystem.test();
final Directory localEngine = fileSystem final Directory localEngine = fileSystem
.directory('$kArbitraryEngineRoot/src/out/android_debug_unopt_arm64/') .directory('$kArbitraryEngineRoot/src/out/android_debug_unopt_arm64/')
...@@ -135,33 +135,6 @@ void main() { ...@@ -135,33 +135,6 @@ void main() {
platform: FakePlatform(environment: <String, String>{}), platform: FakePlatform(environment: <String, String>{}),
); );
expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path),
matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug_unopt',
targetEngine: '/arbitrary/engine/src/out/android_debug_unopt_arm64',
),
);
expect(logger.statusText, contains('Warning! You are using a locally built engine (--local-engine) but have not specified --local-engine-host'));
});
testWithoutContext('fails if --local-engine-host is emitted and treatMissingLocalEngineHostAsFatal is set', () async {
final FileSystem fileSystem = MemoryFileSystem.test();
final Directory localEngine = fileSystem
.directory('$kArbitraryEngineRoot/src/out/android_debug_unopt_arm64/')
..createSync(recursive: true);
fileSystem.directory('$kArbitraryEngineRoot/src/out/host_debug_unopt/').createSync(recursive: true);
final BufferLogger logger = BufferLogger.test();
final LocalEngineLocator localEngineLocator = LocalEngineLocator(
fileSystem: fileSystem,
flutterRoot: 'flutter/flutter',
logger: logger,
userMessages: UserMessages(),
platform: FakePlatform(environment: <String, String>{}),
treatMissingLocalEngineHostAsFatal: true,
);
await expectLater( await expectLater(
localEngineLocator.findEnginePath(localEngine: localEngine.path), localEngineLocator.findEnginePath(localEngine: localEngine.path),
throwsToolExit(message: 'You are using a locally built engine (--local-engine) but have not specified --local-engine-host'), throwsToolExit(message: 'You are using a locally built engine (--local-engine) but have not specified --local-engine-host'),
...@@ -186,7 +159,7 @@ void main() { ...@@ -186,7 +159,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path), await localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug', hostEngine: '/arbitrary/engine/src/out/host_debug',
targetEngine: '/arbitrary/engine/src/out/ios_debug', targetEngine: '/arbitrary/engine/src/out/ios_debug',
...@@ -212,7 +185,7 @@ void main() { ...@@ -212,7 +185,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path), await localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: localEngine.path),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug', hostEngine: '/arbitrary/engine/src/out/host_debug',
targetEngine: '/arbitrary/engine/src/out/host_debug', targetEngine: '/arbitrary/engine/src/out/host_debug',
...@@ -237,7 +210,7 @@ void main() { ...@@ -237,7 +210,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path), await localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: localEngine.path),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug_unopt_arm64', hostEngine: '/arbitrary/engine/src/out/host_debug_unopt_arm64',
targetEngine: '/arbitrary/engine/src/out/host_debug_unopt_arm64', targetEngine: '/arbitrary/engine/src/out/host_debug_unopt_arm64',
...@@ -264,7 +237,7 @@ void main() { ...@@ -264,7 +237,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path), await localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug', hostEngine: '/arbitrary/engine/src/out/host_debug',
targetEngine: '/arbitrary/engine/src/out/ios_debug_sim', targetEngine: '/arbitrary/engine/src/out/ios_debug_sim',
...@@ -292,7 +265,7 @@ void main() { ...@@ -292,7 +265,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: localEngine.path), await localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: 'host_debug_unopt'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/host_debug_unopt', hostEngine: '/arbitrary/engine/src/out/host_debug_unopt',
targetEngine: '/arbitrary/engine/src/out/ios_debug_sim_unopt', targetEngine: '/arbitrary/engine/src/out/ios_debug_sim_unopt',
...@@ -315,7 +288,7 @@ void main() { ...@@ -315,7 +288,7 @@ void main() {
); );
await expectToolExitLater( await expectToolExitLater(
localEngineLocator.findEnginePath(localEngine: localEngine.path), localEngineLocator.findEnginePath(localEngine: localEngine.path, localHostEngine: 'host_debug'),
contains('No Flutter engine build found at /arbitrary/engine/src/out/host_debug'), contains('No Flutter engine build found at /arbitrary/engine/src/out/host_debug'),
); );
}); });
...@@ -344,7 +317,7 @@ void main() { ...@@ -344,7 +317,7 @@ void main() {
); );
expect( expect(
await localEngineLocator.findEnginePath(localEngine: 'ios_debug'), await localEngineLocator.findEnginePath(localEngine: 'ios_debug', localHostEngine: 'host_debug'),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: 'flutter/engine/src/out/host_debug', hostEngine: 'flutter/engine/src/out/host_debug',
targetEngine: 'flutter/engine/src/out/ios_debug', targetEngine: 'flutter/engine/src/out/ios_debug',
...@@ -366,7 +339,7 @@ void main() { ...@@ -366,7 +339,7 @@ void main() {
); );
await expectToolExitLater( await expectToolExitLater(
localEngineLocator.findEnginePath(localEngine: '/path/to/nothing'), localEngineLocator.findEnginePath(localEngine: '/path/to/nothing', localHostEngine: '/path/to/nothing'),
contains('Unable to detect local Flutter engine src directory'), contains('Unable to detect local Flutter engine src directory'),
); );
}); });
...@@ -390,7 +363,7 @@ void main() { ...@@ -390,7 +363,7 @@ void main() {
); );
expect( expect(
await localWasmEngineLocator.findEnginePath(localEngine: localWasmEngine.path), await localWasmEngineLocator.findEnginePath(localEngine: localWasmEngine.path, localHostEngine: localWasmEngine.path),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/wasm_whatever', hostEngine: '/arbitrary/engine/src/out/wasm_whatever',
targetEngine: '/arbitrary/engine/src/out/wasm_whatever', targetEngine: '/arbitrary/engine/src/out/wasm_whatever',
...@@ -408,7 +381,7 @@ void main() { ...@@ -408,7 +381,7 @@ void main() {
); );
expect( expect(
await localWebEngineLocator.findEnginePath(localEngine: localWebEngine.path), await localWebEngineLocator.findEnginePath(localEngine: localWebEngine.path, localHostEngine: localWebEngine.path),
matchesEngineBuildPaths( matchesEngineBuildPaths(
hostEngine: '/arbitrary/engine/src/out/web_whatever', hostEngine: '/arbitrary/engine/src/out/web_whatever',
targetEngine: '/arbitrary/engine/src/out/web_whatever', targetEngine: '/arbitrary/engine/src/out/web_whatever',
......
Markdown is supported
0% or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment