Skip to content

Conversation

@dannyvalentesonos
Copy link
Contributor

Summary

A Custom Device's log reader doesn't do anything, and thus flutter attach will fail to ever find the flutter engine's VM service.

This change adds a new readLogs command to the CustomDeviceConfig, and which allows a custom device to read the logs from the running flutter app on the actual custom device, and then allow flutter attach to work.

Fixes #170634

Testing

Created a custom device, and added the following readLogs command in the custom_devices.json file:

"readLogs": [
        "sshpass",
        "-f",
        "/home/dvalente/player.txt",
        "ssh",
        "[email protected]",
        "tail -Fn +1 /opt/log/flutter.log"
      ]

Then, running flutter attach works every time.

Tested the following scenarios:

  • Launched flutter attach before app is running on device, and once app runs, flutter attach succeeds
  • Launched flutter attach after app is already running on device, and flutter attach succeeds.
  • Used flutter attach in Android Studio, and hot reload works just like any Android device.

@flutter-dashboard
Copy link

It looks like this pull request may not have tests. Please make sure to add tests or get an explicit test exemption before merging.

If you are not sure if you need tests, consider this rule of thumb: the purpose of a test is to make sure someone doesn't accidentally revert the fix. Ask yourself, is there anything in your PR that you feel it is important we not accidentally revert back to how it was before your fix?

Reviewers: Read the Tree Hygiene page and make sure this patch meets those guidelines before LGTMing.If you believe this PR qualifies for a test exemption, contact "@test-exemption-reviewer" in the #hackers channel in Discord (don't just cc them here, they won't see it!). The test exemption team is a small volunteer group, so all reviewers should feel empowered to ask for tests, without delegating that responsibility entirely to the test exemption group.

@github-actions github-actions bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Jun 14, 2025
@dannyvalentesonos dannyvalentesonos force-pushed the topic/fix_vm_discovery_custom_device branch 2 times, most recently from e11f162 to 7b1d1c7 Compare June 16, 2025 18:19
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.

I like this! I do think we'll need some more thorough tests that actually exercise the behavior of the log reader before I can approve this. This could be as simple as setting the readLogs command to echo some message and then verifying we're actually able to see it.

@bkonyi bkonyi requested a review from matanlurey June 23, 2025 14:51
@dannyvalentesonos
Copy link
Contributor Author

I like this! I do think we'll need some more thorough tests that actually exercise the behavior of the log reader before I can approve this. This could be as simple as setting the readLogs command to echo some message and then verifying we're actually able to see it.

Thanks for the comment @bkonyi! Can you point me to where this test should live? I have not been able to find where the other commands from the custom device are unit tested anywhere. Thanks!

@bkonyi
Copy link
Contributor

bkonyi commented Jul 2, 2025

Thanks for the comment @bkonyi! Can you point me to where this test should live? I have not been able to find where the other commands from the custom device are unit tested anywhere. Thanks!

I think they can live in packages/flutter_tools/test/commands.shard/hermetic/custom_devices_test.dart

@dannyvalentesonos dannyvalentesonos force-pushed the topic/fix_vm_discovery_custom_device branch 5 times, most recently from 0b4c094 to 79ea8ff Compare July 7, 2025 21:57
@dannyvalentesonos
Copy link
Contributor Author

Thanks for the comment @bkonyi! Can you point me to where this test should live? I have not been able to find where the other commands from the custom device are unit tested anywhere. Thanks!

I think they can live in packages/flutter_tools/test/commands.shard/hermetic/custom_devices_test.dart

@bkonyi I've added the unit test. Thanks!

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.

LGTM overall! Thanks for adding the test!

A Custom Device's log reader doesn't do anything, and thus `flutter
attach` will fail to ever find the flutter engine's VM service.

This change adds a new `readLogs` command to the CustomDeviceConfig, and
which allows a custom device to read the logs from the running flutter
app on the actual custom device, and then allow flutter attach to work.
@dannyvalentesonos dannyvalentesonos force-pushed the topic/fix_vm_discovery_custom_device branch from 128dc92 to 9672a8d Compare July 8, 2025 14:33
@dannyvalentesonos
Copy link
Contributor Author

@bkonyi I do not have access to merge this PR. Would you do it for me please? Thanks!

@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
@matanlurey
Copy link
Contributor

I got it.

@auto-submit
Copy link
Contributor

auto-submit bot commented Jul 9, 2025

autosubmit label was removed for flutter/flutter/170635, because This PR has not met approval requirements for merging. The PR author is not a member of flutter-hackers and needs 1 more review(s) in order to merge this PR.

  • Merge guidelines: A PR needs at least one approved review if the author is already part of flutter-hackers or two member reviews if the author is not a flutter-hacker before re-applying the autosubmit label. Reviewers: If you left a comment approving, please use the "approve" review action instead.

