From da080e6976acd5c9f281f227f7df634ccfa84fae Mon Sep 17 00:00:00 2001 From: Daco Harkes Date: Tue, 21 Jan 2025 10:03:28 +0100 Subject: [PATCH] [native assets] Cleanup dead code 2 (#161916) 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]. [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 --- .../isolated/native_assets/native_assets.dart | 37 ------------------- .../fake_native_assets_build_runner.dart | 11 ------ .../isolated/native_assets_test.dart | 19 ---------- 3 files changed, 67 deletions(-) diff --git a/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart b/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart index 275ac73209..53a29c4937 100644 --- a/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart +++ b/packages/flutter_tools/lib/src/isolated/native_assets/native_assets.dart @@ -150,12 +150,6 @@ Future 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 hasPackageConfig(); - /// All packages in the transitive dependencies that have a `build.dart`. Future> 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 hasPackageConfig() { - return fileSystem.file(packageConfigPath).exists(); - } - @override Future> 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 _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 _nativeBuildRequired(FlutterNativeAssetsBuildRunner buildRunner) async { - if (await _hasNoPackageConfig(buildRunner)) { - return false; - } final List packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets(); if (packagesWithNativeAssets.isEmpty) { globals.logger.printTrace( @@ -433,9 +401,6 @@ Future ensureNoNativeAssetsOrOsIsSupported( FileSystem fileSystem, FlutterNativeAssetsBuildRunner buildRunner, ) async { - if (await _hasNoPackageConfig(buildRunner)) { - return; - } final List packagesWithNativeAssets = await buildRunner.packagesWithNativeAssets(); if (packagesWithNativeAssets.isEmpty) { globals.logger.printTrace( @@ -669,7 +634,6 @@ Future _runDartBuild({ ...await validateCodeAssetInApplication(assets), ], workingDirectory: projectUri, - includeParentEnvironment: true, linkingEnabled: linkingEnabled, ); if (buildResult == null) { @@ -703,7 +667,6 @@ Future _runDartBuild({ ...await validateCodeAssetInApplication(assets), ], workingDirectory: projectUri, - includeParentEnvironment: true, buildResult: buildResult, ); if (linkResult == null) { diff --git a/packages/flutter_tools/test/general.shard/isolated/fake_native_assets_build_runner.dart b/packages/flutter_tools/test/general.shard/isolated/fake_native_assets_build_runner.dart index bf95e0558d..a4dbd2e422 100644 --- a/packages/flutter_tools/test/general.shard/isolated/fake_native_assets_build_runner.dart +++ b/packages/flutter_tools/test/general.shard/isolated/fake_native_assets_build_runner.dart @@ -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 [], 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 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 hasPackageConfig() async { - hasPackageConfigInvocations++; - return hasPackageConfigResult; - } - @override Future> packagesWithNativeAssets() async { packagesWithNativeAssetsInvocations++; diff --git a/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart b/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart index 1760cc440e..0a19cf75a4 100644 --- a/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart +++ b/packages/flutter_tools/test/general.shard/isolated/native_assets_test.dart @@ -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: {ProcessManager: () => FakeProcessManager.empty()}, - () async { - await runFlutterSpecificDartBuild( - environmentDefines: {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: {