[flutter_tools] build ios-frameworks: option to exclude plugin frameworks from the build #129739
Conversation
p-mazhnik
changed the title
p-mazhnik
force-pushed
the
ios-frameworks-no-plugins
branch
from
876467d to
c74bf10
Compare
p-mazhnik
force-pushed
the
ios-frameworks-no-plugins
branch
from
59f0075 to
95d4343
Compare
github-actions
bot
added
c: contributor-productivity
|
Am I understanding correctly that the only benefit here is reducing build times? If so, in isolation it's not clear to me that this is worth the complexity of adding and supporting. The flag seems rather difficult to use correctly, and it seems like the benefit of having it is minimal, so it seems like adding it as a supported output mode is something of a foot-gun. |
|
We also need this. Currently we have one Flutter module used in two iOS App, it has a number of plugins which transitively depend on many of native iOS Pods. We build this module twice with some difference in During each build process, the But we only need a new App.xcframework at the second build, so a flag like |
|
@stuartmorgan Please reconsider this, reducing build time is something important when build time is a annoying problem. IMO, it is sad to ignore a helpful feature only for avoiding people use it wrong |
|
Similar problem, we are also facing the build time cost problem. |
|
Per discussion on discord, this is on @stuartmorgan TODO list. |
|
We need this too. |
|
@stuartmorgan friendly ping |
1 similar comment
|
@stuartmorgan friendly ping |
| ..addFlag('plugins', | ||
| defaultsTo: true, | ||
| help: 'Whether to produce frameworks for the plugins. ' | ||
| 'Consider using "--no-plugins" if plugin frameworks are not required.', |
There was a problem hiding this comment.
This second sentence doesn't seem to be adding anything; it's just saying to consider using the flag if you want the behavior of the flag. (It also sounds like people should pass it if they have no plugins, which isn't necessary or useful.)
I think this should instead discuss how this would need to be used. E.g., "This is intended for cases where plugins are already being built separately."
| help: 'Whether to produce frameworks for the plugins. ' | ||
| 'Consider using "--no-plugins" if plugin frameworks are not required.', | ||
| ) | ||
| ..addFlag('static', |
There was a problem hiding this comment.
Passing both --static and --no-plugins should be an error, since that would be a mistake, and it's not clear what someone would expect it to do.
There was a problem hiding this comment.
Added this to validations
| globals.printStatus('Frameworks written to ${outputDirectory.path}.'); | ||
|
|
||
| if (!project.isModule && hasPlugins(project)) { | ||
| if (!project.isModule && boolArg('plugins') && hasPlugins(project)) { |
There was a problem hiding this comment.
Why is this being gated on the plugins flag? This step should be basically free (so wouldn't impact build times, which as I understand it is the driver for this PR), and it's not at all clear to me that it wouldn't be needed in the use cases described here. Isn't the intended outcome that the app still has and uses plugins, they are just built separately? Building all the plugins via some other mechanism doesn't remove the need for a registrant.
stuartmorgan-g
left a comment
stuartmorgan-g
left a comment
There was a problem hiding this comment.
LGTM. To @jmagman for a second review.
| await _testBuildFrameworksWithoutPlugins(projectDir, platform: 'macos'); | ||
|
|
||
| section('check --static cannot be used with the --no-plugins flag'); | ||
| await _testStaticAndNoPlugins(projectDir); |
There was a problem hiding this comment.
I think this is testing ios-frameworks only but you put it in the macOS test section. I can't remember if we allow the static flag for macOS (if we do I suspect it doesn't do anything?) I know these files are huge and a mess. 😞
There was a problem hiding this comment.
Good catch! _testStaticAndNoPlugins function run build ios-framework internally, that is why tests pass. --static is not supported for macos.
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
…n frameworks from the build (flutter/flutter#129739)
8 participants
A lot of details are written in the feature request: #114692.
tl;dr: Options B & C from the add2app iOS guide have a limitation (build error) in case the Flutter plugin and native iOS app have a shared dependency. We can use a workaround to avoid the issue, but in this case we don't need to build frameworks for plugins.
Closes #114692
Part of #130220
Pre-launch Checklist
///).If you need help, consider asking for advice on the #hackers-new channel on Discord.