firka/flutter
3
0
Fork 1
Code Issues Pull Requests Actions Packages Projects Releases Wiki Activity

[native assets] Cleanup dead code 2 (#161916)

Browse Source 
This PR deletes dead code.

The `FlutterNativeAssetsBuildRunnerImpl` has a `PackageConfig` argument,
so the package config file must exist. This means the `hasPackageConfig`
method is meaningless, it will always return `true`.

The only case where it might have returned false was in the unit test
mock. This unit test is now deleted.

(It must be the case that `flutter_tools` internally ensures `pub get`
has been run in the project before it reaches any code paths that try to
build native assets.)

## Pre-launch Checklist

- [x] I read the [Contributor Guide] and followed the process outlined
there for submitting PRs.
- [x] I read the [Tree Hygiene] wiki page, which explains my
responsibilities.
- [x] I read and followed the [Flutter Style Guide], including [Features
we expect every widget to implement].
- [x] I signed the [CLA].
- [ ] I listed at least one issue that this PR fixes in the description
above.
- [ ] I updated/added relevant documentation (doc comments with `///`).
- [ ] I added new tests to check the change I am making, or this PR is
[test-exempt].
- [ ] I followed the [breaking change policy] and added [Data Driven
Fixes] where supported.
- [x] All existing and new tests are passing.

If you need help, consider asking for advice on the #hackers-new channel
on [Discord].

<!-- Links -->
[Contributor Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#overview
[Tree Hygiene]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md
[test-exempt]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#tests
[Flutter Style Guide]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md
[Features we expect every widget to implement]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Style-guide-for-Flutter-repo.md#features-we-expect-every-widget-to-implement
[CLA]: https://cla.developers.google.com/
[flutter/tests]: https://github.com/flutter/tests
[breaking change policy]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Tree-hygiene.md#handling-breaking-changes
[Discord]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Chat.md
[Data Driven Fixes]:
https://github.com/flutter/flutter/blob/main/docs/contributing/Data-driven-Fixes.md
This commit is contained in:
Daco Harkes
2025-01-21 10:03:28 +01:00
committed by GitHub
parent d0e03173bd
commit da080e6976
3 changed files with 0 additions and 67 deletions
Download Patch File Download Diff File Expand all files Collapse all files

37
packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart
View File

@@ -150,12 +150,6 @@ Future<void> installCodeAssets({
/// It enables mocking `package:native_assets_builder` package.
/// It also enables mocking native toolchain discovery via [cCompilerConfig].
abstract interface class FlutterNativeAssetsBuildRunner {
/// Whether the project has a `.dart_tools/package_config.json`.
///
/// If there is no package config, [packagesWithNativeAssets], [build] and
/// [link] must not be invoked.
Future<bool> hasPackageConfig();
/// All packages in the transitive dependencies that have a `build.dart`.
Future<List<Package>> packagesWithNativeAssets();
@@ -166,7 +160,6 @@ abstract interface class FlutterNativeAssetsBuildRunner {
required BuildConfigCreator configCreator,
required BuildValidator buildValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required bool linkingEnabled,
});
@@ -178,7 +171,6 @@ abstract interface class FlutterNativeAssetsBuildRunner {
required LinkConfigCreator configCreator,
required LinkValidator linkValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required BuildResult buildResult,
});
@@ -235,11 +227,6 @@ class FlutterNativeAssetsBuildRunnerImpl implements FlutterNativeAssetsBuildRunn
fileSystem: fileSystem,
);
@override
Future<bool> hasPackageConfig() {
return fileSystem.file(packageConfigPath).exists();
}
@override
Future<List<Package>> packagesWithNativeAssets() async {
final PackageLayout packageLayout = PackageLayout.fromPackageConfig(
@@ -260,7 +247,6 @@ class FlutterNativeAssetsBuildRunnerImpl implements FlutterNativeAssetsBuildRunn
required BuildConfigCreator configCreator,
required BuildValidator buildValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required bool linkingEnabled,
}) {
@@ -288,7 +274,6 @@ class FlutterNativeAssetsBuildRunnerImpl implements FlutterNativeAssetsBuildRunn
required LinkConfigCreator configCreator,
required LinkValidator linkValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required BuildResult buildResult,
}) {
@@ -387,24 +372,7 @@ bool _nativeAssetsLinkingEnabled(BuildMode buildMode) {
}
}
/// Checks whether this project does not yet have a package config file.
///
/// A project has no package config when `pub get` has not yet been run.
///
/// 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(FlutterNativeAssetsBuildRunner buildRunner) async {
final bool packageConfigExists = await buildRunner.hasPackageConfig();
if (!packageConfigExists) {
globals.logger.printTrace('No package config found. Skipping native assets compilation.');
}
return !packageConfigExists;
}
Future<bool> _nativeBuildRequired(FlutterNativeAssetsBuildRunner buildRunner) async {
if (await _hasNoPackageConfig(buildRunner)) {
return false;
}
final List<Package> packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets();
if (packagesWithNativeAssets.isEmpty) {
globals.logger.printTrace(
@@ -433,9 +401,6 @@ Future<void> ensureNoNativeAssetsOrOsIsSupported(
FileSystem fileSystem,
FlutterNativeAssetsBuildRunner buildRunner,
) async {
if (await _hasNoPackageConfig(buildRunner)) {
return;
}
final List<Package> packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets();
if (packagesWithNativeAssets.isEmpty) {
globals.logger.printTrace(
@@ -669,7 +634,6 @@ Future<DartBuildResult> _runDartBuild({
...await validateCodeAssetInApplication(assets),
],
workingDirectory: projectUri,
includeParentEnvironment: true,
linkingEnabled: linkingEnabled,
);
if (buildResult == null) {
@@ -703,7 +667,6 @@ Future<DartBuildResult> _runDartBuild({
...await validateCodeAssetInApplication(assets),
],
workingDirectory: projectUri,
includeParentEnvironment: true,
buildResult: buildResult,
);
if (linkResult == null) {

11
packages/flutter_tools/test/general.shard/isolated/fake_native_assets_build_runner.dart
View File

@@ -13,7 +13,6 @@ export 'package:native_assets_cli/code_assets_builder.dart' show CodeAsset, Dyna
/// relies on doing process calls to `pub` and the local file system.
class FakeFlutterNativeAssetsBuildRunner implements FlutterNativeAssetsBuildRunner {
FakeFlutterNativeAssetsBuildRunner({
this.hasPackageConfigResult = true,
this.packagesWithNativeAssetsResult = const <Package>[],
this.onBuild,
this.onLink,
@@ -27,14 +26,12 @@ class FakeFlutterNativeAssetsBuildRunner implements FlutterNativeAssetsBuildRunn
final LinkResult? Function(LinkConfig)? onLink;
final BuildResult? buildResult;
final LinkResult? linkResult;
final bool hasPackageConfigResult;
final List<Package> packagesWithNativeAssetsResult;
final CCompilerConfig? cCompilerConfigResult;
final CCompilerConfig? ndkCCompilerConfigResult;
int buildInvocations = 0;
int linkInvocations = 0;
int hasPackageConfigInvocations = 0;
int packagesWithNativeAssetsInvocations = 0;
@override
@@ -44,7 +41,6 @@ class FakeFlutterNativeAssetsBuildRunner implements FlutterNativeAssetsBuildRunn
required BuildConfigCreator configCreator,
required BuildValidator buildValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required bool linkingEnabled,
}) async {
@@ -78,7 +74,6 @@ class FakeFlutterNativeAssetsBuildRunner implements FlutterNativeAssetsBuildRunn
required LinkConfigValidator configValidator,
required LinkValidator linkValidator,
required ApplicationAssetValidator applicationAssetValidator,
required bool includeParentEnvironment,
required Uri workingDirectory,
required BuildResult buildResult,
}) async {
@@ -105,12 +100,6 @@ class FakeFlutterNativeAssetsBuildRunner implements FlutterNativeAssetsBuildRunn
return result;
}
@override
Future<bool> hasPackageConfig() async {
hasPackageConfigInvocations++;
return hasPackageConfigResult;
}
@override
Future<List<Package>> packagesWithNativeAssets() async {
packagesWithNativeAssetsInvocations++;

19
packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart
View File

@@ -12,7 +12,6 @@ import 'package:flutter_tools/src/build_info.dart';
import 'package:flutter_tools/src/build_system/build_system.dart';
import 'package:flutter_tools/src/build_system/targets/native_assets.dart';
import 'package:flutter_tools/src/features.dart';
import 'package:flutter_tools/src/globals.dart' as globals;
import 'package:flutter_tools/src/isolated/native_assets/native_assets.dart';
import 'package:native_assets_cli/code_assets_builder.dart';
import 'package:package_config/package_config_types.dart';
@@ -47,24 +46,6 @@ void main() {
projectUri = environment.projectDir.uri;
});
testUsingContext(
'build with no package config',
overrides: <Type, Generator>{ProcessManager: () => FakeProcessManager.empty()},
() async {
await runFlutterSpecificDartBuild(
environmentDefines: <String, String>{kBuildMode: BuildMode.debug.cliName},
targetPlatform: TargetPlatform.windows_x64,
projectUri: projectUri,
fileSystem: fileSystem,
buildRunner: FakeFlutterNativeAssetsBuildRunner(hasPackageConfigResult: false),
);
expect(
(globals.logger as BufferLogger).traceText,
contains('No package config found. Skipping native assets compilation.'),
);
},
);
testUsingContext(
'Native assets: non-bundled libraries require no copying',
overrides: <Type, Generator>{