Skip to content

Conversation

@Mr-Pepe
Copy link
Contributor

@Mr-Pepe Mr-Pepe commented Sep 24, 2025

Resolves #5797

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 [Flutter Style Guide], including [Features we expect every widget to implement].
  • I signed the [CLA].
  • I listed at least one issue that this PR fixes in the description above.
  • I updated/added relevant documentation (doc comments with ///).
  • I added new tests to check the change I am making, or this PR is [test-exempt].
  • I followed the [breaking change policy] and added [Data Driven Fixes] where supported.
  • All existing and new tests are passing.

A whole bunch of tool tests failed locally even before the change, so I'll have to see CI results to know if something actually broke due to the change.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 24, 2025
@Mr-Pepe Mr-Pepe force-pushed the 09-23-exit_with_code_1_when_calling_flutter_build_without_arguments branch from bf5bf12 to 8bf090b Compare September 24, 2025 06:49
@Mr-Pepe Mr-Pepe changed the title Exit with code 1 when calling flutter build without arguments. Exit with code 1 when calling flutter build without arguments Sep 24, 2025
Copy link
Contributor

@gemini-code-assist gemini-code-assist bot left a comment

Choose a reason for hiding this comment

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

Code Review

This pull request correctly implements the desired behavior for flutter build to exit with code 1 when called without arguments. It achieves this by modifying the command runner to show the help message and then force a non-zero exit code. The addition of a new test case ensures this functionality is verified. I have one suggestion to improve the code's readability by refactoring to use async/await.

@Mr-Pepe
Copy link
Contributor Author

Mr-Pepe commented Sep 27, 2025

No idea why Google testing is hanging but otherwise looking good from my side

@Mr-Pepe
Copy link
Contributor Author

Mr-Pepe commented Oct 7, 2025

@bkonyi You labeled the referenced issue as still valid. Are you interested in reviewing this PR?

return super.run(args).then((_) {
if (exitWithCodeOne) {
// No need to print anything because the help was already printed.
throwToolExit('', exitCode: 1);
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we just call exitWithHooks(1, shutdownHooks: ...) instead? I'm pretty sure this will result in Error: being printed.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't see Error: being printed, only the help:

❯ ../../bin/flutter build
Building flutter tool...
Resolving dependencies... 
Downloading packages... 
Got dependencies.
Build an executable app or install bundle.

Global options:
-h, --help                  Print this usage information.
-v, --verbose               Noisy logging, including all shell commands executed.
                            If used with "--help", shows hidden options. If used with "flutter doctor", shows
                            additional diagnostic information. (Use "-vv" to force verbose logging in those
                            cases.)
-d, --device-id             Target device id or name (prefixes allowed).
    --version               Reports the version of this tool.
    --enable-analytics      Enable telemetry reporting each time a flutter or dart command runs.
    --disable-analytics     Disable telemetry reporting each time a flutter or dart command runs, until it is
                            re-enabled.
    --suppress-analytics    Suppress analytics reporting for the current CLI invocation.

Usage: flutter build <subcommand> [arguments]
-h, --help    Print this usage information.

Available subcommands:
  aar         Build a repository containing an AAR and a POM file.
  apk         Build an Android APK file from your app.
  appbundle   Build an Android App Bundle file from your app.
  bundle      Build the Flutter assets directory from your app.
  linux       Build a Linux desktop application.
  web         Build a web application bundle.

Run "flutter help" to see global options.

❯ echo $status
1

Copy link
Contributor

Choose a reason for hiding this comment

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

Yeah, I still think this would be better if we just call exitWithHooks(1, shutdownHooks: ...) instead. throwToolExit is meant for situations where we provide actionable feedback to users, but we're just using it to get to the root exception handler here to call exitWithHooks(...). We may as well just do it here.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done 👍

@bkonyi bkonyi added the waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds label Nov 10, 2025
@Mr-Pepe Mr-Pepe force-pushed the 09-23-exit_with_code_1_when_calling_flutter_build_without_arguments branch from 3c6b17e to 66abe05 Compare December 7, 2025 14:42
@Mr-Pepe
Copy link
Contributor Author

Mr-Pepe commented Dec 7, 2025

Updated the PR to use exitWithHooks

Copy link
Contributor

@bkonyi bkonyi left a comment

Choose a reason for hiding this comment

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

Looks good to me! We'll just need one more reviewer to land this (@chingjun can you please take a look? 😄)

@bkonyi bkonyi requested a review from chingjun December 11, 2025 16:06
@bkonyi bkonyi added the autosubmit Merge PR when tree becomes green via auto submit App label Dec 11, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Dec 12, 2025
Merged via the queue into flutter:master with commit 4de6fcc Dec 12, 2025
141 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Dec 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Dec 12, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Dec 12, 2025
flutter/flutter@e25d71b...6e1aa82

2025-12-12 [email protected] Add documentation for what to do when bumping the minimum sdk version supported by flutter (flutter/flutter#179795)
2025-12-12 [email protected] Roll Skia from ee08571bbf67 to e66816c3645e (2 revisions) (flutter/flutter#179792)
2025-12-12 [email protected] Roll Packages from 36383d6 to 0ac7a03 (1 revision) (flutter/flutter#179793)
2025-12-12 [email protected] Roll Fuchsia Linux SDK from QssSL8DkxIbMvf89C... to fppT9ZrwbFx7iYrIh... (flutter/flutter#179785)
2025-12-12 [email protected] Roll Skia from 4251aa7454da to ee08571bbf67 (1 revision) (flutter/flutter#179783)
2025-12-12 [email protected] Roll Skia from 344ff9c41fb6 to 4251aa7454da (2 revisions) (flutter/flutter#179779)
2025-12-12 [email protected] Fix draggable scrollable sheet example drag speed is off  (flutter/flutter#179179)
2025-12-12 [email protected] [ios][pv] accept/reject gesture based on hitTest (with new widget API) (flutter/flutter#179659)
2025-12-12 [email protected] Roll Skia from 7ad91f64b468 to 344ff9c41fb6 (1 revision) (flutter/flutter#179774)
2025-12-12 [email protected] [fuchsia] temporarily disable touch-input-test due to the flakiness (flutter/flutter#179747)
2025-12-12 [email protected] Roll Skia from 979b7ae09145 to 7ad91f64b468 (6 revisions) (flutter/flutter#179770)
2025-12-12 [email protected] Exit with code 1 when calling `flutter build` without arguments (flutter/flutter#175900)
2025-12-12 [email protected] [ Tool ] Assemble: "Improper" validation error when dealing with base64 encoded dart-define value  (flutter/flutter#178737)
2025-12-12 98614782+auto-submit[bot]@users.noreply.github.com Reverts "[ Analysis ] Added initial implementation of the `flutter_analyzer_plugin` (#175679)" (flutter/flutter#179766)
2025-12-11 [email protected] Add 3.38.5 changelog (flutter/flutter#179750)
2025-12-11 [email protected] Roll Dart SDK from 3d4b989bdc41 to 9a65db770758 (2 revisions) (flutter/flutter#179746)
2025-12-11 [email protected] fix: fresh install of windows support (flutter/flutter#179720)
2025-12-11 [email protected] [ Analysis ] Added initial implementation of the `flutter_analyzer_plugin` (flutter/flutter#175679)
2025-12-11 [email protected] Roll Skia from d570e2317110 to 979b7ae09145 (4 revisions) (flutter/flutter#179742)
2025-12-11 [email protected] chore: linux fuchsia tests are flaking (flutter/flutter#179744)
2025-12-11 [email protected] Roll Packages from 74a5a53 to 36383d6 (4 revisions) (flutter/flutter#179740)
2025-12-11 [email protected] Revert "Android implementation of content sizing" (flutter/flutter#179698)
2025-12-11 [email protected] [skia] Disable legacy image decoders (flutter/flutter#179277)
2025-12-11 [email protected] Roll Skia from deb0153719dd to d570e2317110 (2 revisions) (flutter/flutter#179734)
2025-12-11 [email protected] Roll Dart SDK from dbcb567e2432 to 3d4b989bdc41 (1 revision) (flutter/flutter#179733)
2025-12-11 [email protected] Roll Fuchsia Linux SDK from u5vxWTRT0HlxOP5_r... to QssSL8DkxIbMvf89C... (flutter/flutter#179729)
2025-12-11 [email protected] Roll Skia from 59c6cad539f7 to deb0153719dd (2 revisions) (flutter/flutter#179721)
2025-12-11 [email protected] Roll Dart SDK from 077062c5e515 to dbcb567e2432 (2 revisions) (flutter/flutter#179715)
2025-12-11 [email protected] Use kPreventOverdraw for arcs with overlapping stroke caps (flutter/flutter#179312)
2025-12-11 [email protected] Roll Skia from e61cc6d073fd to 59c6cad539f7 (22 revisions) (flutter/flutter#179714)
2025-12-10 [email protected] Reland: Add framework-side hitTestBehavior support to Semantics (#178817) (flutter/flutter#179352)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-packages
Please CC [email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Packages: https://github.com/flutter/flutter/issues/new/choose

To report a problem with the AutoRoller itself, please file a bug:
https://issues.skia.org/issues/new?component=1389291&template=1850622

Documentation for the AutoRoller is here:
https://skia.googlesource.com/buildbot/+doc/main/autoroll/README.md
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

tool Affects the "flutter" command-line tool. See also t: labels. waiting for customer response The Flutter team cannot make further progress on this issue until the original reporter responds

Projects

None yet

Development

Successfully merging this pull request may close these issues.

flutter build is returning zero, should return not zero ?

3 participants