Skip to content

Commit 43d438f

Browse files
authored
Build hooks: Don't require toolchain for unit tests (flutter#178954)
To ensure build hooks emit code assets that are compatible with the main app, Flutter tools pass a `CCompilerConfig` toolchain configuration object to hooks. This is generally what we want, hooks on macOS should use the same `clang` from XCode as the one used to compile the app and native Flutter plugins for instance. In some cases however, we need to run hooks without necessarily compiling a full Flutter app with native sources. A good example for this is `flutter test`, which runs unit / widget tests in a regular Dart VM without embedding it in a Flutter application. So since `flutter test` wouldn't invoke a native compiler, running build hooks shouldn't fail if the expected toolchain is missing. Currently however, `flutter test` tries to resolve a compiler toolchain for the host platform. Doing that on Windows already allows not passing a `CCompilerConfig` if VSCode wasn't found, but on macOS and Linux, this crashes. This fixes the issue by allowing those methods to return `null` instead of throwing. They still throw by default, but for the test target they are configured to not pass a toolchain to hooks if none could be resolved. This means that hooks not invoking the provided toolchain (say because they're only downloading native artifacts instead) would now work, whereas previously `flutter test` would crash if no toolchian was found. This closes flutter#178715 (but only the part shared in the original issue description, @dcharkes suggested fixing a similar issue in the same PR but that is _not_ done here).
1 parent c70e825 commit 43d438f

File tree

6 files changed

+218
-36
lines changed

6 files changed

+218
-36
lines changed

packages/flutter_tools/lib/src/isolated/native_assets/linux/native_assets.dart

Lines changed: 44 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -9,10 +9,16 @@ import '../../../base/file_system.dart';
99
import '../../../base/io.dart';
1010
import '../../../globals.dart' as globals;
1111

12-
/// Flutter expects `clang++` to be on the path on Linux hosts.
12+
/// Returns a [CCompilerConfig] matching a toolchain that would be used to compile the main app with
13+
/// CMake on Linux.
1314
///
14-
/// Search for the accompanying `clang`, `ar`, and `ld`.
15-
Future<CCompilerConfig> cCompilerConfigLinux() async {
15+
/// Flutter expects `clang++` to be on the path on Linux hosts, which this uses to search for the
16+
/// accompanying `clang`, `ar`, and `ld`.
17+
///
18+
/// If [throwIfNotFound] is false, this is allowed to fail (in which case `null`) is returned. This
19+
/// is used for `flutter test` setups, where no main app is compiled and we thus don't want a
20+
/// `clang` toolchain to be a requirement.
21+
Future<CCompilerConfig?> cCompilerConfigLinux({required bool throwIfNotFound}) async {
1622
const kClangPlusPlusBinary = 'clang++';
1723
// NOTE: these binaries sometimes have different names depending on the installation;
1824
// thus, we check for a few possible options (in order of preference).
@@ -25,30 +31,53 @@ Future<CCompilerConfig> cCompilerConfigLinux() async {
2531
kClangPlusPlusBinary,
2632
]);
2733
if (whichResult.exitCode != 0) {
28-
throwToolExit('Failed to find $kClangPlusPlusBinary on PATH.');
34+
if (throwIfNotFound) {
35+
throwToolExit('Failed to find $kClangPlusPlusBinary on PATH.');
36+
} else {
37+
return null;
38+
}
2939
}
3040
File clangPpFile = globals.fs.file((whichResult.stdout as String).trim());
3141
clangPpFile = globals.fs.file(await clangPpFile.resolveSymbolicLinks());
3242

3343
final Directory clangDir = clangPpFile.parent;
34-
return CCompilerConfig(
35-
linker: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kLdBinaryOptions),
36-
compiler: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kClangBinaryOptions),
37-
archiver: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kArBinaryOptions),
44+
Uri? findExecutable({required List<String> possibleExecutableNames, required Directory path}) {
45+
final Uri? found = _findExecutableIfExists(
46+
possibleExecutableNames: possibleExecutableNames,
47+
path: path,
48+
);
49+
50+
if (found == null && throwIfNotFound) {
51+
throwToolExit('Failed to find any of $possibleExecutableNames in $path');
52+
}
53+
54+
return found;
55+
}
56+
57+
final Uri? linker = findExecutable(path: clangDir, possibleExecutableNames: kLdBinaryOptions);
58+
final Uri? compiler = findExecutable(
59+
path: clangDir,
60+
possibleExecutableNames: kClangBinaryOptions,
3861
);
62+
final Uri? archiver = findExecutable(path: clangDir, possibleExecutableNames: kArBinaryOptions);
63+
64+
if (linker == null || compiler == null || archiver == null) {
65+
assert(!throwIfNotFound); // otherwise, findExecutable would have thrown
66+
return null;
67+
}
68+
return CCompilerConfig(linker: linker, compiler: compiler, archiver: archiver);
3969
}
4070

