Fix incorrectly checking for invalid environment variables in the tool #164101

ueman · 2025-02-25T14:28:23Z

This PR removes a check that looks for the use of Flutter version related Dart define keys in the environment variables (not Dart defines) of the user of the Flutter tool. We don't need to check the environment variables, we only need to check the user defined Dart defines.

Here's how it currently works:

User builds a Flutter app via tool
tool checks environment variables for Flutter version related Dart define keys and throws if it finds and (and)
tool checks list of user defined Dart defines for Flutter version related Dart define keys and throws if it finds any

This PR removes the check in step 2, since step 3 is what we actually care about.

The FLUTTER_GIT_URL environment variable is set on forks and the Dart define is used to make that information available to applications at runtime. However, environment variables don't propagate to be Dart defines, thus the above mentioned check is not needed.

List which issues are fixed by this PR. You must list at least one issue. An issue is not required if the PR fixes something trivial like a typo.

Fixes #164093

If you had to change anything in the flutter/tests repo, include a link to the migration guide as per the breaking change policy.

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.

If you need help, consider asking for advice on the #hackers-new channel on Discord.

TahaTesser

I can confirm this fixes the regression.

matanlurey

Please update the PR description; it's not clear to me why removing checks for Dart defines fixes FLUTTER_GIT_URL?

ueman · 2025-02-25T19:12:07Z

I've updated the PR description

TahaTesser

LGTM!

TahaTesser · 2025-02-26T14:05:18Z

Typically, a new test would be added to prevent a regression in the future. In this case, a test to verify the tool doesn't exit when using a custom FLUTTER_GIT_URL is needed. Otherwise, you'd have to request a test exemption. I'd advise adding a regression test.

ueman · 2025-02-26T21:02:55Z

What would be a reasonable test for that? I'm asking since I'm guessing you want a test with a least amount of mocks/fakes, but that makes the test harder to write.

Is there by chance a similar test already somewhere?

TahaTesser · 2025-02-27T08:08:14Z

What would be a reasonable test for that? I'm asking since I'm guessing you want a test with a least amount of mocks/fakes, but that makes the test harder to write.

Is there by chance a similar test already somewhere?

You can copy this test for FLUTTER_GIT_URL and update it for your fix.

flutter/packages/flutter_tools/test/general.shard/runner/flutter_command_test.dart

Lines 1395 to 1412 in f1f463c

    
           testUsingContext( 
        
             'FLUTTER_GIT_URL is set in dartDefines', 
        
             () async { 
        
               final DummyFlutterCommand flutterCommand = DummyFlutterCommand(packagesPath: 'foo'); 
        
               final BuildInfo buildInfo = await flutterCommand.getBuildInfo( 
        
                 forcedBuildMode: BuildMode.debug, 
        
               ); 
        
               expect( 
        
                 buildInfo.dartDefines, 
        
                 contains('FLUTTER_GIT_URL=https://github.com/flutter/flutter.git'), 
        
               ); 
        
             }, 
        
             overrides: <Type, Generator>{ 
        
               ProcessManager: () => FakeProcessManager.any(), 
        
               FlutterVersion: () => FakeFlutterVersion(), 
        
             }, 
        
           );

TahaTesser

LGTM again! Thanks for adding the test.

