Skip to content

Serve command still works with --machine flag #8638

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 2 commits into from
Dec 17, 2024
Merged

Serve command still works with --machine flag #8638

Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
c98483b
Serve command still works with --machine
elliette Dec 16, 2024
1d255fd
Add exception
elliette Dec 17, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
54 changes: 38 additions & 16 deletions tool/lib/commands/serve.dart
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -157,6 +157,17 @@ class ServeCommand extends Command {
final devToolsAppBuildMode =
results[SharedCommandArgs.buildMode.flagName] as String;
final serveWithDartSdk = results[_serveWithDartSdkFlag] as String?;
final forMachine = results[_machineFlag] as bool;

// TODO(https://github.com/flutter/devtools/issues/8643): Support running in
// machine mode with a debuggable DevTools app.
if (runApp && forMachine) {
throw Exception(
'Machine mode is not supported with `flutter run` DevTools.\n'
'Please use either --machine or --run-app, not both.\n'
'See https://github.com/flutter/devtools/issues/8643 for details.',
);
}

// Any flag that we aren't removing here is intended to be passed through.
final remainingArguments =
Expand Down Expand Up @@ -271,23 +282,34 @@ class ServeCommand extends Command {
}

// This call will not exit until explicitly terminated by the user.
final cliCommand = CliCommand.dart([
if (debugServer) ...['run', '--observe=0'],
ddsServeLocalScriptPath,
if (runApp)
// When running DevTools via `flutter run`, the [flutterRunProcess]
// below will launch DevTools in the browser.
'--no-launch-browser'
else
// Only pass a build location if the server is serving the web assets
// (i.e. not when DevTools app is ran via `flutter run`).
'--devtools-build=$devToolsBuildLocation',
// Pass any args that were provided to our script along. This allows IDEs
// to pass `--machine` (etc.) so that this script can behave the same as
// the "dart devtools" command for testing local DevTools/server changes.
...remainingArguments,
], sdkOverride: serveWithDartSdk);
if (forMachine) {
// If --machine flag is true, then the output is a tool-readable JSON.
// Therefore, skip reading the process output and instead just run the
// process.
return processManager.runProcess(
Copy link
Member

@kenzieschmoll kenzieschmoll Dec 17, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

if we run the process I think we will wait for this process to exit, which will cause this script to fail for the case where runApp is true. We wait for the serveLocalProcess exit below, so we should be able to use startIndependentProcess here still

Copy link
Member Author

@elliette elliette Dec 17, 2024

Choose a reason for hiding this comment

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

Chatted offline that for now we can check whether --machine and --run-app are both set and if so throw an exception.

cliCommand,
workingDirectory: localDartSdkLocation,
);
}

final serveLocalProcess = await startIndependentProcess(
CliCommand.dart([
if (debugServer) ...['run', '--observe=0'],
ddsServeLocalScriptPath,
if (runApp)
// When running DevTools via `flutter run`, the [flutterRunProcess]
// below will launch DevTools in the browser.
'--no-launch-browser'
else
// Only pass a build location if the server is serving the web assets
// (i.e. not when DevTools app is ran via `flutter run`).
'--devtools-build=$devToolsBuildLocation',
// Pass any args that were provided to our script along. This allows IDEs
// to pass `--machine` (etc.) so that this script can behave the same as
// the "dart devtools" command for testing local DevTools/server changes.
...remainingArguments,
], sdkOverride: serveWithDartSdk),
cliCommand,
workingDirectory: localDartSdkLocation,
waitForOutput: _devToolsServerAddressLine,
onOutput: processServeLocalOutput,
Expand Down
Loading