4171
/// Searches for an executable with a name in [possibleExecutableNames]
4272
/// at [path] and returns the first one it finds, if one is found.
43-
/// Otherwise, throws an error.
44-
Uri _findExecutableIfExists({
73+
/// Otherwise, returns `null`.
74+
Uri? _findExecutableIfExists({
4575
required List<String> possibleExecutableNames,
4676
required Directory path,
4777
}) {
4878
return possibleExecutableNames
49-
.map((execName) => path.childFile(execName))
50-
.where((file) => file.existsSync())
51-
.map((file) => file.uri)
52-
.firstOrNull ??
53-
throwToolExit('Failed to find any of $possibleExecutableNames in $path');
79+
.map((execName) => path.childFile(execName))
80+
.where((file) => file.existsSync())
81+
.map((file) => file.uri)
82+
.firstOrNull;
5483
}

packages/flutter_tools/lib/src/isolated/native_assets/macos/native_assets_host.dart

Lines changed: 20 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -168,23 +168,35 @@ Future<void> codesignDylib(
168168
/// Flutter expects `xcrun` to be on the path on macOS hosts.
169169
///
170170
/// Use the `clang`, `ar`, and `ld` that would be used if run with `xcrun`.
171-
Future<CCompilerConfig> cCompilerConfigMacOS() async {
172-
return CCompilerConfig(
173-
compiler: await _findXcrunBinary('clang'),
174-
archiver: await _findXcrunBinary('ar'),
175-
linker: await _findXcrunBinary('ld'),
176-
);
171+
///
172+
/// If no XCode installation was found, [throwIfNotFound] controls whether this
173+
/// throws or returns `null`.
174+
Future<CCompilerConfig?> cCompilerConfigMacOS({required bool throwIfNotFound}) async {
175+
final Uri? compiler = await _findXcrunBinary('clang', throwIfNotFound);
176+
final Uri? archiver = await _findXcrunBinary('ar', throwIfNotFound);
177+
final Uri? linker = await _findXcrunBinary('ld', throwIfNotFound);
178+
179+
if (compiler == null || archiver == null || linker == null) {
180+
assert(!throwIfNotFound);
181+
return null;
182+
}
183+
184+
return CCompilerConfig(compiler: compiler, archiver: archiver, linker: linker);
177185
}
178186

179187
/// Invokes `xcrun --find` to find the full path to [binaryName].
180-
Future<Uri> _findXcrunBinary(String binaryName) async {
188+
Future<Uri?> _findXcrunBinary(String binaryName, bool throwIfNotFound) async {
181189
final ProcessResult xcrunResult = await globals.processManager.run(<String>[
182190
'xcrun',
183191
'--find',
184192
binaryName,
185193
]);
186194
if (xcrunResult.exitCode != 0) {
187-
throwToolExit('Failed to find $binaryName with xcrun:\n${xcrunResult.stderr}');
195+
if (throwIfNotFound) {
196+
throwToolExit('Failed to find $binaryName with xcrun:\n${xcrunResult.stderr}');
197+
} else {
198+
return null;
199+
}
188200
}
189201
return Uri.file((xcrunResult.stdout as String).trim());
190202
}

packages/flutter_tools/lib/src/isolated/native_assets/targets.dart

Lines changed: 26 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -194,7 +194,19 @@ sealed class CodeAssetTarget extends AssetBuildTarget {
194194

195195
late final CCompilerConfig? cCompilerConfigSync;
196196

197-
Future<void> setCCompilerConfig();
197+
/// On platforms where the Flutter app is compiled with a native toolchain, configures this target
198+
/// to contain a [CCompilerConfig] matching that toolchain.
199+
///
200+
/// While hooks are supposed to be able to find a toolchain on their own, we want them to use the
201+
/// same tools used to build the main app to make static linking easier in the future. So if we're
202+
/// e.g. on Linux and use `clang` to compile the app, hooks should use the same `clang` as a
203+
/// compiler too.
204+
///
205+
/// If [mustMatchAppBuild] is true (the default), this should throw if the expected toolchain
206+
/// could not be found. For `flutter test` setups where no app is compiled, we _prefer_ to use the
207+
/// same toolchain but would allow not passing a [CCompilerConfig] if that fails. This allows
208+
/// hooks that only download code assets instead of compiling them to still function.
209+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true});
198210

199211
List<CodeAssetExtension> get codeAssetExtensions {
200212
return <CodeAssetExtension>[
@@ -223,15 +235,18 @@ class WindowsAssetTarget extends CodeAssetTarget {
223235
];
224236

225237
@override
226-
Future<void> setCCompilerConfig() async => cCompilerConfigSync = await cCompilerConfigWindows();
238+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) async =>
239+
// TODO(simolus3): Respect the mustMatchAppBuild option in cCompilerConfigWindows.
240+
cCompilerConfigSync = await cCompilerConfigWindows();
227241
}
228242

229243
final class LinuxAssetTarget extends CodeAssetTarget {
230244
LinuxAssetTarget({required super.supportedAssetTypes, required super.architecture})
231245
: super(os: OS.linux);
232246

233247
@override
234-
Future<void> setCCompilerConfig() async => cCompilerConfigSync = await cCompilerConfigLinux();
248+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) async =>
249+
cCompilerConfigSync = await cCompilerConfigLinux(throwIfNotFound: mustMatchAppBuild);
235250

236251
@override
237252
List<ProtocolExtension> get extensions => <ProtocolExtension>[
@@ -252,7 +267,8 @@ final class IOSAssetTarget extends CodeAssetTarget {
252267
final FileSystem fileSystem;
253268

254269
@override
255-
Future<void> setCCompilerConfig() async => cCompilerConfigSync = await cCompilerConfigMacOS();
270+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) async =>
271+
cCompilerConfigSync = await cCompilerConfigMacOS(throwIfNotFound: mustMatchAppBuild);
256272

257273
IOSCodeConfig _getIOSConfig(Map<String, String> environmentDefines, FileSystem fileSystem) {
258274
final String? sdkRoot = environmentDefines[kSdkRoot];
@@ -299,7 +315,8 @@ final class MacOSAssetTarget extends CodeAssetTarget {
299315
}
300316

301317
@override
302-
Future<void> setCCompilerConfig() async => cCompilerConfigSync = await cCompilerConfigMacOS();
318+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) async =>
319+
cCompilerConfigSync = await cCompilerConfigMacOS(throwIfNotFound: mustMatchAppBuild);
303320
}
304321

305322
final class AndroidAssetTarget extends CodeAssetTarget {
@@ -315,7 +332,8 @@ final class AndroidAssetTarget extends CodeAssetTarget {
315332
final AndroidCodeConfig? _androidCodeConfig;
316333

317334
@override
318-
Future<void> setCCompilerConfig() async => cCompilerConfigSync = await cCompilerConfigAndroid();
335+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) async =>
336+
cCompilerConfigSync = await cCompilerConfigAndroid();
319337

320338
@override
321339
List<ProtocolExtension> get extensions => <ProtocolExtension>[
@@ -362,7 +380,8 @@ final class FlutterTesterAssetTarget extends CodeAssetTarget {
362380
CCompilerConfig? get cCompilerConfigSync => subtarget.cCompilerConfigSync;
363381

364382
@override
365-
Future<void> setCCompilerConfig() async => subtarget.setCCompilerConfig();
383+
Future<void> setCCompilerConfig({bool mustMatchAppBuild = true}) =>
384+
subtarget.setCCompilerConfig(mustMatchAppBuild: false);
366385
}
367386

368387
List<AndroidArch> _androidArchs(TargetPlatform targetPlatform, String? androidArchsEnvironment) {

packages/flutter_tools/test/general.shard/isolated/linux/native_assets_test.dart

Lines changed: 22 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -93,7 +93,7 @@ void main() {
9393
await fileSystem.file('/some/path/to/llvm-ar').create();
9494
await fileSystem.file('/some/path/to/ld.lld').create();
9595

96-
final CCompilerConfig result = await cCompilerConfigLinux();
96+
final CCompilerConfig result = (await cCompilerConfigLinux(throwIfNotFound: true))!;
9797
expect(result.compiler, Uri.file('/some/path/to/clang'));
9898
},
9999
);
@@ -115,15 +115,15 @@ void main() {
115115
await fileSystem.file('/path/to/$execName').create(recursive: true);
116116
}
117117

118-
final CCompilerConfig result = await cCompilerConfigLinux();
118+
final CCompilerConfig result = (await cCompilerConfigLinux(throwIfNotFound: true))!;
119119
expect(result.linker, Uri.file('/path/to/ld'));
120120
expect(result.compiler, Uri.file('/path/to/clang'));
121121
expect(result.archiver, Uri.file('/path/to/ar'));
122122
},
123123
);
124124

125125
testUsingContext(
126-
'cCompilerConfigLinux with missing binaries',
126+
'cCompilerConfigLinux with missing binaries when required',
127127
overrides: <Type, Generator>{
128128
ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
129129
const FakeCommand(command: <Pattern>['which', 'clang++'], stdout: '/a/path/to/clang++'),
@@ -137,7 +137,25 @@ void main() {
137137

138138
await fileSystem.file('/a/path/to/clang++').create(recursive: true);
139139

140-
expect(cCompilerConfigLinux(), throwsA(isA<ToolExit>()));
140+
expect(cCompilerConfigLinux(throwIfNotFound: true), throwsA(isA<ToolExit>()));
141+
},
142+
);
143+
144+
testUsingContext(
145+
'cCompilerConfigLinux with missing binaries when not required',
146+
overrides: <Type, Generator>{
147+
ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
148+
const FakeCommand(command: <Pattern>['which', 'clang++'], stdout: '/a/path/to/clang++'),
149+
]),
150+
FileSystem: () => fileSystem,
151+
},
152+
() async {
153+
if (!const LocalPlatform().isLinux) {
154+
return;
155+
}
156+
157+
await fileSystem.file('/a/path/to/clang++').create(recursive: true);
158+
expect(cCompilerConfigLinux(throwIfNotFound: false), completes);
141159
},
142160
);
143161
}

packages/flutter_tools/test/general.shard/isolated/macos/native_assets_test.dart

Lines changed: 45 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,7 @@ import 'package:code_assets/code_assets.dart';
66
import 'package:file/file.dart';
77
import 'package:file/memory.dart';
88
import 'package:flutter_tools/src/artifacts.dart';
9+
import 'package:flutter_tools/src/base/common.dart';
910
import 'package:flutter_tools/src/base/file_system.dart';
1011
import 'package:flutter_tools/src/base/logger.dart';
1112
import 'package:flutter_tools/src/base/platform.dart';
@@ -391,7 +392,7 @@ void main() {
391392
return;
392393
}
393394

394-
final CCompilerConfig result = await cCompilerConfigMacOS();
395+
final CCompilerConfig result = (await cCompilerConfigMacOS(throwIfNotFound: true))!;
395396
expect(
396397
result.compiler,
397398
Uri.file(
@@ -429,10 +430,52 @@ void main() {
429430
return;
430431
}
431432

432-
final CCompilerConfig result = await cCompilerConfigMacOS();
433+
final CCompilerConfig result = (await cCompilerConfigMacOS(throwIfNotFound: true))!;
433434
expect(result.compiler, Uri.file('/nix/store/random-path-to-clang-wrapper/bin/clang'));
434435
expect(result.archiver, Uri.file('/nix/store/random-path-to-clang-wrapper/bin/ar'));
435436
expect(result.linker, Uri.file('/nix/store/random-path-to-clang-wrapper/bin/ld'));
436437
},
437438
);
439+
440+
testUsingContext(
441+
'missing xcode when required',
442+
overrides: <Type, Generator>{
443+
ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
444+
for (final binary in <String>['clang', 'ar', 'ld'])
445+
FakeCommand(
446+
command: <Pattern>['xcrun', '--find', binary],
447+
exitCode: 1,
448+
stderr: 'not found',
449+
),
450+
]),
451+
},
452+
() async {
453+
if (!const LocalPlatform().isMacOS) {
454+
return;
455+
}
456+
457+
await expectLater(cCompilerConfigMacOS(throwIfNotFound: true), throwsA(isA<ToolExit>()));
458+
},
459+
);
460+
461+
testUsingContext(
462+
'missing xcode when not required',
463+
overrides: <Type, Generator>{
464+
ProcessManager: () => FakeProcessManager.list(<FakeCommand>[
465+
for (final binary in <String>['clang', 'ar', 'ld'])
466+
FakeCommand(
467+
command: <Pattern>['xcrun', '--find', binary],
468+
exitCode: 1,
469+
stderr: 'not found',
470+
),
471+
]),
472+
},
473+
() async {
474+
if (!const LocalPlatform().isMacOS) {
475+
return;
476+
}
477+
478+
expect(await cCompilerConfigMacOS(throwIfNotFound: false), isNull);
479+
},
480+
);
438481
}

0 commit comments

Comments
 (0)