[flutter_tools] Fix native assets crash on Linux custom devices#188384
Conversation
When native assets are enabled, the Flutter tool attempts to resolve the C compiler configuration. On Linux, this is done by reading `CMakeCache.txt` from the native build directory to align toolchains. However, for custom devices or `flutter build bundle` commands, no native app build is performed, meaning `CMakeCache.txt` does not exist. This previously caused the tool to crash unconditionally. This change: 1. Adds a `throwIfNotFound` parameter to `cCompilerConfigLinux` (defaulting to `true`) to allow graceful failures (returning `null`) when the CMake cache or toolchain is missing. 2. Updates `LinuxAssetTarget.setCCompilerConfig` to only require the compiler configuration (passing `throwIfNotFound: true`) if a native app build directory actually exists. Otherwise, it allows it to be optional and return `null`. 3. Fixes and adds unit tests, and introduces a new integration test verifying that `flutter build bundle` succeeds on Linux with native assets. Fixes flutter#187980
github-actions
Bot
added
tool
|
An existing Git SHA, To re-trigger presubmits after closing or re-opeing a PR, or pushing a HEAD commit (i.e. with |
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request introduces a throwIfNotFound parameter to cCompilerConfigLinux to allow graceful handling of missing compiler configurations by returning null instead of throwing a ToolExit. It also updates setCCompilerConfig and adds corresponding unit and integration tests. The review feedback suggests catching FileSystemException alongside ToolExit to prevent unhandled crashes during symbolic link resolution, and simplifying the configuration logic in setCCompilerConfig to avoid potential regressions where missing directories silently bypass error throwing.
| } on ToolExit { | ||
| if (throwIfNotFound) { | ||
| rethrow; | ||
| } | ||
| return null; | ||
| } |
There was a problem hiding this comment.
The resolveSymbolicLinks() call on line 88 can throw a FileSystemException if the compiler path specified in CMakeCache.txt does not exist on the host system (for example, due to a stale cache or an invalid toolchain path). Since only ToolExit is caught here, any FileSystemException will propagate and cause the tool to crash, even when throwIfNotFound is set to false. Catching FileSystemException as well ensures graceful failure and robustness.
} on ToolExit {
if (throwIfNotFound) {
rethrow;
}
return null;
} on FileSystemException {
if (throwIfNotFound) {
rethrow;
}
return null;
}
There was a problem hiding this comment.
Thanks @bkonyi! 🙏 (Sorry for the slow reply on the bug, I was traveling.)
When native assets are enabled, the Flutter tool attempts to resolve the C compiler configuration. On Linux, this is done by reading
CMakeCache.txtfrom the native build directory to align toolchains.However, for custom devices or
flutter build bundlecommands, no native app build is performed, meaningCMakeCache.txtdoes not exist. This previously caused the tool to crash unconditionally.
Right, that's a bug.
- Adds a
throwIfNotFoundparameter tocCompilerConfigLinux(defaulting totrue) to allow graceful failures (returningnull) when the CMake cache or toolchain is missing.
Alternatively, we could avoid passing cmakeDirectory? Do we know the contexts where we wont have a cmake build for sure?
That would be cleaner, but distinguishing the build context at the target level is surprisingly brittle. Both native and bundle-only builds share the same BuildHooks/LinkHooks targets and pass environment.outputDir as the appBuildDirectory, leaving us without a clean way to tell them apart there. Checking cmakeBuildDirectory.existsSync() in targets.dart is much more robust as it directly verifies if a CMake environment is physically available. As far as I can tell, since CMake always runs before flutter assemble in native builds, this is a safe proxy. If it's missing, we safely return null instead of crashing. |
…links Stale or invalid compiler paths in CMakeCache.txt can cause resolveSymbolicLinks() to throw a FileSystemException. This change ensures we catch FileSystemException alongside ToolExit in cCompilerConfigLinux, returning null if throwIfNotFound is false, preventing unhandled crashes. Also adds unit tests using a mock throwing file system.
| cCompilerConfigSync = await cCompilerConfigLinux( | ||
| cmakeDirectory: mustMatchAppBuild ? cmakeBuildDirectory! : null, | ||
| cmakeDirectory: isNativeAppBuild ? cmakeBuildDirectory : null, | ||
| throwIfNotFound: mustMatchAppBuild && isNativeAppBuild, |
There was a problem hiding this comment.
isNativeAppBuild is on both lines, that feels a bit weird. E.g. if the directory is not found we don't pass it. If we don't pass it then we early return null if the directory is null so we will never throw.
throwIfNotFound: mustMatchAppBuild,If isNativeAppBuild is false, cmakeDirectory is passed as null, which already triggers an early return of null inside cCompilerConfigLinux. Therefore, we do not need to check isNativeAppBuild again in throwIfNotFound, and can simplify it to just mustMatchAppBuild. Addresses Daco's review feedback.
flutter-dashboard
Bot
removed
the
autosubmit
3 participants
When native assets are enabled, the Flutter tool attempts to resolve the C compiler configuration. On Linux, this is done by reading
CMakeCache.txtfrom the native build directory to align toolchains.However, for custom devices or
flutter build bundlecommands, no native app build is performed, meaningCMakeCache.txtdoes not exist. This previously caused the tool to crash unconditionally.This PR:
throwIfNotFoundparameter tocCompilerConfigLinux(defaulting totrue) to allow graceful failures (returningnull) when the CMake cache or toolchain is missing.LinuxAssetTarget.setCCompilerConfigto only require the compiler configuration (passingthrowIfNotFound: true) if a native app build directory actually exists. Otherwise, it allows it to be optional and returnnull.flutter build bundlesucceeds on Linux with native assets.Fixes #187980