fix: A referenced default package must be a plugin package #152134
Conversation
github-actions
bot
added
the
tool
|
Open for review @stuartmorgan |
| @@ -1263,7 1263,7 @@ Map<String, List<Plugin>> _resolvePluginImplementations( | |||
| } | |||
| } else { | |||
| globals.printWarning( | |||
| 'Package ${plugin.name}:$platformKey references $defaultImplPluginName:$platformKey as the default plugin, but the package does not exist.\n' | |||
| 'Package ${plugin.name}:$platformKey references $defaultImplPluginName:$platformKey as the default plugin, but the package does not exist, or is not a plugin package.\n' | |||
There was a problem hiding this comment.
Actually, this may also be slightly wrong. Is this check based on post-platform-filtering, or pre-platform-filtering? I.e., if $defaultImplPluginName exists, and is a plugin package, but does not have a flutter:plugin:platforms: entry for platformKey, will we still hit this codepath?
If so, we need to rephrase to "or is not a plugin package that provides a $platformKey implementation."
There was a problem hiding this comment.
@stuartmorgan
This check involves all plugins, which have this condition. So it's pre-platform-filtering. Means, all with the flutter: plugin: in the pubspec are considered. So I think the error message is correct that way.
We test the specific platform implementation a few lines above, which should cover your mentioned case (if I understood your concern right): https://github.com/flutter/flutter/pull/152134/files#diff-68361f6fc9aafa5fa7de042eec4603dd4f321eb783a00edd57780ef78c1094bbR1259
There was a problem hiding this comment.
Makes sense, thanks. Ideally the check should be stricter and require a platform implementation, but we can revisit that if it ever actually matters in practice.
| @@ -1263,7 1263,7 @@ Map<String, List<Plugin>> _resolvePluginImplementations( | |||
| } | |||
| } else { | |||
| globals.printWarning( | |||
| 'Package ${plugin.name}:$platformKey references $defaultImplPluginName:$platformKey as the default plugin, but the package does not exist.\n' | |||
| 'Package ${plugin.name}:$platformKey references $defaultImplPluginName:$platformKey as the default plugin, but the package does not exist, or is not a plugin package.\n' | |||
| 'Ask the maintainers of ${plugin.name} to either avoid referencing a default implementation via `platforms: $platformKey: default_package: $defaultImplPluginName` ' | |||
| 'or create a plugin named $defaultImplPluginName.\n', | |||
There was a problem hiding this comment.
Let's also change this to "or ensure that $defaultImplPluginNameis an existing plugin package that specifies aplatformKey` implementation."
There was a problem hiding this comment.
See the other comment.
christopherfujino
added
the
autosubmit
Clarify that a referenced default package must also be a plugin package (one, which provides a pluginClass or a dartPluginClass).
Fixes #152037
More precisely #152037 (comment).
Introduced in #137040
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.