Unverified Commit 851497ff authored by Christopher Fujino's avatar Christopher Fujino Committed by GitHub

[flutter_tools] fix tests with no native assets running native asset build (#135474)

Fixes https://github.com/flutter/flutter/issues/135461
parent 67d4a831
......@@ -23,7 +23,7 @@ Future<Uri?> dryRunNativeAssetsIOS({
required Uri projectUri,
required FileSystem fileSystem,
}) async {
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
return null;
}
......@@ -72,7 +72,7 @@ Future<List<Uri>> buildNativeAssetsIOS({
required Uri yamlParentDirectory,
required FileSystem fileSystem,
}) async {
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
await writeNativeAssetsYaml(<Asset>[], yamlParentDirectory, fileSystem);
return <Uri>[];
}
......
......@@ -23,7 +23,7 @@ Future<Uri?> dryRunNativeAssetsLinux({
bool flutterTester = false,
required FileSystem fileSystem,
}) async {
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
return null;
}
......@@ -90,7 +90,7 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsLinux({
// CMake requires the folder to exist to do copying.
await buildDir.create(recursive: true);
}
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
final Uri nativeAssetsYaml = await writeNativeAssetsYaml(<Asset>[], yamlParentDirectory ?? buildUri_, fileSystem);
return (nativeAssetsYaml, <Uri>[]);
}
......
......@@ -23,7 +23,7 @@ Future<Uri?> dryRunNativeAssetsMacOS({
bool flutterTester = false,
required FileSystem fileSystem,
}) async {
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
return null;
}
......@@ -77,7 +77,7 @@ Future<(Uri? nativeAssetsYaml, List<Uri> dependencies)> buildNativeAssetsMacOS({
}) async {
const OS targetOs = OS.macOS;
final Uri buildUri_ = nativeAssetsBuildUri(projectUri, targetOs);
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (!await nativeBuildRequired(buildRunner)) {
final Uri nativeAssetsYaml = await writeNativeAssetsYaml(<Asset>[], yamlParentDirectory ?? buildUri_, fileSystem);
return (nativeAssetsYaml, <Uri>[]);
}
......
......@@ -214,7 +214,7 @@ BuildMode nativeAssetsBuildMode(build_info.BuildMode buildMode) {
///
/// Native asset builds cannot be run without a package config. If there is
/// no package config, leave a logging trace about that.
Future<bool> hasNoPackageConfig(NativeAssetsBuildRunner buildRunner) async {
Future<bool> _hasNoPackageConfig(NativeAssetsBuildRunner buildRunner) async {
final bool packageConfigExists = await buildRunner.hasPackageConfig();
if (!packageConfigExists) {
globals.logger.printTrace('No package config found. Skipping native assets compilation.');
......@@ -222,24 +222,23 @@ Future<bool> hasNoPackageConfig(NativeAssetsBuildRunner buildRunner) async {
return !packageConfigExists;
}
/// Checks that if native assets is disabled, none of the dependencies declare
/// native assets.
///
/// If any of the dependencies have native assets, but native assets are
/// disabled, exits the tool.
Future<bool> isDisabledAndNoNativeAssets(NativeAssetsBuildRunner buildRunner) async {
if (featureFlags.isNativeAssetsEnabled) {
Future<bool> nativeBuildRequired(NativeAssetsBuildRunner buildRunner) async {
if (await _hasNoPackageConfig(buildRunner)) {
return false;
}
final List<Package> packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets();
if (packagesWithNativeAssets.isEmpty) {
return true;
return false;
}
final String packageNames = packagesWithNativeAssets.map((Package p) => p.name).join(' ');
throwToolExit(
'Package(s) $packageNames require the native assets feature to be enabled. '
'Enable using `flutter config --enable-native-assets`.',
);
if (!featureFlags.isNativeAssetsEnabled) {
final String packageNames = packagesWithNativeAssets.map((Package p) => p.name).join(' ');
throwToolExit(
'Package(s) $packageNames require the native assets feature to be enabled. '
'Enable using `flutter config --enable-native-assets`.',
);
}
return true;
}
/// Ensures that either this project has no native assets, or that native assets
......@@ -252,7 +251,7 @@ Future<void> ensureNoNativeAssetsOrOsIsSupported(
FileSystem fileSystem,
NativeAssetsBuildRunner buildRunner,
) async {
if (await hasNoPackageConfig(buildRunner)) {
if (await _hasNoPackageConfig(buildRunner)) {
return;
}
final List<Package> packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets();
......@@ -345,12 +344,7 @@ Future<Uri?> dryRunNativeAssets({
buildRunner: buildRunner,
);
} else {
await ensureNoNativeAssetsOrOsIsSupported(
projectUri,
const LocalPlatform().operatingSystem,
fileSystem,
buildRunner,
);
await nativeBuildRequired(buildRunner);
nativeAssetsYaml = null;
}
case build_info.TargetPlatform.linux_arm64:
......@@ -389,7 +383,7 @@ Future<Uri?> dryRunNativeAssetsMultipeOSes({
required FileSystem fileSystem,
required Iterable<build_info.TargetPlatform> targetPlatforms,
}) async {
if (await hasNoPackageConfig(buildRunner) || await isDisabledAndNoNativeAssets(buildRunner)) {
if (await nativeBuildRequired(buildRunner)) {
return null;
}
......
......@@ -14,6 +14,7 @@ import 'package:flutter_tools/src/build_system/targets/native_assets.dart';
import 'package:flutter_tools/src/features.dart';
import 'package:flutter_tools/src/native_assets.dart';
import 'package:native_assets_cli/native_assets_cli.dart' as native_assets_cli;
import 'package:package_config/package_config.dart' show Package;
import '../../../src/common.dart';
import '../../../src/context.dart';
......@@ -104,6 +105,7 @@ void main() {
await createPackageConfig(iosEnvironment);
final NativeAssetsBuildRunner buildRunner = FakeNativeAssetsBuildRunner(
packagesWithNativeAssetsResult: <Package>[Package('foo', iosEnvironment.buildDir.uri)],
buildResult: FakeNativeAssetsBuilderResult(assets: <native_assets_cli.Asset>[
native_assets_cli.Asset(
id: 'package:foo/foo.dart',
......
......@@ -6,6 +6,7 @@ import 'package:file/file.dart';
import 'package:file/memory.dart';
import 'package:file_testing/file_testing.dart';
import 'package:flutter_tools/src/artifacts.dart';
import 'package:flutter_tools/src/base/common.dart';
import 'package:flutter_tools/src/base/file_system.dart';
import 'package:flutter_tools/src/base/logger.dart';
import 'package:flutter_tools/src/base/platform.dart';
......@@ -86,6 +87,24 @@ void main() {
);
});
testUsingContext('does not throw if clang not present but no native assets present', overrides: <Type, Generator>{
FeatureFlags: () => TestFeatureFlags(isNativeAssetsEnabled: true),
ProcessManager: () => FakeProcessManager.empty(),
}, () async {
final File packageConfig = environment.projectDir.childFile('.dart_tool/package_config.json');
await packageConfig.create(recursive: true);
await buildNativeAssetsLinux(
projectUri: projectUri,
buildMode: BuildMode.debug,
fileSystem: fileSystem,
buildRunner: _BuildRunnerWithoutClang(),
);
expect(
(globals.logger as BufferLogger).traceText,
isNot(contains('Building native assets for ')),
);
});
testUsingContext('dry run for multiple OSes with no package config', overrides: <Type, Generator>{
ProcessManager: () => FakeProcessManager.empty(),
}, () async {
......@@ -372,3 +391,8 @@ void main() {
expect(result.cc, Uri.file('/some/path/to/clang'));
});
}
class _BuildRunnerWithoutClang extends FakeNativeAssetsBuildRunner {
@override
Future<CCompilerConfig> get cCompilerConfig async => throwToolExit('Failed to find clang++ on the PATH.');
}
......@@ -2420,7 +2420,7 @@ flutter:
expect(buildRunner.buildInvocations, 0);
expect(buildRunner.dryRunInvocations, 1);
expect(buildRunner.hasPackageConfigInvocations, 1);
expect(buildRunner.packagesWithNativeAssetsInvocations, 0);
expect(buildRunner.packagesWithNativeAssetsInvocations, 1);
}),
overrides: <Type, Generator>{
ProcessManager: () => FakeProcessManager.any(),
......
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