Skip to content

Stop inheriting BuildCommand; register only the build subcommands we support - #52

Merged
DenisovAV merged 3 commits into
devfrom
refactor/build-command-decoupling
Aug 5, 2026
Merged

Stop inheriting BuildCommand; register only the build subcommands we support#52
DenisovAV merged 3 commits into
devfrom
refactor/build-command-decoupling

Conversation

@DenisovAV

Copy link
Copy Markdown
Contributor

Independent of #50 — different files, merges in either order.

flutter-tvos build currently offers thirteen subcommands, twelve of them upstream's: aar, apk, appbundle, ios, ios-framework, ipa, macos, macos-framework, swift-package, web. On a tvOS toolchain that is not just noise — flutter-tvos build ios would build an iOS app against an engine compiled for tvOS with the SDK pins swapped underneath it, and nobody has checked where that ends.

We got them by accident. TvosBuildCommand extends BuildCommand to obtain four trivial members — a name, a description, a category, a failing runCommand — and pays seventeen constructor parameters forwarded straight through, none of which it reads. They exist to construct the platform subcommands we did not want.

Why this is worth doing now

That forwarding is half the cost of supporting another Flutter version. Measured against a real flutter_tools 3.32.8 checkout:

errors
before 25build.dart 12, tvos_device.dart 8, build_targets/application.dart 4, executable.dart 1
after 13build.dart gone entirely

Upstream reshaped BuildCommand's dependency injection between 3.41 and 3.44, which is where those twelve come from. Composing instead of inheriting removes them permanently — that constructor can churn freely now and never reach us again.

