Skip to content

Commit 78cfc1a

Browse files
authored
Plugin.isDevDependency if exclusively in dev_dependencies (#157462)
Work towards flutter/flutter#56591. I explicitly want an LGTM from @andrewkolos @jmagman @jonahwilliams before merging. --- After this PR, `<Plugin>.isDevDependency` is resolved based on the following logic, IFF: - The plugin comes from a package _A_ listed in the app's package's `dev_dependencies: ...` - The package _A_ is not a normal dependency of any transitive non-dev dependency of the app See [`compute_dev_dependencies_test.dart`](https://github.com/flutter/flutter/blob/51676093a3b67434405aacf4d56088573863630a/packages/flutter_tools/test/general.shard/compute_dev_dependencies_test.dart) for probably the best specification of this behavior. We (still) do not write the property to disk (i.e. it never makes it to `.flutter-plugins-dependencies`), so there is no impact to build artifacts at this time; that would come in a follow-up PR (and then follow-up follow-up PRs for the various build systems in both Gradle and Xcode to actually use that value to omit dependencies). Some tests had to be updated; for the most part it was updating the default `ProcessManager` because a call to `dart pub deps --json` is now made in code that computes what plugins are available, but there should be no change in behavior. _/cc @jonasfj @sigurdm for FYI only (we talked on an internal thread about this; see dart-lang/sdk#56968 _/cc @camsim99 @cbracken @johnmccutchan for visibility on the change._
1 parent bd65732 commit 78cfc1a

27 files changed

+959
-153
lines changed

packages/flutter_tools/lib/src/commands/create.dart

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -459,7 +459,7 @@ class CreateCommand extends CreateBase {
459459
macOSPlatform: includeMacos,
460460
windowsPlatform: includeWindows,
461461
webPlatform: includeWeb,
462-
writeLegacyPluginsList: boolArg(FlutterGlobalOptions.kImplicitPubspecResolution, global: true),
462+
useImplicitPubspecResolution: boolArg(FlutterGlobalOptions.kImplicitPubspecResolution, global: true),
463463
);
464464
}
465465
}

packages/flutter_tools/lib/src/commands/packages.dart

Lines changed: 4 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -386,14 +386,14 @@ class PackagesGetCommand extends FlutterCommand {
386386
if (rootProject != null) {
387387
// We need to regenerate the platform specific tooling for both the project
388388
// itself and example(if present).
389-
final bool writeLegacyPluginsList = boolArg(FlutterGlobalOptions.kImplicitPubspecResolution, global: true);
389+
final bool useImplicitPubspecResolution = boolArg(FlutterGlobalOptions.kImplicitPubspecResolution, global: true);
390390
await rootProject.regeneratePlatformSpecificTooling(
391-
writeLegacyPluginsList: writeLegacyPluginsList,
391+
useImplicitPubspecResolution: useImplicitPubspecResolution,
392392
);
393393
if (example && rootProject.hasExampleApp && rootProject.example.pubspecFile.existsSync()) {
394394
final FlutterProject exampleProject = rootProject.example;
395395
await exampleProject.regeneratePlatformSpecificTooling(
396-
writeLegacyPluginsList: writeLegacyPluginsList,
396+
useImplicitPubspecResolution: useImplicitPubspecResolution,
397397
);
398398
}
399399
}
@@ -407,7 +407,7 @@ class PackagesGetCommand extends FlutterCommand {
407407
return <Plugin>[];
408408
}
409409

410-
return findPlugins(rootProject, throwOnError: false);
410+
return findPlugins(rootProject, throwOnError: false, useImplicitPubspecResolution: boolArg(FlutterGlobalOptions.kImplicitPubspecResolution, global: true));
411411
})();
412412

