[CP-beta]Build hooks: Don't require toolchain for unit tests #179211
Closed
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
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).
Author
|
@dcharkes please fill out the PR description above, afterwards the release team will review this request. |
|
This pull request was opened from and to a release candidate branch. This should only be done as part of the official Flutter release process. If you are attempting to make a regular contribution to the Flutter project, please close this PR and follow the instructions at Tree Hygiene for detailed instructions on contributing to Flutter. Reviewers: Use caution before merging pull requests to release branches. Ensure the proper procedure has been followed. |
github-actions
bot
added
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Code Review
This pull request modifies the build hooks to not require a C++ toolchain for unit tests. This is achieved by making the toolchain discovery logic conditionally throw an error or return null, and then updating the FlutterTesterAssetTarget to not require the toolchain. The changes are logical and include corresponding test updates for Linux and macOS. I have one suggestion to refactor the Linux implementation for better consistency and readability.
Comment on lines
43
to
83
| final Directory clangDir = clangPpFile.parent; | ||
| return CCompilerConfig( | ||
| linker: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kLdBinaryOptions), | ||
| compiler: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kClangBinaryOptions), | ||
| archiver: _findExecutableIfExists(path: clangDir, possibleExecutableNames: kArBinaryOptions), | ||
| Uri? findExecutable({required List<String> possibleExecutableNames, required Directory path}) { | ||
| final Uri? found = _findExecutableIfExists( | ||
| possibleExecutableNames: possibleExecutableNames, | ||
| path: path, | ||
| ); | ||
|
|
||
| if (found == null && throwIfNotFound) { | ||
| throwToolExit('Failed to find any of $possibleExecutableNames in $path'); | ||
| } | ||
|
|
||
| return found; | ||
| } | ||
|
|
||
| final Uri? linker = findExecutable(path: clangDir, possibleExecutableNames: kLdBinaryOptions); | ||
| final Uri? compiler = findExecutable( | ||
| path: clangDir, | ||
| possibleExecutableNames: kClangBinaryOptions, | ||
| ); | ||
| final Uri? archiver = findExecutable(path: clangDir, possibleExecutableNames: kArBinaryOptions); | ||
|
|
||
| if (linker == null || compiler == null || archiver == null) { | ||
| assert(!throwIfNotFound); // otherwise, findExecutable would have thrown | ||
| return null; | ||
| } | ||
| return CCompilerConfig(linker: linker, compiler: compiler, archiver: archiver); | ||
| } | ||
|
|
||
| /// Searches for an executable with a name in [possibleExecutableNames] | ||
| /// at [path] and returns the first one it finds, if one is found. | ||
| /// Otherwise, throws an error. | ||
| Uri _findExecutableIfExists({ | ||
| /// Otherwise, returns `null`. | ||
| Uri? _findExecutableIfExists({ | ||
| required List<String> possibleExecutableNames, | ||
| required Directory path, | ||
| }) { | ||
| return possibleExecutableNames | ||
| .map((execName) => path.childFile(execName)) | ||
| .where((file) => file.existsSync()) | ||
| .map((file) => file.uri) | ||
| .firstOrNull ?? | ||
| throwToolExit('Failed to find any of $possibleExecutableNames in $path'); | ||
| .map((execName) => path.childFile(execName)) | ||
| .where((file) => file.existsSync()) | ||
| .map((file) => file.uri) | ||
| .firstOrNull; | ||
| } |
Contributor
There was a problem hiding this comment.
For consistency with cCompilerConfigMacOS and to simplify the logic, you could move the conditional throwing logic into _findExecutableIfExists. This removes the need for the local findExecutable function and makes cCompilerConfigLinux easier to read.
final Directory clangDir = clangPpFile.parent;
final Uri? linker = _findExecutableIfExists(
path: clangDir,
possibleExecutableNames: kLdBinaryOptions,
throwIfNotFound: throwIfNotFound,
);
final Uri? compiler = _findExecutableIfExists(
path: clangDir,
possibleExecutableNames: kClangBinaryOptions,
throwIfNotFound: throwIfNotFound,
);
final Uri? archiver = _findExecutableIfExists(
path: clangDir,
possibleExecutableNames: kArBinaryOptions,
throwIfNotFound: throwIfNotFound,
);
if (linker == null || compiler == null || archiver == null) {
assert(!throwIfNotFound); // otherwise, _findExecutableIfExists would have thrown
return null;
}
return CCompilerConfig(linker: linker, compiler: compiler, archiver: archiver);
}
/// Searches for an executable with a name in [possibleExecutableNames]
/// at [path] and returns the first one it finds, if one is found.
///
/// If not found, throws if [throwIfNotFound] is true, otherwise returns `null`.
Uri? _findExecutableIfExists({
required List<String> possibleExecutableNames,
required Directory path,
required bool throwIfNotFound,
}) {
final Uri? result = possibleExecutableNames
.map((execName) => path.childFile(execName))
.where((file) => file.existsSync())
.map((file) => file.uri)
.firstOrNull;
if (result == null && throwIfNotFound) {
throwToolExit('Failed to find any of $possibleExecutableNames in $path');
}
return result;
}
Contributor
There was a problem hiding this comment.
This is a cherry pick. We're not going to change the code for a cherry pick! Context Gemini, context!
Contributor
|
For stable the automatic cherry pick process didn't work. I'll make a cherry pick manually. @simolus3 Any remarks about my description above of this PR? |
This was referenced Nov 28, 2025
Contributor
|
No remarks from my side, the description looks good to me 👍 |
auto-submit bot
pushed a commit
that referenced
this pull request
Dec 2, 2025
) ### Issue Link: #178954 (beta CP: #179211) ### Impact Description: Running `flutter test` on Linux or MacOS which does not have a `clang` starts failing on projects that target only Android otherwise (and do not require a Linux or MacOS toolchain) when there are build hooks anywhere in the dependencies. This PR changes the behavior for `flutter test` to only pass `clang` and friends if they are installed. This means that _if_ a Linux/MacOS toolchain is installed on the system, it will be used for both `flutter run -d Linux`/`MacOS` and `flutter test` (so that the same native compiler is used), but if it's not installed on the system both `flutter run -d AndroidDevice` and `flutter test` run successfully. This means end users don't have to go modify their systems/CIs to install Linux/MacOS toolchains for Android projects. ### Changelog Description: [flutter/178954] Fixes running `flutter test` on Linux/MacOS for Android projects with build hooks without the desktop native tooling installed. ### Workaround: All end users have to install `clang` and `ldd` on Linux/MacOS hosts and CI bots. ### Risk: What is the risk level of this cherry-pick? It changes the flow in Flutter tools from throwing an error to returning a nullable value. ### Test Coverage: Are you confident that your fix is well-tested by automated tests? ### Validation Steps: On a Linux and MacOS system, make a Flutter app that only targets Android. Add a build hook. And ensure there's no `clang` on the `PATH` (Can be verified by `flutter doctor -v` finding no tooling for Flutter desktop). Run `flutter test` and it shouldn't fail due to `clang` not being installed. For MacOS, XCode needs to not be installed.
Contributor
|
We cut a new beta (3.40.0-0.1.pre) from master, so we don't need to cherry-pick this anymore. |
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
4 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Issue Link:
#178954
(stable CP: #179214)
Impact Description:
Running
flutter teston Linux or MacOS which does not have aclangstarts failing on projects that target only Android otherwise (and do not require a Linux or MacOS toolchain) when there are build hooks anywhere in the dependencies.This PR changes the behavior for
flutter testto only passclangand friends if they are installed. This means that if a Linux/MacOS toolchain is installed on the system, it will be used for bothflutter run -d Linux/MacOSandflutter test(so that the same native compiler is used), but if it's not installed on the system bothflutter run -d AndroidDeviceandflutter testrun successfully.This means end users don't have to go modify their systems/CIs to install Linux/MacOS toolchains for Android projects.
Changelog Description:
[flutter/178954] Fixes running
flutter teston Linux/MacOS for Android projects with build hooks without the desktop native tooling installed.Workaround:
All end users have to install
clangandlddon Linux/MacOS hosts and CI bots.Risk:
What is the risk level of this cherry-pick?
It changes the flow in Flutter tools from throwing an error to returning a nullable value.
Test Coverage:
Are you confident that your fix is well-tested by automated tests?
Validation Steps:
On a Linux and MacOS system, make a Flutter app that only targets Android. Add a build hook. And ensure there's no
clangon thePATH(Can be verified byflutter doctor -vfinding no tooling for Flutter desktop). Runflutter testand it shouldn't fail due toclangnot being installed. For MacOS, XCode needs to not be installed.