[flutter_tools] support all engine debugging options with FLUTTER_ENGINE_SWITCH environment variables#67150
Conversation
flutter-dashboard
bot
added
tool
| <String>[ | ||
| executable, | ||
| ], | ||
| environment: _computeEnvironment(debuggingOptions, traceStartup, route), |
There was a problem hiding this comment.
Do these options work in release mode on other platforms? Since they won't currently on desktop, maybe we should see if this is non-empty and if the build mode is release, output a warning that the options will be ignored and to file an issue describing their use case.
There was a problem hiding this comment.
The tooling will already ignore the unsupported values when constructing the debugging options object:
That seems like a reasonable place to insert some logging to warn users if they are passing flags that have no effect.
| /// * FLUTTER_ENGINE_SWITCHES to the number of switches. | ||
| /// * FLUTTER_ENGINE_SWITCH_<N> (indexing from 1) to the individual switches. | ||
| Map<String, String> _computeEnvironment(DebuggingOptions debuggingOptions, bool traceStartup, String route) { | ||
| int flags = 1; |
There was a problem hiding this comment.
Wouldn't it be clearer to start this as zero, increment flags at the start of addFlag instead of the end, and not have to subtract in finish?
| } | ||
| if (!debuggingOptions.debuggingEnabled) { | ||
| finish(); | ||
| return environment; |
There was a problem hiding this comment.
Skimming, I almost missed that this was an early return instead of just another flag. Maybe wrap all the rest in if (debuggingOptions.debuggingEnabled) {...} instead of early return so it's clearer?
(As a concrete issue, I think there's a good chance that if I were ever to add anything to the flags here as currently written, I would accidentally add it to the end of the function not realizing that it wouldn't always get there.)
There was a problem hiding this comment.
Updated to only have a single return
3 participants
Description
Allow providing all debugging options to the desktop engine via the FLUTTER_ENGINE_SWITCH_ environment variables.
Fixes #66532
Fixes #46005
Fixes #58882
The underling engine changes have already landed for Windows, macOS, but linux is still in progress