Skip to content

Conversation

@goosetapa
Copy link
Contributor

@goosetapa goosetapa commented Sep 21, 2024

PR Title:

Remove block and line comments when detecting '.flutter-plugins' in settings.gradle


Description:

This PR modifies the configureLegacyPluginEachProjects function to remove block (/* ... */) and line (// ...) comments from the settings.gradle or settings.gradle.kts file content before checking for the presence of the '.flutter-plugins' string. This ensures that only uncommented, meaningful code is considered during the detection, preventing false positives when the string appears within comments.

Why is this change necessary?

In some cases, the '.flutter-plugins' string may be present inside comments in the settings.gradle file. The existing implementation does not account for this and may incorrectly detect the string even when it's commented out. This can lead to unintended behavior, such as configuring plugin projects when it is not necessary.

By removing comments before performing the check, we prevent false positives and ensure that the detection logic is accurate, only acting when the '.flutter-plugins' string is present in active code.

Changes Made:

  • Added comment removal logic:

    • Removed block comments (/* ... */) using the regular expression /(?s)\/\*.*?\*\//.
      • The (?s) flag enables dot-all mode, allowing . to match newline characters.
    • Removed line comments (// ...) using the regular expression /(?m)\/\/.*$.
      • The (?m) flag enables multi-line mode, so ^ and $ match the start and end of each line.
    • Combined both comment removal steps into a single chain for efficiency.
  • Updated the string detection:

    • The check for '.flutter-plugins' is now performed on the uncommented content of the settings.gradle file.
    • This ensures that only meaningful, uncommented code is considered during detection.

Issue Fixed:


Pre-launch Checklist


If you need any further assistance or have questions, feel free to reach out!


Links:

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests before merging. If you need an exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!).

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing. The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@google-cla
Copy link

google-cla bot commented Sep 21, 2024

Thanks for your pull request! It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA).

View this failed invocation of the CLA check for more information.

For the most up to date status, view the checks section at the bottom of the pull request.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 21, 2024
@andrewkolos andrewkolos self-requested a review September 26, 2024 19:49
@bkonyi bkonyi requested review from bkonyi and removed request for andrewkolos October 3, 2024 20:23
Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry for the delay on this review. LGTM!

Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, it looks like we're missing some imports in gradle_test.dart. Can you add those, fix the trailing space on line 893, and run the test to make sure it's passing?

@flutter-dashboard
Copy link

This pull request executed golden file tests, but it has not been updated in a while (20+ days). Test results from Gold expire after as many days, so this pull request will need to be updated with a fresh commit in order to get results from Gold.

For more guidance, visit Writing a golden file test for package:flutter.

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.

@goosetapa goosetapa reopened this Nov 8, 2024
@goosetapa
Copy link
Contributor Author

To pass the PR checks, I have made several commits, resulting in a lengthy commit history. Therefore, I have performed a rebase and force-pushed the final code submission.

@goosetapa
Copy link
Contributor Author

Actually, it looks like we're missing some imports in gradle_test.dart. Can you add those, fix the trailing space on line 893, and run the test to make sure it's passing?

Yes, I've made the changes as suggested. I've moved the test code from the general.shard to the integration.shard and taken care of it.

@goosetapa goosetapa requested a review from bkonyi November 8, 2024 12:17
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 8, 2024
@bkonyi
Copy link
Contributor

bkonyi commented Nov 8, 2024

Thanks @goosetapa! Looks good to me!

Copy link
Contributor

@andrewkolos andrewkolos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM :)

@andrewkolos andrewkolos added the autosubmit Merge PR when tree becomes green via auto submit App label Nov 11, 2024
@auto-submit auto-submit bot merged commit 18071ec into flutter:master Nov 11, 2024
144 checks passed
@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Nov 11, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 12, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Nov 13, 2024
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Nov 13, 2024
Roll Flutter from c8510f2 to ed553d1 (48 revisions)

flutter/flutter@c8510f2...ed553d1

