Skip to content

Commit d74d687

Browse files
[ci] Check repo-level package metadata (#5811)
Adds a new tool command (and runs it in CI) to check that each package: - is listed correctly in the repo-level README.md table - has a CODEOWNERS entry In the future we could add other things (e.g., auto-label), but these were the main things we've had issues with recently. Updates README.md and CODEOWNERS to fix failures it found: - Adds a couple of missing CODEOWNERS - Expands the web implementation CODEOWNERS to individual packages so that we don't have to special-case handling in the tool - Fixes some minor mistakes in README.md - URL-encodes all `:`s in the README.md links (which is why ever line shows as changed); it worked without that in practice, but it should really be encoded, and having it consistently encoded made things easier for the tooling.
1 parent c5349bc commit d74d687

File tree

7 files changed

+690
-44
lines changed

7 files changed

+690
-44
lines changed

.ci/targets/repo_checks.yaml

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,10 @@ tasks:
4040
script: script/tool_runner.sh
4141
args: ["gradle-check"]
4242
always: true
43+
- name: Repository-level package metadata validation
44+
script: script/tool_runner.sh
45+
args: ["check-repo-package-info"]
46+
always: true
4347
- name: Dependabot coverage validation
4448
script: script/tool_runner.sh
4549
args: ["dependabot-check"]

CODEOWNERS

Lines changed: 14 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -50,7 +50,16 @@ third_party/packages/cupertino_icons/** @MitchellGoodwin
5050
# matching entry takes precedence.
5151

5252
# - Web
53-
packages/**/*_web/** @ditman
53+
packages/camera/camera_web/** @ditman
54+
packages/file_selector/file_selector_web/** @ditman
55+
packages/google_maps_flutter/google_maps_flutter_web/** @ditman
56+
packages/google_sign_in/google_sign_in_web/** @ditman
57+
packages/image_picker/image_picker_for_web/** @ditman
58+
packages/pointer_interceptor/pointer_interceptor_web/** @ditman
59+
packages/shared_preferences/shared_preferences_web/** @ditman
60+
packages/url_launcher/url_launcher_web/** @ditman
61+
packages/video_player/video_player_web/** @ditman
62+
packages/webview_flutter/webview_flutter_web/** @ditman
5463

5564
# - Android
5665
packages/camera/camera_android/** @camsim99
@@ -68,6 +77,8 @@ packages/quick_actions/quick_actions_android/** @camsim99
6877
packages/shared_preferences/shared_preferences_android/** @reidbaker
6978
packages/url_launcher/url_launcher_android/** @gmackall
7079
packages/video_player/video_player_android/** @camsim99
80+
# Owned by ecosystem team for now during the wrapper evaluation.
81+
packages/webview_flutter/webview_flutter_android/** @bparrishMines
7182

7283
# - iOS
7384
packages/camera/camera_avfoundation/** @hellohuanlin
@@ -76,9 +87,10 @@ packages/google_maps_flutter/google_maps_flutter_ios/** @hellohuanlin
7687
packages/google_sign_in/google_sign_in_ios/** @vashworth
7788
packages/image_picker/image_picker_ios/** @vashworth
7889
packages/in_app_purchase/in_app_purchase_storekit/** @louisehsu
79-
packages/ios_platform_images/ios/** @jmagman
90+
packages/ios_platform_images/** @jmagman
8091
packages/local_auth/local_auth_ios/** @louisehsu
8192
packages/path_provider/path_provider_foundation/** @jmagman
93+
packages/pointer_interceptor/pointer_interceptor_ios/** @ditman
8294
packages/quick_actions/quick_actions_ios/** @hellohuanlin
8395
packages/shared_preferences/shared_preferences_foundation/** @tarrinneal
8496
packages/url_launcher/url_launcher_ios/** @jmagman

README.md

Lines changed: 44 additions & 42 deletions
Large diffs are not rendered by default.

script/tool/lib/src/common/repository_package.dart

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,4 +192,9 @@ class RepositoryPackage {
192192
}
193193
return null;
194194
}
195+
196+
/// Returns true if the package is not marked as "publish_to: none".
197+
bool isPublishable() {
198+
return parsePubspec().publishTo != 'none';
199+
}
195200
}

script/tool/lib/src/main.dart

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -33,6 +33,7 @@ import 'publish_command.dart';
3333
import 'pubspec_check_command.dart';
3434
import 'readme_check_command.dart';
3535
import 'remove_dev_dependencies_command.dart';
36+
import 'repo_package_info_check_command.dart';
3637
import 'update_dependency_command.dart';
3738
import 'update_excerpts_command.dart';
3839
import 'update_min_sdk_command.dart';
@@ -82,6 +83,7 @@ void main(List<String> args) {
8283
..addCommand(PubspecCheckCommand(packagesDir))
8384
..addCommand(ReadmeCheckCommand(packagesDir))
8485
..addCommand(RemoveDevDependenciesCommand(packagesDir))
86+
..addCommand(RepoPackageInfoCheckCommand(packagesDir))
8587
..addCommand(DartTestCommand(packagesDir))
8688
..addCommand(UpdateDependencyCommand(packagesDir))
8789
..addCommand(UpdateExcerptsCommand(packagesDir))
Lines changed: 205 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,205 @@
1+
// Copyright 2013 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:file/file.dart';
6+
7+
import 'common/core.dart';
8+
import 'common/output_utils.dart';
9+
import 'common/package_looping_command.dart';
10+
import 'common/repository_package.dart';
11+
12+
const int _exitBadTableEntry = 3;
13+
const int _exitUnknownPackageEntry = 4;
14+
15+
/// A command to verify repository-level metadata about packages, such as
16+
/// repo README and CODEOWNERS entries.
17+
class RepoPackageInfoCheckCommand extends PackageLoopingCommand {
18+
/// Creates Dependabot check command instance.
19+
RepoPackageInfoCheckCommand(super.packagesDir, {super.gitDir});
20+
21+
late Directory _repoRoot;
22+
23+
/// Data from the root README.md table of packages.
24+
final Map<String, List<String>> _readmeTableEntries =
25+
<String, List<String>>{};
26+
27+
/// Packages with entries in CODEOWNERS.
28+
final List<String> _ownedPackages = <String>[];
29+
30+
@override
31+
final String name = 'repo-package-info-check';
32+
33+
@override
34+
List<String> get aliases => <String>['check-repo-package-info'];
35+
36+
@override
37+
final String description =
38+
'Checks that all packages are listed correctly in the repo README.';
39+
40+
@override
41+
final bool hasLongOutput = false;
42+
43+
@override
44+
Future<void> initializeRun() async {
45+
_repoRoot = packagesDir.fileSystem.directory((await gitDir).path);
46+
47+
// Extract all of the README.md table entries.
48+
final RegExp namePattern = RegExp(r'\[(.*?)\]\(');
49+
for (final String line
50+
in _repoRoot.childFile('README.md').readAsLinesSync()) {
51+
// Find all the table entries, skipping the header.
52+
if (line.startsWith('|') &&
53+
!line.startsWith('| Package') &&
54+
!line.startsWith('|-')) {
55+
final List<String> cells = line
56+
.split('|')
57+
.map((String s) => s.trim())
58+
.where((String s) => s.isNotEmpty)
59+
.toList();
60+
// Extract the name, removing any markdown escaping.
61+
final String? name =
62+
namePattern.firstMatch(cells[0])?.group(1)?.replaceAll(r'\_', '_');
63+
if (name == null) {
64+
printError('Unexpected README table line:\n $line');
65+
throw ToolExit(_exitBadTableEntry);
66+
}
67+
_readmeTableEntries[name] = cells;
68+
69+
if (!(packagesDir.childDirectory(name).existsSync() ||
70+
thirdPartyPackagesDir.childDirectory(name).existsSync())) {
71+
printError('Unknown package "$name" in root README.md table.');
72+
throw ToolExit(_exitUnknownPackageEntry);
73+
}
74+
}
75+
}
76+
77+
// Extract all of the CODEOWNERS package entries.
78+
final RegExp packageOwnershipPattern =
79+
RegExp(r'^((?:third_party/)?packages/(?:[^/]*/)?([^/]*))/\*\*');
80+
for (final String line
81+
in _repoRoot.childFile('CODEOWNERS').readAsLinesSync()) {
82+
final RegExpMatch? match = packageOwnershipPattern.firstMatch(line);
83+
if (match == null) {
84+
continue;
85+
}
86+
final String path = match.group(1)!;
87+
final String name = match.group(2)!;
88+
if (!_repoRoot.childDirectory(path).existsSync()) {
89+
printError('Unknown directory "$path" in CODEOWNERS');
90+
throw ToolExit(_exitUnknownPackageEntry);
91+
}
92+
_ownedPackages.add(name);
93+
}
94+
}
95+
96+
@override
97+
Future<PackageResult> runForPackage(RepositoryPackage package) async {
98+
final String packageName = package.directory.basename;
99+
final List<String> errors = <String>[];
100+
101+
// All packages should have an owner.
102+
// Platform interface packages are considered to be owned by the app-facing
103+
// package owner.
104+
if (!(_ownedPackages.contains(packageName) ||
105+
package.isPlatformInterface &&
106+
_ownedPackages.contains(package.directory.parent.basename))) {
107+
printError('${indentation}Missing CODEOWNERS entry.');
108+
errors.add('Missing CODEOWNERS entry');
109+
}
110+
111+
// Any published package should be in the README table.
112+
// For federated plugins, only the app-facing package is listed.
113+
if (package.isPublishable() &&
114+
(!package.isFederated || package.isAppFacing)) {
115+
final List<String>? cells = _readmeTableEntries[packageName];
116+
117+
if (cells == null) {
118+
printError('${indentation}Missing repo root README.md table entry');
119+
errors.add('Missing repo root README.md table entry');
120+
} else {
121+
// Extract the two parts of a "[label](link)" .md link.
122+
final RegExp mdLinkPattern = RegExp(r'^\[(.*)\]\((.*)\)$');
123+
// Possible link targets.
124+
for (final String cell in cells) {
125+
final RegExpMatch? match = mdLinkPattern.firstMatch(cell);
126+
if (match == null) {
127+
printError(
128+
'${indentation}Invalid repo root README.md table entry: "$cell"');
129+
errors.add('Invalid root README.md table entry');
130+
} else {
131+
final String encodedIssueTag =
132+
Uri.encodeComponent(_issueTagForPackage(packageName));
133+
final String encodedPRTag =
134+
Uri.encodeComponent(_prTagForPackage(packageName));
135+
final String anchor = match.group(1)!;
136+
final String target = match.group(2)!;
137+
138+
// The anchor should be one of:
139+
// - The package name (optionally with any underscores escaped)
140+
// - An image with a name-based link
141+
// - An image with a tag-based link
142+
final RegExp packageLink =
143+
RegExp(r'^!\[.*\]\(https://img.shields.io/pub/.*/'
144+
'$packageName'
145+
r'(?:\.svg)?\)$');
146+
final RegExp issueTagLink = RegExp(
147+
r'^!\[.*\]\(https://img.shields.io/github/issues/flutter/flutter/'
148+
'$encodedIssueTag'
149+
r'\?label=\)$');
150+
final RegExp prTagLink = RegExp(
151+
r'^!\[.*\]\(https://img.shields.io/github/issues-pr/flutter/packages/'
152+
'$encodedPRTag'
153+
r'\?label=\)$');
154+
if (!(anchor == packageName ||
155+
anchor == packageName.replaceAll('_', r'\_') ||
156+
packageLink.hasMatch(anchor) ||
157+
issueTagLink.hasMatch(anchor) ||
158+
prTagLink.hasMatch(anchor))) {
159+
printError(
160+
'${indentation}Incorrect anchor in root README.md table: "$anchor"');
161+
errors.add('Incorrect anchor in root README.md table');
162+
}
163+
164+
// The link should be one of:
165+
// - a relative link to the in-repo package
166+
// - a pub.dev link to the package
167+
// - a github label link to the package's label
168+
final RegExp pubDevLink =
169+
RegExp('^https://pub.dev/packages/$packageName(?:/score)?\$');
170+
final RegExp gitHubIssueLink = RegExp(
171+
'^https://github.com/flutter/flutter/labels/$encodedIssueTag\$');
172+
final RegExp gitHubPRLink = RegExp(
173+
'^https://github.com/flutter/packages/labels/$encodedPRTag\$');
174+
if (!(target == './packages/$packageName/' ||
175+
target == './third_party/packages/$packageName/' ||
176+
pubDevLink.hasMatch(target) ||
177+
gitHubIssueLink.hasMatch(target) ||
178+
gitHubPRLink.hasMatch(target))) {
179+
printError(
180+
'${indentation}Incorrect link in root README.md table: "$target"');
181+
errors.add('Incorrect link in root README.md table');
182+
}
183+
}
184+
}
185+
}
186+
}
187+
188+
return errors.isEmpty
189+
? PackageResult.success()
190+
: PackageResult.fail(errors);
191+
}
192+
193+
String _prTagForPackage(String packageName) => 'p: $packageName';
194+
195+
String _issueTagForPackage(String packageName) {
196+
switch (packageName) {
197+
case 'google_maps_flutter':
198+
return 'p: maps';
199+
case 'webview_flutter':
200+
return 'p: webview';
201+
default:
202+
return 'p: $packageName';
203+
}
204+
}
205+
}

0 commit comments

Comments
 (0)