413413
late final String? _androidEmbeddingVersion = _rootProject?.android.getEmbeddingVersion().toString().split('.').last;
Lines changed: 164 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,164 @@
1+
// Copyright 2014 The Flutter Authors. All rights reserved.
2+
// Use of this source code is governed by a BSD-style license that can be
3+
// found in the LICENSE file.
4+
5+
import 'package:process/process.dart';
6+
7+
import 'base/io.dart';
8+
import 'base/logger.dart';
9+
import 'convert.dart';
10+
11+
/// Returns dependencies of [project] that are _only_ used as `dev_dependency`.
12+
///
13+
/// That is, computes and returns a subset of dependencies, where the original
14+
/// set is based on packages listed as [`dev_dependency`][dev_deps] in the
15+
/// `pubspec.yaml` file, and removing packages from that set that appear as
16+
/// dependencies (implicitly non-dev) in any non-dev package depended on.
17+
Future<Set<String>> computeExclusiveDevDependencies(
18+
ProcessManager processes, {
19+
required Logger logger,
20+
required String projectPath,
21+
}) async {
22+
final ProcessResult processResult = await processes.run(
23+
<String>['dart', 'pub', 'deps', '--json'],
24+
workingDirectory: projectPath,
25+
);
26+
27+
Never fail([String? reason]) {
28+
final Object? stdout = processResult.stdout;
29+
if (stdout is String && stdout.isNotEmpty) {
30+
logger.printTrace(stdout);
31+
}
32+
final String stderr = processResult.stderr.toString();
33+
throw StateError(
34+
'dart pub deps --json ${reason != null ? 'had unexpected output: $reason' : 'failed'}'
35+
'${stderr.isNotEmpty ? '\n$stderr' : ''}',
36+
);
37+
}
38+
39+
// Guard against dart pub deps crashing.
40+
final Map<String, Object?> jsonResult;
41+
if (processResult.exitCode != 0 || processResult.stdout is! String) {
42+
fail();
43+
}
44+
45+
// Guard against dart pub deps having explicitly invalid output.
46+
final String stdout;
47+
try {
48+
stdout = processResult.stdout as String;
49+
50+
// This is an indication that `FakeProcessManager.any` was used, which by
51+
// contract emits exit code 0 and no output on either stdout or stderr. To
52+
// avoid this code, we'd have to go and make this function injectable into
53+
// every callsite and mock-it out manually, which at the time of this
54+
// writing was 130+ unit test cases alone.
55+
//
56+
// So, this is the lesser of two evils.
57+
if (stdout.isEmpty && processResult.stderr == '') {
58+
return <String>{};
59+
}
60+
61+
jsonResult = json.decode(stdout) as Map<String, Object?>;
62+
} on FormatException catch (e) {
63+
fail('$e');
64+
}
65+
66+
List<T> asListOrFail<T>(Object? value, String name) {
67+
if (value is! List<Object?>) {
68+
fail('Expected field "$name" to be a list, got "$value"');
69+
}
70+
return <T>[
71+
for (final Object? any in value)
72+
if (any is T) any else fail('Expected element to be a $T, got "$any"')
73+
];
74+
}
75+
76+
// Parse the JSON roughly in the following format:
77+
//
78+
// ```json
79+
// {
80+
// "root": "my_app",
81+
// "packages": [
82+
// {
83+
// "name": "my_app",
84+
// "kind": "root",
85+
// "dependencies": [
86+
// "foo_plugin",
87+
// "bar_plugin"
88+
// ],
89+
// "directDependencies": [
90+
// "foo_plugin"
91+
// ],
92+
// "devDependencies": [
93+
// "bar_plugin"
94+
// ]
95+
// }
96+
// ]
97+
// }
98+
// ```
99+
final List<Map<String, Object?>> packages = asListOrFail(
100+
jsonResult['packages'],
101+
'packages',
102+
);
103+
104+
Map<String, Object?> packageWhere(
105+
bool Function(Map<String, Object?>) test, {
106+
required String reason,
107+
}) {
108+
return packages.firstWhere(test, orElse: () => fail(reason));
109+
}
110+
111+
final Map<String, Object?> rootPackage = packageWhere(
112+
(Map<String, Object?> package) => package['kind'] == 'root',
113+
reason: 'A package with kind "root" was not found.',
114+
);
115+
116+
// Start initially with every `devDependency` listed.
117+
final Set<String> devDependencies = asListOrFail<String>(
118+
rootPackage['devDependencies'],
119+
'devDependencies',
120+
).toSet();
121+
122+
// Then traverse and exclude non-dev dependencies that list that dependency.
123+
//
124+
// This avoids the pathalogical problem of using, say, `path_provider` in a
125+
// package's dev_dependencies:, but a (non-dev) dependency using it as a
126+
// standard dependency - in that case we would not want to report it is used
127+
// as a dev dependency.
128+
final Set<String> visited = <String>{};
129+
void visitPackage(String packageName) {
130+
final bool wasAlreadyVisited = !visited.add(packageName);
131+
if (wasAlreadyVisited) {
132+
return;
133+
}
134+
135+
final Map<String, Object?> package = packageWhere(
136+
(Map<String, Object?> package) => package['name'] == packageName,
137+
reason: 'A package with name "$packageName" was not found',
138+
);
139+
140+
// Do not traverse packages that themselves are dev dependencies.
141+
if (package['kind'] == 'dev') {
142+
return;
143+
}
144+
145+
final List<String> directDependencies = asListOrFail(
146+
package['directDependencies'],
147+
'directDependencies',
148+
);
149+
150+
// Remove any listed dependency from dev dependencies; it might have been
151+
// a dev dependency for the app (root) package, but it is being used as a
152+
// real dependency for a dependend on package, so we would not want to send
153+
// a signal that the package can be ignored/removed.
154+
devDependencies.removeAll(directDependencies);
155+
156+
// And continue visiting (visitPackage checks for circular loops).
157+
directDependencies.forEach(visitPackage);
158+
}
159+
160+
// Start with the root package.
161+
visitPackage(rootPackage['name']! as String);
162+
163+
return devDependencies;
164+
}

packages/flutter_tools/lib/src/dart_pub_json_formatter.dart

Lines changed: 0 additions & 63 deletions
This file was deleted.

0 commit comments

Comments
 (0)