@auto-submit auto-submit bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
@matanlurey matanlurey added the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
@auto-submit auto-submit bot added this pull request to the merge queue Jul 9, 2025
Merged via the queue into flutter:master with commit 953b6e4 Jul 9, 2025
139 checks passed
@flutter-dashboard flutter-dashboard bot removed the autosubmit Merge PR when tree becomes green via auto submit App label Jul 9, 2025
@dannyvalentesonos
Copy link
Contributor Author

@bkonyi From code inspection, I noticed that the desktop implementations also have the same issue. If so, I feel it could be beneficial to add the same type of thing for those platforms too...

engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Jul 10, 2025
auto-submit bot pushed a commit to flutter/packages that referenced this pull request Jul 10, 2025
flutter/flutter@ac12f66...43657f3

2025-07-10 [email protected] [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 [email protected] Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 [email protected] Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 [email protected] Refactor compositor classes (flutter/flutter#171414)
2025-07-10 [email protected] Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 [email protected] Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 [email protected] Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 [email protected] Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 [email protected] Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 [email protected] Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 [email protected] Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 [email protected] Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 [email protected] Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 [email protected] [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 [email protected] Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 [email protected] feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 [email protected] Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 [email protected] Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 [email protected] Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 [email protected] Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 [email protected] Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 [email protected] Make `labels` field an array (flutter/flutter#171906)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (#171897)" (flutter/flutter#171910)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (flutter/flutter#171897)
2025-07-09 [email protected] [skia] Fix flag fiddling for Fuchsia, FreeType, & friends (flutter/flutter#171874)

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
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 10, 2025
…ice (flutter#170635)

## Summary

A Custom Device's log reader doesn't do anything, and thus `flutter
attach` will fail to ever find the flutter engine's VM service.

This change adds a new `readLogs` command to the CustomDeviceConfig, and
which allows a custom device to read the logs from the running flutter
app on the actual custom device, and then allow flutter attach to work.

Fixes flutter#170634

## Testing

Created a custom device, and added the following readLogs command in the
`custom_devices.json` file:
```
"readLogs": [
        "sshpass",
        "-f",
        "/home/dvalente/player.txt",
        "ssh",
        "[email protected]",
        "tail -Fn +1 /opt/log/flutter.log"
      ]
```

Then, running `flutter attach` works every time.

**Tested the following scenarios:**
* Launched `flutter attach` before app is running on device, and once
app runs, flutter attach succeeds
* Launched `flutter attach` after app is already running on device, and
flutter attach succeeds.
* Used `flutter attach` in Android Studio, and hot reload works just
like any Android device.
@dannyvalentesonos dannyvalentesonos deleted the topic/fix_vm_discovery_custom_device branch July 25, 2025 15:12
azatech pushed a commit to azatech/flutter that referenced this pull request Jul 28, 2025
…ice (flutter#170635)

## Summary

A Custom Device's log reader doesn't do anything, and thus `flutter
attach` will fail to ever find the flutter engine's VM service.

This change adds a new `readLogs` command to the CustomDeviceConfig, and
which allows a custom device to read the logs from the running flutter
app on the actual custom device, and then allow flutter attach to work.

Fixes flutter#170634

## Testing

Created a custom device, and added the following readLogs command in the
`custom_devices.json` file:
```
"readLogs": [
        "sshpass",
        "-f",
        "/home/dvalente/player.txt",
        "ssh",
        "[email protected]",
        "tail -Fn +1 /opt/log/flutter.log"
      ]
```

Then, running `flutter attach` works every time.

**Tested the following scenarios:**
* Launched `flutter attach` before app is running on device, and once
app runs, flutter attach succeeds
* Launched `flutter attach` after app is already running on device, and
flutter attach succeeds.
* Used `flutter attach` in Android Studio, and hot reload works just
like any Android device.
vashworth pushed a commit to vashworth/packages that referenced this pull request Jul 30, 2025
…r#9589)

flutter/flutter@ac12f66...43657f3

2025-07-10 [email protected] [web] Add frame number support. (flutter/flutter#171592)
2025-07-10 [email protected] Fix the hitTest issue of reversed SliverMainAxisGroup. (flutter/flutter#171073)
2025-07-10 [email protected] Roll Fuchsia Linux SDK from 0-xqmXWc4cXzw3tfe... to lO64ePNEGrGzs-MFC... (flutter/flutter#171937)
2025-07-10 [email protected] Refactor compositor classes (flutter/flutter#171414)
2025-07-10 [email protected] Give an actionable error to `flutter_test.*tap` of a `RenderSliver` (flutter/flutter#171930)
2025-07-10 [email protected] Fix the issue with `SliverMainAxisGroups` growing in the reverse direction during layout. (flutter/flutter#171005)
2025-07-09 [email protected] Adds a MCP server for working with the engine (flutter/flutter#171738)
2025-07-09 [email protected] Use Async SurfaceHolder Callback to remove need for setting alpha workaround (flutter/flutter#171398)
2025-07-09 [email protected] Update `CHANGELOG` for 3.32.5, 3.32.6 stable hotfix releases (flutter/flutter#171891)
2025-07-09 [email protected] Add `flutter config --enable-omit-legacy-version-file` (flutter/flutter#171903)
2025-07-09 [email protected] Allow flutter attach to discover flutter engine running on Custom Device (flutter/flutter#170635)
2025-07-09 [email protected] Hide the rarely direct used `--sample` argument by default (flutter/flutter#171898)
2025-07-09 [email protected] Support `NO_COLOR` to opt-out of `flutter` tool ANSI colors (flutter/flutter#171892)
2025-07-09 [email protected] [Android 16] Added Docs to Warn Users that SystemChrome.setPreferredOrientations will Not Work (flutter/flutter#171089)
2025-07-09 [email protected] Add analytics events for wasm dry runs on web builds (flutter/flutter#171818)
2025-07-09 [email protected] feat: new builders for size experiment (flutter/flutter#171886)
2025-07-09 [email protected] Update `.gitignore`s (flutter/flutter#171907)
2025-07-09 [email protected] Add total execution time to the flutter upgrade command (flutter/flutter#171475)
2025-07-09 [email protected] Simplify the template for infrastructure requests (flutter/flutter#171905)
2025-07-09 [email protected] Add detailed error message for BorderRadiusDirectional (flutter/flutter#171805)
2025-07-09 [email protected] Add public postmortem of the 3.32.3 release. (flutter/flutter#171904)
2025-07-09 [email protected] Make `labels` field an array (flutter/flutter#171906)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (#171897)" (flutter/flutter#171910)
2025-07-09 98614782+auto-submit[bot]@users.noreply.github.com Reverts "Roll Packages from cba2e90 to 4a231ae (5 revisions) (#171879)" (flutter/flutter#171897)
2025-07-09 [email protected] [skia] Fix flag fiddling for Fuchsia, FreeType, & friends (flutter/flutter#171874)

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
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 14, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 15, 2025
engine-flutter-autoroll added a commit to engine-flutter-autoroll/packages that referenced this pull request Aug 16, 2025
ksokolovskyi pushed a commit to ksokolovskyi/flutter that referenced this pull request Aug 19, 2025
…ice (flutter#170635)

## Summary

A Custom Device's log reader doesn't do anything, and thus `flutter
attach` will fail to ever find the flutter engine's VM service.

This change adds a new `readLogs` command to the CustomDeviceConfig, and
which allows a custom device to read the logs from the running flutter
app on the actual custom device, and then allow flutter attach to work.

Fixes flutter#170634

## Testing

Created a custom device, and added the following readLogs command in the
`custom_devices.json` file:
```
"readLogs": [
        "sshpass",
        "-f",
        "/home/dvalente/player.txt",
        "ssh",
        "[email protected]",
        "tail -Fn +1 /opt/log/flutter.log"
      ]
```

Then, running `flutter attach` works every time.

**Tested the following scenarios:**
* Launched `flutter attach` before app is running on device, and once
app runs, flutter attach succeeds
* Launched `flutter attach` after app is already running on device, and
flutter attach succeeds.
* Used `flutter attach` in Android Studio, and hot reload works just
like any Android device.
lucaantonelli pushed a commit to lucaantonelli/flutter that referenced this pull request Nov 21, 2025
…ice (flutter#170635)

## Summary

A Custom Device's log reader doesn't do anything, and thus `flutter
attach` will fail to ever find the flutter engine's VM service.

This change adds a new `readLogs` command to the CustomDeviceConfig, and
which allows a custom device to read the logs from the running flutter
app on the actual custom device, and then allow flutter attach to work.

Fixes flutter#170634

## Testing

Created a custom device, and added the following readLogs command in the
`custom_devices.json` file:
```
"readLogs": [
        "sshpass",
        "-f",
        "/home/dvalente/player.txt",
        "ssh",
        "[email protected]",
        "tail -Fn +1 /opt/log/flutter.log"
      ]
```

Then, running `flutter attach` works every time.

**Tested the following scenarios:**
* Launched `flutter attach` before app is running on device, and once
app runs, flutter attach succeeds
* Launched `flutter attach` after app is already running on device, and
flutter attach succeeds.
* Used `flutter attach` in Android Studio, and hot reload works just
like any Android device.
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.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Flutter attach never finds the running app on a custom device

3 participants