This matters because each supported Flutter version needs its own release line of this CLI (see #50), so every one of those errors is a cost paid per version, not once.

build bundle is kept alongside build tvos: platform-neutral, used from CI for flutter_assets, nothing version-specific about it.

Result

$ flutter-tvos build --help
  bundle   Build the Flutter assets directory from your app.
  tvos     Build an Apple tvOS application.

The call site in executable.dart goes from twenty lines to one. 297 tests pass, analyzer clean.

This removes user-visible commands. They were never supported or tested here, and several were actively unsafe, but if anything in CI shells out to flutter-tvos build apk it will now fail rather than silently do something odd.

@MAUstaoglu MAUstaoglu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Right change, and the analyzer-error count is what makes it worth doing now rather than at the next upgrade.

Three things.

The description says this is independent of #50 — different files. It isn't. Both edit lib/executable.dart, and #50 moves the exact TvosBuildCommand(...) block this one rewrites into the new tvosCommands(). #50 is approved and goes first, so this needs a rebase on top of it. GitHub only calls it mergeable because #50 hasn't landed yet.

Nothing locks the result in. You made this argument yourself in #50 — the absences are the load-bearing half — and it applies here more than it did there: if a Flutter upgrade puts extends BuildCommand back, all 297 tests still pass and build apk is quietly on offer again. expect(TvosBuildCommand(...).subcommands.keys, {'tvos', 'bundle'}) is a few lines and is the only thing that stops a merge from undoing this.

The reason given for keeping build bundle doesn't hold up. BundleBuilder builds globals.buildTargets.copyFlutterBundle, and executable.dart registers upstream BuildTargetsImpl — so it runs upstream CopyFlutterBundle → KernelSnapshot → DartPluginRegistrantTarget, the exact chain TvosCopyFlutterBundle and TvosKernelSnapshot exist to replace. A bundle built that way won't register *_tvos plugins, and --target-platform has no tvos in its allowed list and defaults to android-arm. Not something this PR broke, but it isn't the platform-neutral survivor either. What actually uses it from CI? If nothing does, drop it too and we're down to one subcommand.

Smaller: the CHANGELOG has an [Unreleased] section and this removes user-visible commands, so it wants a ### Removed line naming them. And doc/architecture.md:91 still says TvosBuildCommand.runCommand() creates the build info — already wrong before, reads worse now that it is an explicit fail().

On sequencing: #48, #50, then this, going out together as 1.5.0 — new commands plus a removal is not a patch release. Engine #11 is a different Flutter line and ships on its own.

@DenisovAV
DenisovAV force-pushed the refactor/build-command-decoupling branch from 2cc3cd7 to 9a15260 Compare August 3, 2026 11:53
@DenisovAV

Copy link
Copy Markdown
Contributor Author

All three were right, thanks — pushed.

Rebased on dev. The "independent of #50" line in the description was wrong and I should have checked rather than asserted it: both edit executable.dart, and #50 moved the very TvosBuildCommand(...) call this rewrites into the new tvosCommands(). It only looked mergeable because #50 had not landed.

build bundle is gone too. You were right that the justification does not survive contact with the code — I traced it: BundleBuilder resolves globals.buildTargets.copyFlutterBundle, executable.dart registers upstream BuildTargetsImpl, so it runs upstream CopyFlutterBundle rather than TvosCopyFlutterBundle, and --target-platform has no tvos value, defaulting to android-arm. Nothing in .github/ or bin/ invokes it. A command that silently returns the wrong answer is worse than one that is absent, so it goes with the rest. flutter-tvos build now offers exactly tvos.

Locked in. Fair hit — I made that argument in #50 and then did not apply it here. test/general/tvos_build_command_test.dart asserts the subcommand list directly, including that none of the twelve upstream ones are present.

CHANGELOG has a ### Removed entry naming the build subcommands, and it covers channel and downgrade from #50 too — they were user-visible removals with no entry either.

doc/architecture.md:91 now names BuildTvosCommand.runCommand() and notes that TvosBuildCommand is only the umbrella.

341 tests pass, analyzer at dev's baseline. Agreed on sequencing — #48, #50, this, out together as 1.5.0.

@MAUstaoglu MAUstaoglu left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

All three closed, and dropping bundle rather than defending it was the right call — the reasoning being in the test as well as the comment is what will keep it dropped.

One thing left, a one-liner: the command description is still 'Build a tvOS app or install bundle.'. There is no bundle any more, so flutter-tvos build --help advertises something that is not there. 'Build a tvOS app.' covers it.

Minor, while you are in the file: the class comment says twelve commands we should not offer, but linux and windows are gated by upstream's supported and never registered on macOS in the first place — the CHANGELOG's eleven is the accurate count.

Also, thanks for folding the channel and downgrade removals into the CHANGELOG. That gap was mine to catch in #50 and I did not.

LGTM.

@DenisovAV

Copy link
Copy Markdown
Contributor Author

Both fixed, pushed.

Description is now 'Build a tvOS app.'.

On the count — you were right, and I checked it against the pre-change help output rather than the constructor this time. Eleven appear on macOS: aar, apk, appbundle, bundle, ios, ios-framework, ipa, macos, macos-framework, swift-package, web. linux and windows are constructed but never reach the help, exactly as you said. Corrected in the class comment and in the test's enumeration; the CHANGELOG already said eleven and was right.

On the CHANGELOG gap in #50 — no need, channel and downgrade went out in that PR on my side too and I did not write the entry either. It only surfaced here because this PR made me look at the file.

341 tests, analyzer at baseline.

@DenisovAV
DenisovAV force-pushed the refactor/build-command-decoupling branch from 4ad122a to 66e9589 Compare August 5, 2026 16:11
`flutter-tvos build` offered thirteen subcommands, twelve of them upstream's:
aar, apk, appbundle, ios, ios-framework, ipa, macos, macos-framework,
swift-package, web and more. That is not merely noise on a tvOS toolchain --
`flutter-tvos build ios` would have built an iOS app against an engine
compiled for tvOS, with the SDK pins swapped underneath it, and nobody has
ever checked where that ends.

We inherited them by accident. TvosBuildCommand extended BuildCommand to get
four trivial members -- a name, a description, a category and a failing
runCommand -- and paid seventeen constructor parameters forwarded straight
through, none of which it read. They exist to construct the platform
subcommands we did not want.

That forwarding was also half the cost of supporting another Flutter
version: twelve of the twenty-five analyzer errors this CLI produces against
flutter_tools 3.32.8 came from this one constructor, because upstream
reshaped BuildCommand's dependency injection between 3.41 and 3.44.
Composing instead of inheriting takes it from 25 errors to 13, permanently
-- upstream can now churn that constructor as freely as it likes.

`build bundle` is kept alongside `build tvos`: it is platform-neutral, used
from CI to produce flutter_assets, and costs nothing version-specific.

The call site in executable.dart goes from twenty lines to one.
@MAUstaoglu was right on all three.

`build bundle` is not the platform-neutral survivor I claimed. BundleBuilder
resolves globals.buildTargets.copyFlutterBundle, and executable.dart
registers upstream's BuildTargetsImpl -- so it runs upstream's
CopyFlutterBundle, KernelSnapshot and DartPluginRegistrantTarget, which are
exactly what TvosCopyFlutterBundle, TvosKernelSnapshot and
TvosDartPluginRegistrantTarget exist to replace. The bundle it produces has
no `*_tvos` plugin registration, and --target-platform has no tvos value at
all, defaulting to android-arm. Nothing in .github/ or bin/ invokes it. A
command that silently returns the wrong answer is worse than one that is
absent, so it goes with the rest.

Nothing locked the result in. I made that argument in #50 -- the absences
are the load-bearing half -- and then did not apply it here. A Flutter
upgrade reinstating `extends BuildCommand` would leave every test passing
and put `build apk` quietly back on offer. tvos_build_command_test now
asserts the subcommand list directly.

Rebased onto dev. The claim that this was independent of #50 was wrong:
both edit executable.dart, and #50 moved the very TvosBuildCommand call this
rewrites into the new tvosCommands(). It only looked mergeable because #50
had not landed yet.

Also: a CHANGELOG "Removed" entry naming the commands that go, covering the
channel and downgrade removals from #50 as well; and doc/architecture.md
said TvosBuildCommand.runCommand() builds the TvosBuildInfo, which was
already wrong and reads worse now that it is an explicit fail().
The command description still read 'Build a tvOS app or install bundle.'
after bundle was removed, so `flutter-tvos build --help` advertised
something that is not there.

And the count was wrong: the class comment and the test both said twelve
upstream subcommands and listed linux and windows among them. Checked
against the pre-change help output rather than the constructor -- eleven
appear on macOS (aar, apk, appbundle, bundle, ios, ios-framework, ipa,
macos, macos-framework, swift-package, web); linux and windows are in
upstream's constructor but never reach the help. The CHANGELOG already said
eleven and was right.
@DenisovAV
DenisovAV force-pushed the refactor/build-command-decoupling branch from 66e9589 to f787209 Compare August 5, 2026 17:06
@DenisovAV
DenisovAV merged commit 3e135ec into dev Aug 5, 2026
2 checks passed
MAUstaoglu added a commit that referenced this pull request Aug 5, 2026
Adds `flutter-tvos versions` and `flutter-tvos use <version>` for switching
the checkout between Flutter versions (#50), stops `build` inheriting
upstream's platform subcommands (#52), fixes `create .` naming the project
"." (#54), and pins the engine to v1.0.2-flutter3.44.8 (#48).

`upgrade` now moves the checkout with `git checkout --force --detach`
instead of `git reset --hard`, which had silently discarded unpushed
commits.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants