Skip to content

Conversation

@jmagman
Copy link
Member

@jmagman jmagman commented Sep 22, 2020

Description

iOS logging broke with #54132. The migration missed this logic:

    // The VM service will not publish logging events unless the debug stream is being listened to.
    // onDebugEvent listens to this stream as a side effect.
    unawaited(connectedVmService.onDebugEvent);

If the debug stream isn't listened to, the VM service won't pass along Stdout/Stderr events.

flutter attach now shows logs.

Related Issues

Fixes #65519.

Tests

Updated ios_device_logger_test.

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.

@jmagman jmagman added the platform-ios iOS applications specifically label Sep 22, 2020
@jmagman jmagman self-assigned this Sep 22, 2020
@flutter-dashboard flutter-dashboard bot added the tool Affects the "flutter" command-line tool. See also t: labels. label Sep 22, 2020
@jmagman
Copy link
Member Author

jmagman commented Sep 22, 2020

If this gets cherry-picked to 1.20, I'll also add a version of https://github.com/flutter/flutter/pull/66293/files#diff-8af27919996ab2011c7860416c33b74bR51 to validate the logs are visible on flutter run.

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

@fluttergithubbot fluttergithubbot merged commit 658e6c8 into flutter:master Sep 23, 2020
@jmagman jmagman deleted the vm-service-logging branch September 23, 2020 00:32
christopherfujino pushed a commit to chris-forks/flutter that referenced this pull request Sep 25, 2020
christopherfujino added a commit that referenced this pull request Sep 25, 2020
* Deprecate VelocityTracker default constructor and added VelocityTracker.withKind constructor (#66043)

* [flutter_tools] fix failure to create ansi spinner if download needs to be retried (#65797)

* Listen to Debug VM stream to get Stdout logs from VMService (#66310)

* Update cherry picked engine

Co-authored-by: Jonah Williams <jonahwilliams@google.com>
Co-authored-by: Jenn Magder <magder@google.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

platform-ios iOS applications specifically tool Affects the "flutter" command-line tool. See also t: labels.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

"print" statements do not work with iOS 13.7 physical device

4 participants