… the tool (flutter/flutter#164101)

Roll Flutter from 2e570ca to 842db35 (59 revisions) flutter/flutter@2e570ca...842db35 2025-03-02 [email protected] Roll Skia from ad64415050aa to 101eee8fce59 (1 revision) (flutter/flutter#164449) 2025-03-02 [email protected] Roll Fuchsia Linux SDK from ln3joxJfRN2XGhvCv... to AO1KirSDI7-MVYNPN... (flutter/flutter#164440) 2025-03-02 [email protected] android: Clean up gen_snapshot artifact build (flutter/flutter#164418) 2025-03-02 [email protected] Start using `bin/cache/engine.{stamp|realm}` instead of `bin/internal/engine.{realm|version}`. (flutter/flutter#164352) 2025-03-01 [email protected] Add macos/android_debug_unopt to local_engine.json (flutter/flutter#164410) 2025-03-01 [email protected] Delete unused build archive targets (flutter/flutter#164414) 2025-03-01 [email protected] Roll Fuchsia Linux SDK from QMun2itYrV_zUYrvW... to ln3joxJfRN2XGhvCv... (flutter/flutter#164423) 2025-03-01 [email protected] Roll Skia from ac14158663ea to ad64415050aa (1 revision) (flutter/flutter#164413) 2025-03-01 [email protected] Update linux_host_engine.json ci/host_release description (flutter/flutter#164402) 2025-03-01 [email protected] In update_engine_version_test.dart, do not populate the test environment with the host platform environment (flutter/flutter#164395) 2025-03-01 [email protected] Roll-forward #164317: Use `bin/cache/engine.stamp` (flutter/flutter#164401) 2025-02-28 [email protected] Make pressing and moving on CupertinoButton closer to native behavior. (flutter/flutter#161731) 2025-02-28 [email protected] Roll Skia from 4005ba3ca7b6 to ac14158663ea (7 revisions) (flutter/flutter#164404) 2025-02-28 [email protected] [macOS] Prepare FlutterKeyboardManager for multi-view (flutter/flutter#163962) 2025-02-28 [email protected] Add PlatformDispatcher.engineId (flutter/flutter#163476) 2025-02-28 [email protected] Move `integration_test.FlutterDeviceScreenshotTest` to the framework slow shard (flutter/flutter#164398) 2025-02-28 [email protected] Fix: Update DelegatedTransition animation parameter correctly (flutter/flutter#163853) 2025-02-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Write an identical value to `bin/cache/engine.stamp` to prepare for migration (#164317)" (flutter/flutter#164396) 2025-02-28 [email protected] Add action for configuring default action of EditableText.onTapUpOutside (flutter/flutter#162575) 2025-02-28 [email protected] Align jvmTarget usages across codebase, while editing build.gradle files align them with android version documentation (flutter/flutter#164200) 2025-02-28 [email protected] Roll Packages from 01d3d5c to 70b41e1 (5 revisions) (flutter/flutter#164380) 2025-02-28 [email protected] [tool] Allow using archiveName in android bundle build (flutter/flutter#162390) 2025-02-28 [email protected] Fix incorrectly checking for invalid environment variables in the tool (flutter/flutter#164101) 2025-02-28 [email protected] Support forward and back buttons (flutter/flutter#164356) 2025-02-28 [email protected] Fix window creation callback for multi-window (flutter/flutter#164353) 2025-02-28 [email protected] Fix flutter doctor usage of eglinfo in failure cases. (flutter/flutter#164334) 2025-02-28 [email protected] Refactor writing of package config in tests (flutter/flutter#163734) 2025-02-28 [email protected] Fixed the issue that Slider's secondaryTrackValue is not updated. (flutter/flutter#163996) 2025-02-28 [email protected] Roll pub packages (flutter/flutter#164357) 2025-02-28 [email protected] Remove `Mac mac_unopt` presubmit retry count (flutter/flutter#164350) 2025-02-28 [email protected] Roll Fuchsia Linux SDK from 1elkOxihZuTEiTXzY... to QMun2itYrV_zUYrvW... (flutter/flutter#164351) 2025-02-28 [email protected] Drag handles only need to be tested on mobile platforms. (flutter/flutter#163723) 2025-02-28 [email protected] [Android] Use java for looking up Android API level. (flutter/flutter#163558) 2025-02-28 [email protected] Write an identical value to `bin/cache/engine.stamp` to prepare for migration (flutter/flutter#164317) 2025-02-27 [email protected] Wires up expanded state in web engine (flutter/flutter#164048) 2025-02-27 [email protected] Remove Cheserton's File (flutter/flutter#164340) 2025-02-27 [email protected] Replace update semantics handler with signal. (flutter/flutter#163583) 2025-02-27 [email protected] Split up the conical gradient fragment shader (flutter/flutter#164058) 2025-02-27 [email protected] Roll Skia from c16b145749d4 to 4005ba3ca7b6 (3 revisions) (flutter/flutter#164339) 2025-02-27 [email protected] Delete and update stale documentation regarding engine/engine hash. (flutter/flutter#164324) 2025-02-27 [email protected] Document how `engine.version` (is/will be) computed (flutter/flutter#164335) 2025-02-27 [email protected] Update conductor to write engine.version file (flutter/flutter#163350) 2025-02-27 [email protected] remove last usages of min/compile/target SdkVersion, align sourceCompatibility across repo and update android version documentation (flutter/flutter#164198) 2025-02-27 [email protected] Update links to the `flutter/engine` repository for the monorepo. (flutter/flutter#164328) 2025-02-27 [email protected] Add empty `io.flutter.app.FlutterApplication` to give deprecation notice, and un-break projects that have not migrated (flutter/flutter#164233) 2025-02-27 [email protected] Revert dart sdks that were causing dartaotruntime issues in g3 (flutter/flutter#164307) ...

… the tool (flutter/flutter#164101)

Roll Flutter from 2e570ca to 842db35 (59 revisions) flutter/flutter@2e570ca...842db35 2025-03-02 [email protected] Roll Skia from ad64415050aa to 101eee8fce59 (1 revision) (flutter/flutter#164449) 2025-03-02 [email protected] Roll Fuchsia Linux SDK from ln3joxJfRN2XGhvCv... to AO1KirSDI7-MVYNPN... (flutter/flutter#164440) 2025-03-02 [email protected] android: Clean up gen_snapshot artifact build (flutter/flutter#164418) 2025-03-02 [email protected] Start using `bin/cache/engine.{stamp|realm}` instead of `bin/internal/engine.{realm|version}`. (flutter/flutter#164352) 2025-03-01 [email protected] Add macos/android_debug_unopt to local_engine.json (flutter/flutter#164410) 2025-03-01 [email protected] Delete unused build archive targets (flutter/flutter#164414) 2025-03-01 [email protected] Roll Fuchsia Linux SDK from QMun2itYrV_zUYrvW... to ln3joxJfRN2XGhvCv... (flutter/flutter#164423) 2025-03-01 [email protected] Roll Skia from ac14158663ea to ad64415050aa (1 revision) (flutter/flutter#164413) 2025-03-01 [email protected] Update linux_host_engine.json ci/host_release description (flutter/flutter#164402) 2025-03-01 [email protected] In update_engine_version_test.dart, do not populate the test environment with the host platform environment (flutter/flutter#164395) 2025-03-01 [email protected] Roll-forward #164317: Use `bin/cache/engine.stamp` (flutter/flutter#164401) 2025-02-28 [email protected] Make pressing and moving on CupertinoButton closer to native behavior. (flutter/flutter#161731) 2025-02-28 [email protected] Roll Skia from 4005ba3ca7b6 to ac14158663ea (7 revisions) (flutter/flutter#164404) 2025-02-28 [email protected] [macOS] Prepare FlutterKeyboardManager for multi-view (flutter/flutter#163962) 2025-02-28 [email protected] Add PlatformDispatcher.engineId (flutter/flutter#163476) 2025-02-28 [email protected] Move `integration_test.FlutterDeviceScreenshotTest` to the framework slow shard (flutter/flutter#164398) 2025-02-28 [email protected] Fix: Update DelegatedTransition animation parameter correctly (flutter/flutter#163853) 2025-02-28 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Write an identical value to `bin/cache/engine.stamp` to prepare for migration (#164317)" (flutter/flutter#164396) 2025-02-28 [email protected] Add action for configuring default action of EditableText.onTapUpOutside (flutter/flutter#162575) 2025-02-28 [email protected] Align jvmTarget usages across codebase, while editing build.gradle files align them with android version documentation (flutter/flutter#164200) 2025-02-28 [email protected] Roll Packages from 01d3d5c to 70b41e1 (5 revisions) (flutter/flutter#164380) 2025-02-28 [email protected] [tool] Allow using archiveName in android bundle build (flutter/flutter#162390) 2025-02-28 [email protected] Fix incorrectly checking for invalid environment variables in the tool (flutter/flutter#164101) 2025-02-28 [email protected] Support forward and back buttons (flutter/flutter#164356) 2025-02-28 [email protected] Fix window creation callback for multi-window (flutter/flutter#164353) 2025-02-28 [email protected] Fix flutter doctor usage of eglinfo in failure cases. (flutter/flutter#164334) 2025-02-28 [email protected] Refactor writing of package config in tests (flutter/flutter#163734) 2025-02-28 [email protected] Fixed the issue that Slider's secondaryTrackValue is not updated. (flutter/flutter#163996) 2025-02-28 [email protected] Roll pub packages (flutter/flutter#164357) 2025-02-28 [email protected] Remove `Mac mac_unopt` presubmit retry count (flutter/flutter#164350) 2025-02-28 [email protected] Roll Fuchsia Linux SDK from 1elkOxihZuTEiTXzY... to QMun2itYrV_zUYrvW... (flutter/flutter#164351) 2025-02-28 [email protected] Drag handles only need to be tested on mobile platforms. (flutter/flutter#163723) 2025-02-28 [email protected] [Android] Use java for looking up Android API level. (flutter/flutter#163558) 2025-02-28 [email protected] Write an identical value to `bin/cache/engine.stamp` to prepare for migration (flutter/flutter#164317) 2025-02-27 [email protected] Wires up expanded state in web engine (flutter/flutter#164048) 2025-02-27 [email protected] Remove Cheserton's File (flutter/flutter#164340) 2025-02-27 [email protected] Replace update semantics handler with signal. (flutter/flutter#163583) 2025-02-27 [email protected] Split up the conical gradient fragment shader (flutter/flutter#164058) 2025-02-27 [email protected] Roll Skia from c16b145749d4 to 4005ba3ca7b6 (3 revisions) (flutter/flutter#164339) 2025-02-27 [email protected] Delete and update stale documentation regarding engine/engine hash. (flutter/flutter#164324) 2025-02-27 [email protected] Document how `engine.version` (is/will be) computed (flutter/flutter#164335) 2025-02-27 [email protected] Update conductor to write engine.version file (flutter/flutter#163350) 2025-02-27 [email protected] remove last usages of min/compile/target SdkVersion, align sourceCompatibility across repo and update android version documentation (flutter/flutter#164198) 2025-02-27 [email protected] Update links to the `flutter/engine` repository for the monorepo. (flutter/flutter#164328) 2025-02-27 [email protected] Add empty `io.flutter.app.FlutterApplication` to give deprecation notice, and un-break projects that have not migrated (flutter/flutter#164233) 2025-02-27 [email protected] Revert dart sdks that were causing dartaotruntime issues in g3 (flutter/flutter#164307) ...

Fix flutter#164093

a36f1dc

github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Feb 25, 2025

ueman mentioned this pull request Feb 25, 2025

App fails to run with custom FLUTTER_GIT_URL environment variable on master #164093

Closed

TahaTesser reviewed Feb 25, 2025

View reviewed changes

TahaTesser requested a review from matanlurey February 25, 2025 14:43

matanlurey requested changes Feb 25, 2025

View reviewed changes

matanlurey approved these changes Feb 25, 2025

View reviewed changes

TahaTesser approved these changes Feb 26, 2025

View reviewed changes

add regression test

463ef5d

TahaTesser approved these changes Feb 28, 2025

View reviewed changes

TahaTesser added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 28, 2025

auto-submit bot added this pull request to the merge queue Feb 28, 2025

Merged via the queue into flutter:master with commit 556ae54 Feb 28, 2025
140 checks passed

flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Feb 28, 2025

engine-flutter-autoroll mentioned this pull request Feb 28, 2025

Roll Flutter from 2e570ca393c8 to b8902e6a1746 (38 revisions) flutter/packages#8759

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 28, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

34d4123

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Feb 28, 2025

Roll Flutter from 2e570ca393c8 to 787adc75ed35 (39 revisions) flutter/packages#8762

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 28, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

dcc2aa1

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Feb 28, 2025

Roll Flutter from 2e570ca393c8 to 845c7779b802 (40 revisions) flutter/packages#8763

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Feb 28, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

7ee463d

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Mar 1, 2025

Roll Flutter from 2e570ca393c8 to 0c055f2c508a (55 revisions) flutter/packages#8767

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 1, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

757516e

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Mar 1, 2025

Roll Flutter from 2e570ca393c8 to 0c055f2c508a (55 revisions) flutter/packages#8768

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 1, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

04ffb63

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 619d4248114d (58 revisions) flutter/packages#8769

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

041a140

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 842db35d27b8 (59 revisions) flutter/packages#8770

Closed

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

5840bf0

… the tool (flutter/flutter#164101)

engine-flutter-autoroll mentioned this pull request Mar 2, 2025

Roll Flutter from 2e570ca393c8 to 842db35d27b8 (59 revisions) flutter/packages#8771

Merged

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Mar 2, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

56594b2

… the tool (flutter/flutter#164101)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

68facc5

… the tool (flutter/flutter#164101)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 20, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

3b68ef1

… the tool (flutter/flutter#164101)

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request May 21, 2025

556ae54 Fix incorrectly checking for invalid environment variables in…

1177899

… the tool (flutter/flutter#164101)

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Fix incorrectly checking for invalid environment variables in the tool #164101

Fix incorrectly checking for invalid environment variables in the tool #164101

Uh oh!

ueman commented Feb 25, 2025 •

edited

Loading

Uh oh!

TahaTesser left a comment

Uh oh!

matanlurey left a comment •

edited

Loading

Uh oh!

ueman commented Feb 25, 2025

Uh oh!

TahaTesser left a comment

Uh oh!

TahaTesser commented Feb 26, 2025

Uh oh!

ueman commented Feb 26, 2025

Uh oh!

TahaTesser commented Feb 27, 2025 •

edited

Loading

Uh oh!

TahaTesser left a comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

Fix incorrectly checking for invalid environment variables in the tool #164101

Fix incorrectly checking for invalid environment variables in the tool #164101

Uh oh!

Conversation

ueman commented Feb 25, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Pre-launch Checklist

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

matanlurey left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

ueman commented Feb 25, 2025

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

TahaTesser commented Feb 26, 2025

Uh oh!

ueman commented Feb 26, 2025

Uh oh!

TahaTesser commented Feb 27, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

TahaTesser left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

ueman commented Feb 25, 2025 •

edited

Loading

matanlurey left a comment •

edited

Loading

TahaTesser commented Feb 27, 2025 •

edited

Loading