Skip to content

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Sep 22, 2020

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

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 the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 22, 2020
@jmagman jmagman changed the title Stream logging from attached debugger on iOS (#66092) Stream logging from attached debugger on iOS Sep 22, 2020
@jmagman
Copy link
Member Author

jmagman commented Sep 22, 2020

smoke_catalina_hot_mode_dev_cycle_ios__benchmark was failing on this commit in the devicelab. And when I reverted it, it stopped failing. However, the crash didn't look related to this change--it was crashing from Xcode, which doesn't go through the flutter tool code paths. I deleted DerivedData and it started working on this patch again.
Let's try again.

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

@jmagman
Copy link
Member Author

jmagman commented Sep 22, 2020

Going to merge this, it's gone through a few versions of these tests, and it will make the flaky build greener. Or it will blow up smoke_catalina_hot_mode_dev_cycle_ios__benchmark again and I'll be confused all over.

@jmagman jmagman merged commit 2be4570 into flutter:master Sep 22, 2020
@jmagman jmagman deleted the attach-debugger branch September 22, 2020 21:59
jmagman added a commit that referenced this pull request Sep 22, 2020
jmagman added a commit that referenced this pull request Sep 22, 2020
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

3 participants