Repository navigation
fix: forward --build-name and --build-number to desktop version.json - #190130
auto-submit[bot] merged 2 commits into
Conversation
On desktop, BuildInfo.toEnvironmentConfig() left out the build name and number, so tool_backend.dart never passed them on to flutter assemble. The generated flutter_assets/version.json kept the pubspec default instead of the value given to --build-name or --build-number. Emit BUILD_NAME and BUILD_NUMBER from toEnvironmentConfig() using the existing null-aware map entries, and forward them as -dBuildName and -dBuildNumber in tool_backend.dart next to the other optional defines. The keys are absent when the flags are not supplied, so default behavior is unchanged. Fixes flutter#152236
github-actions
Bot
added
the
tool
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request adds support for extracting and passing BUILD_NAME and BUILD_NUMBER environment variables to the backend tool, including them in the environment configuration map, and adds a unit test to verify this behavior. A critical compilation error was identified in build_info.dart where invalid Dart syntax is used to conditionally add these variables to the configuration map; using collection if elements is recommended instead.
| 'BUILD_NAME': ?buildName, | ||
| 'BUILD_NUMBER': ?buildNumber, |
There was a problem hiding this comment.
The syntax 'BUILD_NAME': ?buildName, is not valid Dart and will cause a compilation error. To conditionally include map entries only when their values are non-null, use Dart's collection if elements.
if (buildName != null) 'BUILD_NAME': buildName,
if (buildNumber != null) 'BUILD_NUMBER': buildNumber,There was a problem hiding this comment.
Yeah this one is actually fine, it is a null-aware map entry (Dart 3.9). The same ?value form is already used a few lines up for SPLIT_DEBUG_INFO, CODE_SIZE_DIRECTORY and FLAVOR, so I just matched that. Rewriting only these two as if (x != null) would make them inconsistent with the rest of the map. analyze is clean and the new test passes on it too.
chingjun
added
the
autosubmit
flutter-dashboard
Bot
removed
the
autosubmit
bkonyi
added
the
cp: stable
3 participants
On desktop,
flutter build linux --build-name=4.5.6produces aflutter_assets/version.jsonthat still shows the pubspec default (1.0.0) instead of the value passed on the command line. The same happens with--build-number. Thatversion.json(read bypackage_info_plus) is generated by the assemble target'sgetVersionInfo, which already honors theBuildNameandBuildNumberdefines. The break is upstream in the desktop build pipeline: those defines never reachflutter assemble.BuildInfo.toEnvironmentConfig(), which produces the environment map written into the generated CMake config, leaves the build name and number out, andtool_backend.dartdoes not forward them when it invokes assemble. Android and web are unaffected because they go throughtoBuildSystemEnvironment(), which already includes both values.This forwards the two values through the desktop path, following the same pattern already used for split-debug-info and tree-shake-icons.
toEnvironmentConfig()now emitsBUILD_NAMEandBUILD_NUMBERvia the existing null-aware map entries, andtool_backend.dartreads them and adds-dBuildNameand-dBuildNumberto the assemble invocation next to the other optional defines. Because the map entries are null-aware, the keys are simply absent when the flags are not supplied, so default behavior is unchanged. No change is needed inlinux.dart, which already applies these defines when they are present.Reproduced by the issue triager and reconfirmed by another user on 3.29.2. I added a
toEnvironmentConfigunit test inbuild_info_test.dartthat asserts the new keys appear when a build name and number are set. The existing encoding test keeps passing because null values stay omitted.Fixes #152236
Pre-launch Checklist
///).