Skip to content

[WTF][WPE] Read the log level string from an Android system property #47352

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
merged 1 commit into from
Jul 16, 2025

Conversation

aperezdc
Copy link
Contributor

@aperezdc aperezdc commented Jun 29, 2025

f8b520c

[WTF][WPE] Read the log level string from an Android system property
https://bugs.webkit.org/show_bug.cgi?id=295175

Reviewed by Nikolas Zimmermann and Michael Catanzaro.

Read the value of the debug.WPEWebKit.log system property to determine
the value returned by WTF::logLevelString() to configure logging
channels on Android.

This allows using the "setprop" command line tool to configure logging:

  adb shell setprop debug.WPEWebKit.log 'Scrolling,Loading'

* Source/WTF/wtf/PlatformWPE.cmake:
* Source/WTF/wtf/android/LoggingAndroid.cpp: Copied from Source/WTF/wtf/unix/LoggingUnix.cpp.
(WTF::logLevelString):
* Source/WTF/wtf/unix/LoggingUnix.cpp: Guard with !OS(ANDROID); remove
unneeded <string.h> header inclusion, and make logLevel "const" as
drive-by fixes.
(WTF::logLevelString):

Canonical link: https://commits.webkit.org/297467@main

44383f0

Misc iOS, visionOS, tvOS & watchOS macOS Linux Windows
✅ 🧪 style ✅ 🛠 ios ✅ 🛠 mac ✅ 🛠 wpe ✅ 🛠 win
✅ 🛠 ios-sim ✅ 🛠 mac-AS-debug 🧪 wpe-wk2 ❌ 🧪 win-tests
✅ 🧪 webkitperl ✅ 🧪 ios-wk2 ✅ 🧪 api-mac ✅ 🧪 api-wpe
✅ 🧪 ios-wk2-wpt ✅ 🧪 mac-wk1 ✅ 🛠 wpe-cairo
✅ 🛠 🧪 jsc ✅ 🧪 api-ios 🧪 mac-wk2 ✅ 🛠 gtk
✅ 🛠 🧪 jsc-arm64 ✅ 🛠 vision 🧪 mac-AS-debug-wk2 🧪 gtk-wk2
✅ 🛠 vision-sim ✅ 🧪 mac-wk2-stress ✅ 🧪 api-gtk
✅ 🛠 🧪 merge ✅ 🧪 vision-wk2 ✅ 🧪 mac-intel-wk2 ✅ 🛠 playstation
✅ 🛠 tv 🛠 mac-safer-cpp ✅ 🛠 jsc-armv7
✅ 🛠 tv-sim ❌ 🧪 jsc-armv7-tests
✅ 🛠 watch
✅ 🛠 watch-sim

@aperezdc aperezdc requested a review from a team as a code owner June 29, 2025 12:37
@aperezdc aperezdc self-assigned this Jun 29, 2025
@aperezdc aperezdc added the WPE WebKit WebKit WPE component label Jun 29, 2025
@aperezdc aperezdc requested a review from spenap June 29, 2025 12:38
@aperezdc
Copy link
Contributor Author

This is most useful in combination with #47339 in order to have the logging messages sent to the Android logging service.

Copy link
Contributor

@spenap spenap left a comment

Choose a reason for hiding this comment

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

Philippe has a valid question, but I'm OK with both your approach and his suggestion. The changes look good!

@aperezdc aperezdc force-pushed the android-logging-sysprop branch from f292bc1 to e22246b Compare June 30, 2025 11:25
@aperezdc aperezdc force-pushed the android-logging-sysprop branch from e22246b to 6adbad7 Compare June 30, 2025 11:29
@aperezdc aperezdc requested review from philn and spenap June 30, 2025 11:30
@aperezdc aperezdc changed the title [WTF][WPE] Read the log level string from an Android system property [WPE] Log messages to the Android log where appropriate Jul 1, 2025
@aperezdc aperezdc changed the title [WPE] Log messages to the Android log where appropriate [WTF][WPE] Read the log level string from an Android system property Jul 1, 2025
@aperezdc aperezdc force-pushed the android-logging-sysprop branch from 6adbad7 to 6e676d2 Compare July 1, 2025 15:11
@aperezdc aperezdc added the GLib Suggested Backport - 2.48 Suggest this merge request be backported to webkitglib/2.48 branch label Jul 1, 2025
@aperezdc
Copy link
Contributor Author

aperezdc commented Jul 7, 2025

Ping reviewers 🏓

Copy link
Contributor

@mcatanzaro mcatanzaro left a comment

Choose a reason for hiding this comment

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

I would do this in a separate LoggingAndroid.cpp, because now half of LoggingUnix.cpp is unrelated to Unix. But whatever.

@aperezdc
Copy link
Contributor Author

aperezdc commented Jul 9, 2025

