Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Conversation

dnfield
Copy link
Contributor

@dnfield dnfield commented Feb 7, 2024

Fixes flutter/flutter#142488

  • Only logs on iOS if Skia is used instead of Impeller.
  • Logs on other platforms if Impeller is used instead of Skia.
  • Uses "IMPORTANT" rather than "ERROR" for these logs. This will show up by default since flutter_tools sets ERROR and above as logs to show.
  • Adds some tests.
  • Makes INFO log print file paths the same as other verbosities.

constexpr LogSeverity kLogNumSeverities = 4;
// A log that is not an error, is important enough to display even if ordinary
// info is hidden.
constexpr LogSeverity kLogImportant = 3;
Copy link
Contributor

Choose a reason for hiding this comment

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

Will this break users who set a log level on the command line? If so, should we add a note in our release notes?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It shouldn't - I'm replacing the FATAL level with IMPORTANT - it's not actually possible to ignore FATAL, but I suppose someone who was suppressing ERROR will now get IMPORTANT. I doubt this is happening much if at all though.

Copy link
Member

@jmagman jmagman left a comment

Choose a reason for hiding this comment

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

We should really move the iOS/macOS logging from syslog to os_log. That's been on my backlog list for a long time...
I think LOG_ALERT still shows up as an error (OS_LOG_TYPE_ERROR)? When you use IMPORTANT does it still look like:

[ERROR:flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.mm] blah

@dnfield
Copy link
Contributor Author

dnfield commented Feb 7, 2024

We should really move the iOS/macOS logging from syslog to os_log. That's been on my backlog list for a long time... I think LOG_ALERT still shows up as an error (OS_LOG_TYPE_ERROR)? When you use IMPORTANT does it still look like:

[ERROR:flutter/shell/platform/darwin/graphics/FlutterDarwinContextMetalImpeller.mm] blah

The [ERROR part is something we're adding to the string getting logged. It will look like [IMPORTANT....

@dnfield
Copy link
Contributor Author

dnfield commented Feb 7, 2024

(it's quite possible that Console.app will still show it as some kind of error even though the string won't say ERROR...)

Copy link
Contributor

@christopherfujino christopherfujino left a comment

Choose a reason for hiding this comment

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

RSLGTM

@dnfield dnfield added the autosubmit Merge PR when tree becomes green via auto submit App label Feb 7, 2024
@auto-submit auto-submit bot merged commit 3a86ea9 into flutter:main Feb 7, 2024
engine-flutter-autoroll added a commit to engine-flutter-autoroll/flutter that referenced this pull request Feb 7, 2024
auto-submit bot pushed a commit to flutter/flutter that referenced this pull request Feb 7, 2024
…143123)

flutter/engine@b1ba9f3...322a461

2024-02-07 [email protected] Roll Skia from eacaa2d3600c to 8332438cbeb9 (6 revisions) (flutter/engine#50451)
2024-02-07 [email protected] [Impeller] Log non-default graphics backend usages, use IMPORTANT rather than ERROR (flutter/engine#50448)

If this roll has caused a breakage, revert this CL and stop the roller
using the controls here:
https://autoroll.skia.org/r/flutter-engine-flutter-autoroll
Please CC [email protected],[email protected],[email protected] on the revert to ensure that a human
is aware of the problem.

To file a bug in Flutter: 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
@dnfield dnfield deleted the log branch February 8, 2024 03:22
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
autosubmit Merge PR when tree becomes green via auto submit App platform-android
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remove "Using the Impeller rendering backend" error that logs on every flutter run.
4 participants