[flutter_plugin_tools] Improve 'repository' check

[stuartmorgan]

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.

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.

[flutter-dashboard]

This pull request is not mergeable in its current state, likely because of a merge conflict. Pre-submit CI jobs were not triggered. Pushing a new commit to this branch that resolves the issue will result in pre-submit jobs being scheduled.

[gaaclarke]

lgtm

[gaaclarke]

nit: s/package/packageDir here and elsewhere as per my previous comment about packages sometimes being an object, sometimes being a path and sometimes being a directory.

[stuartmorgan]

This is an override that exists in almost every command in the tool, and it's called package in every case; I really don't want to make just one command inconsistent in this regard.

Instead I'll do a follow-up PR shortly to introduce a Package object that we can start switching over to, and make sure the initial round of conversion in that PR includes all of the runForPackage instances.

[gaaclarke]

nit s/packagePath/relativePackagePath

[stuartmorgan]

Good call, done.

[gaaclarke]

Relative to what?

[stuartmorgan]

Fixed to repositoryPackagesDirRelativePath, and added discussion of it to the comment (which I completely forgot to do originally).

[gaaclarke]

FYI I'm running into this problem with pigeon too where I assert the error text matches for a certain scenario but they are a bit fragile. It would be better to assert on a codename for errors like some other compilers do.

Error: Missing "implements: foo" in "plugin" section. [missing_implements] then use assert(error, contains('[missing_implements]')

[stuartmorgan]

I accidentally sent a PR from a branch that was on top of another PR, rather than master; this isn't actually part of the change. But in general this would apply to the whole tool though, so is worth considering.

The main difference relative to a compiler is that being directly read by humans is the only intended use case for these log messages. That means that the text itself is actually the thing I want to assert on, because if we somehow broke things such that instead of:

Missing "implements: plugin_a" in "plugin" section. [missing_implements]

it just printed:

[missing_implements]

that would be really bad.

I assume what you mean when you say they are fragile is that if the wording of the message changes, the tests need to change as well. Sometimes that is indeed mildly annoying (e.g., if it's a typo fix) but in most cases that's actually the desired outcome because having a clear, detailed error message is a critical part of the desired behavior.

(There's also no "ignore missing_implements" option, nor do we ever intend to build one since this isn't a general-purpose tool, which is the other main use case for having those kind of machine-friendly error names.)

[stuartmorgan]

submit-queue status is wrong yet again; landing into what is actually a green tree.