I would do this in a separate LoggingAndroid.cpp, because now half of LoggingUnix.cpp is unrelated to Unix. But whatever.

Actually, I am going to do that. It makes little sense in Android anyway to use environment variables (it's quite inconvenient, requires rebuilding the application package with a wrapper script) and I will make it only read the system property at runtime.

@aperezdc aperezdc force-pushed the android-logging-sysprop branch from 6e676d2 to 55e3a44 Compare July 9, 2025 22:56
@aperezdc aperezdc requested review from mcatanzaro and a team July 10, 2025 21:09
Copy link
Contributor

@svillar svillar left a comment

Choose a reason for hiding this comment

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

Shouldn't we include the adb command in docs? It's quite useful for developing and it'd be a shame forcing devs to look in the code if they can just see it on a README

@aperezdc
Copy link
Contributor Author

Shouldn't we include the adb command in docs? It's quite useful for developing and it'd be a shame forcing devs to look in the code if they can just see it on a README

My plan was to send a PR to update the page at https://docs.webkit.org/Build%20%26%20Debug/Logging.html but indeed it would be good to have it in the manual. I can do that in a follow-up patch, too.

@aperezdc aperezdc force-pushed the android-logging-sysprop branch from 55e3a44 to 44383f0 Compare July 16, 2025 13:32
@aperezdc aperezdc added the merge-queue Applied to send a pull request to merge-queue label Jul 16, 2025
@aperezdc aperezdc removed the merge-queue Applied to send a pull request to merge-queue label Jul 16, 2025
@aperezdc aperezdc changed the title [WTF][WPE] Read the log level string from an Android system property [WPE] Log messages to the Android log where appropriate Jul 16, 2025
@aperezdc aperezdc added merge-queue Applied to send a pull request to merge-queue and removed merge-queue Applied to send a pull request to merge-queue labels Jul 16, 2025
@aperezdc aperezdc changed the title [WPE] Log messages to the Android log where appropriate [WTF][WPE] Read the log level string from an Android system property Jul 16, 2025
@aperezdc aperezdc added the merge-queue Applied to send a pull request to merge-queue label Jul 16, 2025
aperezdc added a commit to aperezdc/WebKit-Documentation that referenced this pull request Jul 16, 2025
Document that the WPE port logs to the logd service on Android, and is
configured using system properties, after the two following patches:

- WebKit/WebKit#47339
- WebKit/WebKit#47352
@aperezdc
Copy link
Contributor Author

Shouldn't we include the adb command in docs? It's quite useful for developing and it'd be a shame forcing devs to look in the code if they can just see it on a README

My plan was to send a PR to update the page at https://docs.webkit.org/Build%20%26%20Debug/Logging.html but indeed it would be good to have it in the manual. I can do that in a follow-up patch, too.

Done here for the documentation site: WebKit/Documentation#128

https://bugs.webkit.org/show_bug.cgi?id=295175

Reviewed by Nikolas Zimmermann and Michael Catanzaro.

Read the value of the debug.WPEWebKit.log system property to determine
the value returned by WTF::logLevelString() to configure logging
channels on Android.

This allows using the "setprop" command line tool to configure logging:

  adb shell setprop debug.WPEWebKit.log 'Scrolling,Loading'

* Source/WTF/wtf/PlatformWPE.cmake:
* Source/WTF/wtf/android/LoggingAndroid.cpp: Copied from Source/WTF/wtf/unix/LoggingUnix.cpp.
(WTF::logLevelString):
* Source/WTF/wtf/unix/LoggingUnix.cpp: Guard with !OS(ANDROID); remove
unneeded <string.h> header inclusion, and make logLevel "const" as
drive-by fixes.
(WTF::logLevelString):

Canonical link: https://commits.webkit.org/297467@main
@webkit-commit-queue
Copy link
Collaborator

Committed 297467@main (f8b520c): https://commits.webkit.org/297467@main

Reviewed commits have been landed. Closing PR #47352 and removing active labels.

@webkit-commit-queue webkit-commit-queue merged commit f8b520c into WebKit:main Jul 16, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 16, 2025
@aperezdc aperezdc deleted the android-logging-sysprop branch July 16, 2025 16:31
@aperezdc
Copy link
Contributor Author

Backported into webkitglib/2.48 as commit 9281920

@aperezdc aperezdc removed the GLib Suggested Backport - 2.48 Suggest this merge request be backported to webkitglib/2.48 branch label Jul 16, 2025
aperezdc added a commit to aperezdc/WebKit-Documentation that referenced this pull request Jul 21, 2025
Document that the WPE port logs to the logd service on Android, and is
configured using system properties, after the two following patches:

- WebKit/WebKit#47339
- WebKit/WebKit#47352
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
WPE WebKit WebKit WPE component
Projects
None yet
Development

Successfully merging this pull request may close these issues.

8 participants