Skip to content

Commit b1fe191

Browse files
[flutter_plugin_tools] Improve 'repository' check (flutter#4244)
Ensures that the full relative path in the 'repository' link is correct, not just the last segment. This ensure that path-level errors (e.g., linking to the group directory rather than the package itself for app-facing packages) are caught. Also fixes the errors that this improved check catches, including several cases where a previously unfederated package wasn't fixed when it was moved to a subdirectory.
1 parent f2b42f7 commit b1fe191

File tree

9 files changed

+81
-17
lines changed

9 files changed

+81
-17
lines changed

packages/in_app_purchase/in_app_purchase/CHANGELOG.md

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,7 @@
1+
## 1.0.8
2+
3+
* Fix repository link in pubspec.yaml.
4+
15
## 1.0.7
26

37
* Remove references to the Android V1 embedding.

packages/in_app_purchase/in_app_purchase/pubspec.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
name: in_app_purchase
22
description: A Flutter plugin for in-app purchases. Exposes APIs for making in-app purchases through the App Store and Google Play.
3-
repository: https://github.com/flutter/plugins/tree/master/packages/in_app_purchase
3+
repository: https://github.com/flutter/plugins/tree/master/packages/in_app_purchase/in_app_purchase
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+in_app_purchase%22
5-
version: 1.0.7
5+
version: 1.0.8
66

77
environment:
88
sdk: ">=2.12.0 <3.0.0"

packages/ios_platform_images/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
## NEXT
1+
## 0.2.0+1
22

33
* Add iOS unit test target.
4+
* Fix repository link in pubspec.yaml.
45

56
## 0.2.0
67

packages/ios_platform_images/pubspec.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
name: ios_platform_images
22
description: A plugin to share images between Flutter and iOS in add-to-app setups.
3-
repository: https://github.com/flutter/plugins/tree/master/packages/ios_platform_images/ios_platform_images
3+
repository: https://github.com/flutter/plugins/tree/master/packages/ios_platform_images
44
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+ios_platform_images%22
5-
version: 0.2.0
5+
version: 0.2.0+1
66

77
environment:
88
sdk: ">=2.12.0 <3.0.0"

packages/quick_actions/quick_actions/CHANGELOG.md

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,6 +1,7 @@
1-
## NEXT
1+
## 0.6.0+6
22

33
* Updated Android lint settings.
4+
* Fix repository link in pubspec.yaml.
45

56
## 0.6.0+5
67

packages/quick_actions/quick_actions/pubspec.yaml

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,9 +1,9 @@
11
name: quick_actions
22
description: Flutter plugin for creating shortcuts on home screen, also known as
33
Quick Actions on iOS and App Shortcuts on Android.
4-
repository: https://github.com/flutter/plugins/tree/master/packages/quick_actions
4+
repository: https://github.com/flutter/plugins/tree/master/packages/quick_actions/quick_actions
55
issue_tracker: https://github.com/flutter/flutter/issues?q=is%3Aissue+is%3Aopen+label%3A%22p%3A+quick_actions%22
6-
version: 0.6.0+5
6+
version: 0.6.0+6
77

88
environment:
99
sdk: ">=2.12.0 <3.0.0"

script/tool/CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
- Added Android native integration test support to `native-test`.
44
- Added a new `android-lint` command to lint Android plugin native code.
55
- Pubspec validation now checks for `implements` in implementation packages.
6+
- Pubspec valitation now checks the full relative path of `repository` entries.
67

78
## 0.5.0
89

script/tool/lib/src/pubspec_check_command.dart

Lines changed: 9 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -98,7 +98,7 @@ class PubspecCheckCommand extends PackageLoopingCommand {
9898

9999
if (pubspec.publishTo != 'none') {
100100
final List<String> repositoryErrors =
101-
_checkForRepositoryLinkErrors(pubspec, packageName: package.basename);
101+
_checkForRepositoryLinkErrors(pubspec, package: package);
102102
if (repositoryErrors.isNotEmpty) {
103103
for (final String error in repositoryErrors) {
104104
printError('$indentation$error');
@@ -154,14 +154,18 @@ class PubspecCheckCommand extends PackageLoopingCommand {
154154

155155
List<String> _checkForRepositoryLinkErrors(
156156
Pubspec pubspec, {
157-
required String packageName,
157+
required Directory package,
158158
}) {
159159
final List<String> errorMessages = <String>[];
160160
if (pubspec.repository == null) {
161161
errorMessages.add('Missing "repository"');
162-
} else if (!pubspec.repository!.path.endsWith(packageName)) {
163-
errorMessages
164-
.add('The "repository" link should end with the package name.');
162+
} else {
163+
final String relativePackagePath =
164+
path.relative(package.path, from: packagesDir.parent.path);
165+
if (!pubspec.repository!.path.endsWith(relativePackagePath)) {
166+
errorMessages
167+
.add('The "repository" link should end with the package path.');
168+
}
165169
}
166170

167171
if (pubspec.homepage != null) {

script/tool/test/pubspec_check_command_test.dart

Lines changed: 57 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -37,15 +37,29 @@ void main() {
3737
runner.addCommand(command);
3838
});
3939

40+
/// Returns the top section of a pubspec.yaml for a package named [name],
41+
/// for either a flutter/packages or flutter/plugins package depending on
42+
/// the values of [isPlugin].
43+
///
44+
/// By default it will create a header that includes all of the expected
45+
/// values, elements can be changed via arguments to create incorrect
46+
/// entries.
47+
///
48+
/// If [includeRepository] is true, by default the path in the link will
49+
/// be "packages/[name]"; a different "packages"-relative path can be
50+
/// provided with [repositoryPackagesDirRelativePath].
4051
String headerSection(
4152
String name, {
4253
bool isPlugin = false,
4354
bool includeRepository = true,
55+
String? repositoryPackagesDirRelativePath,
4456
bool includeHomepage = false,
4557
bool includeIssueTracker = true,
4658
}) {
59+
final String repositoryPath = repositoryPackagesDirRelativePath ?? name;
4760
final String repoLink = 'https://github.com/flutter/'
48-
'${isPlugin ? 'plugins' : 'packages'}/tree/master/packages/$name';
61+
'${isPlugin ? 'plugins' : 'packages'}/tree/master/'
62+
'packages/$repositoryPath';
4963
final String issueTrackerLink =
5064
'https://github.com/flutter/flutter/issues?'
5165
'q=is%3Aissue+is%3Aopen+label%3A%22p%3A+$name%22';
@@ -250,6 +264,32 @@ ${devDependenciesSection()}
250264
);
251265
});
252266

267+
test('fails when repository is incorrect', () async {
268+
final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);
269+
270+
pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
271+
${headerSection('plugin', isPlugin: true, repositoryPackagesDirRelativePath: 'different_plugin')}
272+
${environmentSection()}
273+
${flutterSection(isPlugin: true)}
274+
${dependenciesSection()}
275+
${devDependenciesSection()}
276+
''');
277+
278+
Error? commandError;
279+
final List<String> output = await runCapturingPrint(
280+
runner, <String>['pubspec-check'], errorHandler: (Error e) {
281+
commandError = e;
282+
});
283+
284+
expect(commandError, isA<ToolExit>());
285+
expect(
286+
output,
287+
containsAllInOrder(<Matcher>[
288+
contains('The "repository" link should end with the package path.'),
289+
]),
290+
);
291+
});
292+
253293
test('fails when issue tracker is missing', () async {
254294
final Directory pluginDirectory = createFakePlugin('plugin', packagesDir);
255295

@@ -446,7 +486,11 @@ ${devDependenciesSection()}
446486
'plugin_a_foo', packagesDir.childDirectory('plugin_a'));
447487

448488
pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
449-
${headerSection('plugin_a_foo', isPlugin: true)}
489+
${headerSection(
490+
'plugin_a_foo',
491+
isPlugin: true,
492+
repositoryPackagesDirRelativePath: 'plugin_a/plugin_a_foo',
493+
)}
450494
${environmentSection()}
451495
${flutterSection(isPlugin: true, implementedPackage: 'plugin_a')}
452496
${dependenciesSection()}
@@ -470,7 +514,11 @@ ${devDependenciesSection()}
470514
createFakePlugin('plugin_a', packagesDir.childDirectory('plugin_a'));
471515

472516
pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
473-
${headerSection('plugin_a', isPlugin: true)}
517+
${headerSection(
518+
'plugin_a',
519+
isPlugin: true,
520+
repositoryPackagesDirRelativePath: 'plugin_a/plugin_a',
521+
)}
474522
${environmentSection()}
475523
${flutterSection(isPlugin: true)}
476524
${dependenciesSection()}
@@ -496,7 +544,12 @@ ${devDependenciesSection()}
496544
packagesDir.childDirectory('plugin_a'));
497545

498546
pluginDirectory.childFile('pubspec.yaml').writeAsStringSync('''
499-
${headerSection('plugin_a_platform_interface', isPlugin: true)}
547+
${headerSection(
548+
'plugin_a_platform_interface',
549+
isPlugin: true,
550+
repositoryPackagesDirRelativePath:
551+
'plugin_a/plugin_a_platform_interface',
552+
)}
500553
${environmentSection()}
501554
${flutterSection(isPlugin: true)}
502555
${dependenciesSection()}

0 commit comments

Comments
 (0)