From 4deff2dd9c32fcd46e86ba92fc0a704d7a9df428 Mon Sep 17 00:00:00 2001 From: Pierre-Louis <6655696+guidezpl@users.noreply.github.com> Date: Fri, 19 Apr 2024 10:46:24 +0200 Subject: [PATCH] Update link branches to `main` (continued) (#146985) I generalized the analysis to match all `googlesource.com` repos. I also added a test and fixed more cases. Part of https://github.com/flutter/flutter/issues/121564 ## 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]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I added new tests to check the change I am making, or this PR is [test-exempt]. - [x] 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/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [Features we expect every widget to implement]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo#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/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat [Data Driven Fixes]: https://github.com/flutter/flutter/wiki/Data-driven-Fixes --- dev/bots/analyze.dart | 46 +++++++++---------- dev/bots/test.dart | 1 - .../packages/foo/bad_repository_links.dart | 12 +++++ dev/bots/test/analyze_test.dart | 25 ++++++++++ dev/customer_testing/ci.bat | 2 +- dev/customer_testing/ci.sh | 2 +- .../flutter/lib/src/gestures/monodrag.dart | 2 +- 7 files changed, 62 insertions(+), 28 deletions(-) create mode 100644 dev/bots/test/analyze-test-input/root/packages/foo/bad_repository_links.dart diff --git a/dev/bots/analyze.dart b/dev/bots/analyze.dart index dc8ecd4a3c..9ef8cb8c34 100644 --- a/dev/bots/analyze.dart +++ b/dev/bots/analyze.dart @@ -1316,38 +1316,36 @@ Future verifyRepositoryLinks(String workingDirectory) async { 'tpn/winsdk-10', }; - const List linkPrefixes = [ - 'https://raw.githubusercontent.com/', - 'https://github.com/', - ]; + // See dev/bots/test/analyze-test-input/root/packages/foo/bad_repository_links.dart + // for examples of repository links that are not allowed. + final RegExp pattern = RegExp(r'^(https:\/\/(?:cs\.opensource\.google|github|raw\.githubusercontent|source\.chromium|([a-z0-9\-]+)\.googlesource)\.)'); final List problems = []; final Set suggestions = {}; - final List files = await _gitFiles(workingDirectory); + final List files = await _allFiles(workingDirectory, null, minimumMatches: 10).toList(); for (final File file in files) { - for (final String linkPrefix in linkPrefixes) { - final Uint8List bytes = file.readAsBytesSync(); - // We allow invalid UTF-8 here so that binaries don't trip us up. - // There's a separate test in this file that verifies that all text - // files are actually valid UTF-8 (see verifyNoBinaries below). - final String contents = utf8.decode(bytes, allowMalformed: true); - int start = 0; - while ((start = contents.indexOf(linkPrefix, start)) >= 0) { - int end = start + linkPrefixes.length; - while (end < contents.length && !stops.contains(contents[end])) { - end += 1; - } - final String url = contents.substring(start, end); - if (url.startsWith(linkPrefix) && !repoExceptions.any(url.contains)) { - if (url.contains('master')) { - problems.add('${file.path} contains $url, which uses the banned "master" branch.'); - suggestions.add('Change the URLs above to the expected pattern by ' + final Uint8List bytes = file.readAsBytesSync(); + // We allow invalid UTF-8 here so that binaries don't trip us up. + // There's a separate test in this file that verifies that all text + // files are actually valid UTF-8 (see verifyNoBinaries below). + final String contents = utf8.decode(bytes, allowMalformed: true); + int start = 0; + while ((start = contents.indexOf('https://', start)) >= 0) { // Find all 'https://' links + int end = start + 8; // Length of 'https://' + while (end < contents.length && !stops.contains(contents[end])) { + end += 1; + } + final String url = contents.substring(start, end).replaceAll('\r', ''); + + if (pattern.hasMatch(url) && !repoExceptions.any(url.contains)) { + if (url.contains('master')) { + problems.add('${file.path} contains $url, which uses the banned "master" branch.'); + suggestions.add('Change the URLs above to the expected pattern by ' 'using the "main" branch if it exists, otherwise adding the ' 'repository to the list of exceptions in analyze.dart.'); - } } - start = end; } + start = end; } } assert(problems.isEmpty == suggestions.isEmpty); diff --git a/dev/bots/test.dart b/dev/bots/test.dart index 3341861fc5..709b7c3b49 100644 --- a/dev/bots/test.dart +++ b/dev/bots/test.dart @@ -143,7 +143,6 @@ Future main(List args) async { 'framework_coverage': frameworkCoverageRunner, 'framework_tests': _runFrameworkTests, 'tool_tests': _runToolTests, - // web_tool_tests is also used by HHH: https://dart.googlesource.com/recipes/+/refs/heads/master/recipes/dart/flutter_engine.py 'web_tool_tests': _runWebToolTests, 'tool_integration_tests': _runIntegrationToolTests, 'android_preview_tool_integration_tests': _runAndroidPreviewIntegrationToolTests, diff --git a/dev/bots/test/analyze-test-input/root/packages/foo/bad_repository_links.dart b/dev/bots/test/analyze-test-input/root/packages/foo/bad_repository_links.dart new file mode 100644 index 0000000000..38417c1831 --- /dev/null +++ b/dev/bots/test/analyze-test-input/root/packages/foo/bad_repository_links.dart @@ -0,0 +1,12 @@ +// Copyright 2014 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +// Check out https://android.googlesource.com/+/master/file1 +// Check out https://chromium.googlesource.com/+/master/file1 +// Check out https://cs.opensource.google.com/+/master/file1 +// Check out https://dart.googlesource.com/+/master/file1 +// Check out https://flutter.googlesource.com/+/master/file1 +// Check out https://source.chromium.org/+/master/file1 +// Check out https://github.com/flutter/flutter/tree/master/file1 +// Check out https://raw.githubusercontent.com/flutter/flutter/blob/master/file1 diff --git a/dev/bots/test/analyze_test.dart b/dev/bots/test/analyze_test.dart index c819e2720d..1ed50e053e 100644 --- a/dev/bots/test/analyze_test.dart +++ b/dev/bots/test/analyze_test.dart @@ -147,6 +147,31 @@ void main() { ); }); + test('analyze.dart - verifyRepositoryLinks', () async { + final String result = await capture(() => verifyRepositoryLinks(testRootPath), shouldHaveErrors: true); + const String bannedBranch = 'master'; + final String file = Platform.isWindows ? + r'test\analyze-test-input\root\packages\foo\bad_repository_links.dart' : + 'test/analyze-test-input/root/packages/foo/bad_repository_links.dart'; + final String lines = [ + '║ $file contains https://android.googlesource.com/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://chromium.googlesource.com/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://cs.opensource.google.com/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://dart.googlesource.com/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://flutter.googlesource.com/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://source.chromium.org/+/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://github.com/flutter/flutter/tree/$bannedBranch/file1, which uses the banned "master" branch.', + '║ $file contains https://raw.githubusercontent.com/flutter/flutter/blob/$bannedBranch/file1, which uses the banned "master" branch.', + '║ Change the URLs above to the expected pattern by using the "main" branch if it exists, otherwise adding the repository to the list of exceptions in analyze.dart.', + ] + .join('\n'); + expect(result, + '╔═╡ERROR #1╞════════════════════════════════════════════════════════════════════\n' + '$lines\n' + '╚═══════════════════════════════════════════════════════════════════════════════\n' + ); + }); + test('analyze.dart - verifyNoBinaries - positive', () async { final String result = await capture(() => verifyNoBinaries( testRootPath, diff --git a/dev/customer_testing/ci.bat b/dev/customer_testing/ci.bat index 8afdafbe61..56cf7db139 100644 --- a/dev/customer_testing/ci.bat +++ b/dev/customer_testing/ci.bat @@ -6,7 +6,7 @@ REM found in the LICENSE file. REM This should match the ci.sh file in this directory. REM This is called from the LUCI recipes: -REM https://flutter.googlesource.com/recipes/+/refs/heads/master/recipe_modules/adhoc_validation/resources/customer_testing.bat +REM https://github.com/flutter/flutter/blob/main/dev/bots/suite_runners/run_customer_testing_tests.dart ECHO. ECHO Updating pub packages... diff --git a/dev/customer_testing/ci.sh b/dev/customer_testing/ci.sh index 3bc3d3ab86..dbc52e9fca 100755 --- a/dev/customer_testing/ci.sh +++ b/dev/customer_testing/ci.sh @@ -6,7 +6,7 @@ # This should match the ci.bat file in this directory. # This is called from .cirrus.yml and the LUCI recipes: -# https://flutter.googlesource.com/recipes/+/refs/heads/master/recipe_modules/adhoc_validation/resources/customer_testing.sh +# https://github.com/flutter/flutter/blob/main/dev/bots/suite_runners/run_customer_testing_tests.dart set -ex diff --git a/packages/flutter/lib/src/gestures/monodrag.dart b/packages/flutter/lib/src/gestures/monodrag.dart index 3ed4b3ad99..d3f44152ff 100644 --- a/packages/flutter/lib/src/gestures/monodrag.dart +++ b/packages/flutter/lib/src/gestures/monodrag.dart @@ -669,7 +669,7 @@ abstract class DragGestureRecognizer extends OneSequenceGestureRecognizer { // it keeps track of the last accepted pointer. If this active pointer // leave up, it will be set to the first accepted pointer. // Refer to the implementation of Android `RecyclerView`(line 3846): - // https://android.googlesource.com/platform/frameworks/support/+/refs/heads/androidx-master-dev/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/RecyclerView.java + // https://android.googlesource.com/platform/frameworks/support/+/refs/heads/androidx-main/recyclerview/recyclerview/src/main/java/androidx/recyclerview/widget/RecyclerView.java int? _activePointer; @override