From e6730613c97cc3628ecda39c20c60ecb5938366f Mon Sep 17 00:00:00 2001 From: "auto-submit[bot]" <98614782+auto-submit[bot]@users.noreply.github.com> Date: Thu, 20 Feb 2025 19:57:25 +0000 Subject: [PATCH] Reverts "Avoid implicitly setting `determineDevDependencies: true` (it's not a safe operation) (#163711)" (#163762) Reverts: flutter/flutter#163711 Initiated by: victorsanni Reason for reverting: This is closing the tree. See https://ci.chromium.org/ui/p/flutter/builders/prod/Linux%20android_release_builds_exclude_dev_dependencies_test/889/overview Original PR Author: matanlurey Reviewed By: {jonahwilliams} This change reverts the following previous change: Closes https://github.com/flutter/flutter/issues/163706. Before this PR, the macOS workflow (or, others, but only macOS had a test) would fail because it calls `refreshPluginsList` manually, and sometimes it would be `determineDevDependencies: null` and sometimes `determineDevDependencies: false`. A value of `determineDevDependencies: null` was interpreted later on as "find dev dependencies", which is not a safe operation. The only real change in this PR is `bool determineDevDependencies = false`, so omitting that parameter means we don't determine dev dependencies. Added some tests, and opted-in an integration test that was failing. Co-authored-by: auto-submit[bot] --- .../lib/src/flutter_plugins.dart | 10 +- packages/flutter_tools/lib/src/project.dart | 7 +- .../test/general.shard/project_test.dart | 93 ------------------- .../macos_content_validation_test.dart | 15 --- 4 files changed, 4 insertions(+), 121 deletions(-) diff --git a/packages/flutter_tools/lib/src/flutter_plugins.dart b/packages/flutter_tools/lib/src/flutter_plugins.dart index 5ae90d9d17f..3d11a0cf4b9 100644 --- a/packages/flutter_tools/lib/src/flutter_plugins.dart +++ b/packages/flutter_tools/lib/src/flutter_plugins.dart @@ -244,8 +244,9 @@ bool _writeFlutterPluginsList( final String? oldPluginsFileStringContent = _readFileContent(pluginsFile); bool pluginsChanged = true; if (oldPluginsFileStringContent != null) { + Object? decodedJson; try { - final Object? decodedJson = jsonDecode(oldPluginsFileStringContent); + decodedJson = jsonDecode(oldPluginsFileStringContent); if (decodedJson is Map) { final String jsonOfNewPluginsMap = jsonEncode(pluginsMap); final String jsonOfOldPluginsMap = jsonEncode(decodedJson[_kFlutterPluginsPluginListKey]); @@ -1099,17 +1100,12 @@ void _createPlatformPluginSymlinks( /// dependencies declared in `pubspec.yaml`. /// /// Assumes `pub get` has been executed since last change to `pubspec.yaml`. -/// -/// Unless explicitly specified, [determineDevDependencies] is disabled by -/// default; if set to `true`, plugins that are development-only dependencies -/// may be labeled or, depending on the platform, omitted from metadata or -/// platform-specific artifacts. Future refreshPluginsList( FlutterProject project, { bool iosPlatform = false, bool macOSPlatform = false, bool forceCocoaPodsOnly = false, - bool determineDevDependencies = false, + bool? determineDevDependencies, bool? generateLegacyPlugins, }) async { final List plugins = await findPlugins( diff --git a/packages/flutter_tools/lib/src/project.dart b/packages/flutter_tools/lib/src/project.dart index 1f95962dcfb..cfa7b18e63d 100644 --- a/packages/flutter_tools/lib/src/project.dart +++ b/packages/flutter_tools/lib/src/project.dart @@ -358,12 +358,7 @@ class FlutterProject { if (!directory.existsSync() || isPlugin) { return; } - await refreshPluginsList( - this, - iosPlatform: iosPlatform, - macOSPlatform: macOSPlatform, - determineDevDependencies: releaseMode ?? false, - ); + await refreshPluginsList(this, iosPlatform: iosPlatform, macOSPlatform: macOSPlatform); if (androidPlatform) { await android.ensureReadyForPlatformSpecificTooling(deprecationBehavior: deprecationBehavior); } diff --git a/packages/flutter_tools/test/general.shard/project_test.dart b/packages/flutter_tools/test/general.shard/project_test.dart index 6dc8c6c68d2..f1ceca8d9d6 100644 --- a/packages/flutter_tools/test/general.shard/project_test.dart +++ b/packages/flutter_tools/test/general.shard/project_test.dart @@ -31,12 +31,6 @@ import '../src/fake_pub_deps.dart'; import '../src/fakes.dart'; void main() { - // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. - // See https://github.com/flutter/flutter/issues/160257 for details. - FeatureFlags enableExplicitPackageDependencies() { - return TestFeatureFlags(isExplicitPackageDependenciesEnabled: true); - } - // TODO(zanderso): remove once FlutterProject is fully refactored. // this is safe since no tests have expectations on the test logger. final BufferLogger logger = BufferLogger.test(); @@ -278,59 +272,6 @@ void main() { await project.regeneratePlatformSpecificTooling(); expectExists(project.android.hostAppGradleRoot.childFile('local.properties')); }); - - testUsingContext( - 'omitted release mode does not determine dev dependencies', - () async { - // Create a plugin. - await aPluginProject(); - // Create a project that depends on that plugin. - final FlutterProject project = await projectWithPluginDependency(); - // Don't bother with Android, we just want the manifest. - project.directory.childDirectory('android').deleteSync(recursive: true); - - await project.regeneratePlatformSpecificTooling(); - expect( - project.flutterPluginsDependenciesFile.readAsStringSync(), - isNot(contains('"dev_dependency":true')), - ); - }, - overrides: { - FeatureFlags: enableExplicitPackageDependencies, - FileSystem: () => MemoryFileSystem.test(), - ProcessManager: () => FakeProcessManager.any(), - Pub: () => FakePubWithPrimedDeps(devDependencies: {'my_plugin'}), - FlutterProjectFactory: - () => FlutterProjectFactory(logger: logger, fileSystem: globals.fs), - }, - ); - - testUsingContext( - 'specified release mode determines dev dependencies', - () async { - // Create a plugin. - await aPluginProject(); - // Create a project that depends on that plugin. - final FlutterProject project = await projectWithPluginDependency(); - // Don't bother with Android, we just want the manifest. - project.directory.childDirectory('android').deleteSync(recursive: true); - - await project.regeneratePlatformSpecificTooling(releaseMode: true); - expect( - project.flutterPluginsDependenciesFile.readAsStringSync(), - contains('"dev_dependency":true'), - ); - }, - overrides: { - FeatureFlags: enableExplicitPackageDependencies, - FileSystem: () => MemoryFileSystem.test(), - ProcessManager: () => FakeProcessManager.any(), - Pub: () => FakePubWithPrimedDeps(devDependencies: {'my_plugin'}), - FlutterProjectFactory: - () => FlutterProjectFactory(logger: logger, fileSystem: globals.fs), - }, - ); - testUsingContext( 'injects plugins for macOS', () async { @@ -1789,40 +1730,6 @@ Future someProject({ return FlutterProject.fromDirectory(directory); } -Future projectWithPluginDependency() async { - final Directory directory = globals.fs.directory('some_project'); - directory.childDirectory('.dart_tool').childFile('package_config.json') - ..createSync(recursive: true) - ..writeAsStringSync(''' -{ - "configVersion": 2, - "packages": [ - { - "name": "my_plugin", - "rootUri": "/plugin_project", - "packageUri": "lib/", - "languageVersion": "2.12" - } - ] -} -'''); - directory.childFile('pubspec.yaml') - ..createSync(recursive: true) - ..writeAsStringSync(''' -name: app_name -flutter: - -dependencies: - my_plugin: - sdk: flutter -'''); - directory.childDirectory('ios').createSync(recursive: true); - final Directory androidDirectory = directory.childDirectory('android') - ..createSync(recursive: true); - androidDirectory.childFile('AndroidManifest.xml').writeAsStringSync(''); - return FlutterProject.fromDirectory(directory); -} - Future aPluginProject({bool legacy = true}) async { final Directory directory = globals.fs.directory('plugin_project'); directory.childDirectory('ios').createSync(recursive: true); diff --git a/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart b/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart index da967be5c19..7f7dd82f058 100644 --- a/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart +++ b/packages/flutter_tools/test/host_cross_arch.shard/macos_content_validation_test.dart @@ -5,7 +5,6 @@ import 'package:file_testing/file_testing.dart'; import 'package:flutter_tools/src/base/file_system.dart'; import 'package:flutter_tools/src/base/io.dart'; -import 'package:flutter_tools/src/features.dart'; import '../integration.shard/test_utils.dart'; import '../src/common.dart'; @@ -15,20 +14,6 @@ void main() { setUpAll(() { processManager.runSync([flutterBin, 'config', '--enable-macos-desktop']); - - // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. - // See https://github.com/flutter/flutter/issues/160257 for details. - if (!explicitPackageDependencies.master.enabledByDefault) { - processManager.runSync([flutterBin, 'config', '--explicit-package-dependencies']); - } - }); - - tearDownAll(() { - // TODO(matanlurey): Remove after `explicit-package-dependencies` is enabled by default. - // See https://github.com/flutter/flutter/issues/160257 for details. - if (!explicitPackageDependencies.master.enabledByDefault) { - processManager.runSync([flutterBin, 'config', '--no-explicit-package-dependencies']); - } }); for (final String buildMode in ['Debug', 'Release']) {