2024-11-13 [email protected] Avoid using platform `ProcessInfo.maxRss` in test. (flutter/flutter#158526)
2024-11-13 [email protected] Roll Packages from 72356fd to 26e123a (19 revisions) (flutter/flutter#158626)
2024-11-13 [email protected] Move `dart pub deps` call to `<Pub>.deps` and use it accordingly (flutter/flutter#158524)
2024-11-13 [email protected] Roll Flutter Engine from 05c14d8b4cd7 to db3e5af2ca22 (1 revision) (flutter/flutter#158553)
2024-11-13 [email protected] Roll Flutter Engine from ef760d6e1f13 to 05c14d8b4cd7 (3 revisions) (flutter/flutter#158551)
2024-11-13 [email protected] Roll Flutter Engine from 08348c9eebcc to ef760d6e1f13 (1 revision) (flutter/flutter#158545)
2024-11-13 [email protected] Marks Mac_arm64_ios hot_mode_dev_cycle_ios__benchmark to be flaky (flutter/flutter#158242)
2024-11-13 [email protected] Roll Flutter Engine from 877abb9ad6ff to 08348c9eebcc (8 revisions) (flutter/flutter#158541)
2024-11-13 [email protected] Allow `devDependencies` to be omitted and not cause a tool crash. (flutter/flutter#158518)
2024-11-13 [email protected] Explain how to use `flutter channel`. (flutter/flutter#158533)
2024-11-13 [email protected] Clean up dependabot config, add github-action group (flutter/flutter#158408)
2024-11-12 [email protected] Update test to include more complete instructions for how to run tests locally, add example to andoid 11 tests as well (flutter/flutter#158528)
2024-11-12 [email protected] force Linux plugin_test to run on Ubuntu 20.04 (flutter/flutter#158529)
2024-11-12 [email protected] Support materialTapTargetSize in PopupMenuButton (flutter/flutter#158357)
2024-11-12 [email protected] [SwiftPM] Update .flutter-plugin-dependencies format (flutter/flutter#158138)
2024-11-12 [email protected] add filesystem error handling to `systemTempDirectory` (flutter/flutter#158481)
2024-11-12 [email protected] Made Cupertino dialog more like a native dialog in dark mode (flutter/flutter#157218)
2024-11-12 [email protected] Roll Flutter Engine from b0a4ca92c49e to 877abb9ad6ff (2 revisions) (flutter/flutter#158506)
2024-11-12 [email protected] Fix `NavigationBar` label style customization on the widget level (flutter/flutter#158510)
2024-11-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Add test for `dynamic_content_color.0.dart` (#158309)" (flutter/flutter#158511)
2024-11-12 [email protected] Add test for `dynamic_content_color.0.dart` (flutter/flutter#158309)
2024-11-12 [email protected] Roll Flutter Engine from a672f971c659 to b0a4ca92c49e (2 revisions) (flutter/flutter#158488)
2024-11-12 [email protected] Roll Flutter Engine from 35041f118744 to a672f971c659 (1 revision) (flutter/flutter#158487)
2024-11-12 [email protected] Roll Flutter Engine from 7b3eacd20eb6 to 35041f118744 (9 revisions) (flutter/flutter#158485)
2024-11-11 [email protected] [SwiftPM] Simplify logic that determines if CocoaPods is used (flutter/flutter#158409)
2024-11-11 [email protected] Fix Chip draws `InkWell.hoverColor` is drawn on top of the provided background color with `hovered` state (flutter/flutter#158454)
2024-11-11 [email protected] Roll Flutter Engine from 3cb6f4de89b6 to 7b3eacd20eb6 (1 revision) (flutter/flutter#158464)
2024-11-11 [email protected] Roll Packages from bb5a258 to 72356fd (8 revisions) (flutter/flutter#158378)
2024-11-11 [email protected] Roll Flutter Engine from e9a44820f302 to 3cb6f4de89b6 (3 revisions) (flutter/flutter#158456)
2024-11-11 [email protected] Replace custom `RPCErrorCodes` with `RPCErrorKind` from `package:vm_service` (flutter/flutter#158379)
2024-11-11 [email protected] Roll Flutter Engine from d90e9f4718b8 to e9a44820f302 (1 revision) (flutter/flutter#158453)
2024-11-11 [email protected] Roll Flutter Engine from 01c76e42c20f to d90e9f4718b8 (1 revision) (flutter/flutter#158443)
2024-11-11 [email protected] Roll Flutter Engine from 9b4c3b3d5518 to 01c76e42c20f (3 revisions) (flutter/flutter#158438)
2024-11-11 [email protected] Remove block and line comments when detecting '.flutter-plugins' in settings.gradle(.kts) (flutter/flutter#155488)
2024-11-11 [email protected] Add `SafeArea` DartPad sample (flutter/flutter#158019)
2024-11-10 [email protected] Marks Linux analyzer_benchmark to be flaky (flutter/flutter#158244)
2024-11-10 [email protected] remove `bringup` status for recently re-subsharded targets (flutter/flutter#158217)
2024-11-09 [email protected] Roll Flutter Engine from 690cdfd09beb to 9b4c3b3d5518 (1 revision) (flutter/flutter#158418)
2024-11-09 [email protected] Marks Mac_benchmark complex_layout_scroll_perf_macos__timeline_summary to be flaky (flutter/flutter#158252)
2024-11-09 [email protected] Roll Flutter Engine from ca6f5110d9d3 to 690cdfd09beb (1 revision) (flutter/flutter#158414)
2024-11-09 [email protected] Roll Flutter Engine from 2f097cfd3d2d to ca6f5110d9d3 (3 revisions) (flutter/flutter#158411)
2024-11-09 [email protected] Roll Flutter Engine from 54df0b8a4784 to 2f097cfd3d2d (1 revision) (flutter/flutter#158407)
2024-11-08 [email protected] Roll Flutter Engine from b7134d373ef8 to 54df0b8a4784 (2 revisions) (flutter/flutter#158405)
2024-11-08 [email protected] Roll Flutter Engine from 6b77347edfc5 to b7134d373ef8 (3 revisions) (flutter/flutter#158402)
2024-11-08 [email protected] Roll Flutter Engine from 1b567e80386e to 6b77347edfc5 (4 revisions) (flutter/flutter#158398)
2024-11-08 [email protected] Roll Flutter Engine from a08bd5a07c2a to 1b567e80386e (1 revision) (flutter/flutter#158393)
...
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 13, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 6, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 7, 2025
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

The Flutter Gradle plugin's configureLegacyPluginEachProjects function problem

3 participants