Skip to content

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Sep 22, 2020

Reland #66092. See #66364 (comment) for details.

Additionally, the debugger will only attach on iOS 13+ where the syslog trick doesn't work anymore.
Also, this includes #66293 to make the integration test an atomic addition to this feature.

Description

As of iOS 13, the host can't directly read the device logs from the host. #43915 connected to the VM observatory and read the log lines from the logging services.

On flutter run and flutter drive (not attach in this PR), instead of waiting to attach to the observatory, instead launch the app with the debugger and stay attached to stream in the logs from lldb. See #54781 (comment) for details. This should also reduce devicelab flakes.

This will give the tool access to:

  • The observatory port log line
  • Full native crash backtraces, including crashes on launch before the engine starts
  • dart print()
  • dart stderr.writeln, other I/O sinks
  • native logging (os_log, asserts etc)
  • Engine logs (may need follow up work to filter out the verbose category)

Notes

  • Keep the VM service connection for flutter attach. A follow up PR could attach the debugger in that scenario as well (without re-launching), and then the VM connection can be removed.
  • Keep the syslog connection for <iOS 13. I wasn't able to get this working on my iOS 9.3.6 test device, so don't regress it. If I can get that working, we can also remove idevicesyslog.

Related Issues

Fixes #44717
Fixes #44718
Fixes #54781

#39240 (fixes run/drive, not attach)
#58078 (fixes run/drive, not attach)

Mitigate #46705 (run/drive, not attach)
Will allow us to turn off mDNS fallback for drive on CI to solve iOS 14 issue #65207

Note to self: check if this fixes #19196

Tests

  • ios_deploy_tests
  • ios_device_logger_tests
  • ios_device_start_prebuilt_tests

Follow up with logging integration tests so this stops regressing.

Also, manually tested:

  • Detaching doesn't stop the app.
  • ios-deploy processes aren't leaking

Checklist

  • I read the [Contributor Guide] and followed the process outlined there for submitting PRs.
  • I signed the [CLA].
  • I read and followed the [Flutter Style Guide], including [Features we expect every widget to implement].
  • I read the [Tree Hygiene] wiki page, which explains my responsibilities.
  • I updated/added relevant documentation (doc comments with ///).
  • All existing and new tests are passing.
  • The analyzer (flutter analyze --flutter-repo) does not report any problems on my PR.
  • I am willing to follow-up on review comments in a timely manner.

Breaking Change

  • No, no existing tests failed, so this is not a breaking change.
  • Yes, this is a breaking change. If not, delete the remainder of this section.

@flutter-dashboard flutter-dashboard bot added c: contributor-productivity Team-specific productivity, code health, technical debt. tool Affects the "flutter" command-line tool. See also t: labels. labels Sep 22, 2020
@jmagman jmagman added platform-ios iOS applications specifically and removed cla: yes labels Sep 22, 2020
Copy link
Contributor

@jonahwilliams jonahwilliams left a comment

Choose a reason for hiding this comment

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

LGTM ship it

@jmagman
Copy link
Member Author

jmagman commented Sep 22, 2020

There are three commits in this PR:

  1. 62fd321 is strictly Stream logging from attached debugger on iOS #66092 with no changes.
  2. ecce616 is strictly Stop logging on iOS process exit #66293 with no changes.
  3. c409a66 is the change to make this iOS 13+ only.

@jonahwilliams
Copy link
Contributor

If A, B, and C work then they should all work together too :)

@jmagman jmagman self-assigned this Sep 22, 2020
@jmagman jmagman merged commit d5b715d into flutter:master Sep 23, 2020
@jmagman jmagman deleted the attach-debugger branch September 23, 2020 00:16
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

c: contributor-productivity Team-specific productivity, code health, technical debt. platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

3 participants