[flutter_plugin_tools] Support YAML exception lists

[stuartmorgan]

Currently the tool accepts --custom-analysis to allow a list of packages for which custom analysis_options.yaml are allowed, and --exclude to exclude a set of packages when running a command against all, or all changed, packages. This results in these exception lists being embedded into CI configuration files (e.g., .cirrus.yaml) or scripts, which makes them harder to maintain, and harder to re-use in other contexts (local runs, new CI systems).

This adds support for both flags to accept paths to YAML files that contain the lists, so that they can be maintained separately, and with inline comments about the reasons things are on the lists.

Also updates the CI to use this new support, eliminating those lists from .cirrus.yaml and tool_runner.sh

Fixes flutter/flutter#86799

Pre-launch Checklist

• I read the Contributor Guide and followed the process outlined there for submitting PRs.
• I read the Tree Hygiene wiki page, which explains my responsibilities.
• I read and followed the relevant style guides and ran the auto-formatter. (Note that unlike the flutter/flutter repo, the flutter/plugins repo does use dart format.)
• I signed the CLA.
• The title of the PR starts with the name of the plugin surrounded by square brackets, e.g. [shared_preferences]
• I listed at least one issue that this PR fixes in the description above.
• I updated pubspec.yaml with an appropriate new version according to the pub versioning philosophy.
• I updated CHANGELOG.md to add a description of the change.
• I updated/added relevant documentation (doc comments with ///).
• I added new tests to check the change I am making or feature I am adding, or Hixie said the PR is test exempt.
• All existing and new tests are passing.

[stuartmorgan]

Nothing uses this, so I removed it as opportunistic cleanup while removing the condition below.

[gaaclarke]

LGTM, nice improvement

[devoncarew]

Hey - in the future, can you ping me when you make changes to ./script/tool_runner.sh analyze that might effect how other repos should drive it? Thanks!

[devoncarew]

https://dart-review.googlesource.com/c/sdk/+/207981

[gaaclarke]

Seems like we should have a comment or ci check to try to avoid this in the future. I didn't even know the dart bots ran flutter plugins tests, makes sense though.

Or we could make a script that wraps up the analyze invocation so we just have to change it in one repo.

[stuartmorgan]

Hey - in the future, can you ping me when you make changes to ./script/tool_runner.sh analyze that might effect how other repos should drive it? Thanks!

Sorry, I completely forgot you'd set that up in the Dart CI :(

Now that we have the config file it may be safer to have that test use flutter_plugin_tools directly rather than using the wrapper script.

[stuartmorgan]

I think either moving off of using the wrapper script and putting a comment in the yaml file, or making a trivial dedicated wrapper per @gaaclarke's suggestion, are the best long-term options, since the goal now is to minimize, or even eliminate, the current shell script

[devoncarew]

I do expect some (infrequent) breakages due to driving this from an upstream repo. But whatever we can do to make this reasonably stable - with most of the knowledge of how to analyze the repo in the repo itself, and only a small entry-point that something off-repo would need to depend on - would be great.

For reference, this is where we drive it from (from our analyzer pre-submit bot): https://github.com/dart-lang/sdk/blob/master/tools/bots/flutter/analyze_flutter_plugins.sh. The less that script has to know about the internals of the flutter/plugins repo to more stable it'll be.

[stuartmorgan]

A wrapper script is safer since it's a single point of failure, it's just not ideal that we would have to run that one command differently forever.

If we put the details of the call in the Dart repo script it will break if we change how analyze should be called (fairly unlikely) or we change that file (e.g., eliminate it once everything is off the legacy options). We could put heavily commented unit test that uses exactly the args you put in that script though. That would have the advantage of covering the custom SDK argument, which you are the only client of (so someone could change it remove later without realizing it would break you, currently).

My preference would be for the latter if you're okay with occasional update as long as we have a process (the test) for making sure we don't forgot to tell you like I did this time.

[devoncarew]

I suspect that longer term we'd want the --custom-analysis arg to go away? Perhaps have it default to a config file in the repo if no --custom-analysis is passed in, so that bare uses of ./script/tool_runner.sh analyze will just work?

[stuartmorgan]

I suspect that longer term we'd want the --custom-analysis arg to go away?

The long term goal is to not have anything in the repo that has its own analysis options: flutter/flutter#76229

Perhaps have it default to a config file in the repo if no --custom-analysis is passed in, so that bare uses of ./script/tool_runner.sh analyze will just work?

I considered this, including making the test exclusion configs a big config file that was broken down by test type and platform that was auto-included. My preference is to make things that we are doing that we don't want to be doing (like using legacy options, or not running tests for some plugins) explicit though.