Skip to content

[WPE][GTK][PlayStation] Remove unneeded WTFLogChannel::subsystem #47375

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.

Sign up for GitHub

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

Jump to bottom
Merged
merged 1 commit into from
Jul 1, 2025
Merged

[WPE][GTK][PlayStation] Remove unneeded WTFLogChannel::subsystem #47375

merged 1 commit into from
Jul 1, 2025
+9 −14

Conversation

aperezdc
Copy link
Contributor

@aperezdc aperezdc commented Jun 30, 2025 •
edited by webkit-early-warning-system
Loading

3695ee5

[WPE][GTK][PlayStation] Remove unneeded WTFLogChannel::subsystem
https://bugs.webkit.org/show_bug.cgi?id=295202

Reviewed by Ross Kirsling.

The WTFLogChannel::subsystem struct member always took the same value,
from the LOG_CHANNEL_WEBKIT_SUBSYSTEM macro, therefore it is possible
to remove the member and use the macro directly. This reduces the size
of the WTFLogChannel struct without affecting the produced log messages,
which avoids breaking tooling used to analyze logs.

While at it, replace fprintf() usage with SAFE_FPRINTF() to keep the
style checker happy.

* Source/WTF/wtf/Assertions.h:
* Source/WTF/wtf/Logger.h:
(WTF::Logger::log):
(WTF::Logger::logVerbose):

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

aa607fd

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 self-assigned this Jun 30, 2025
@webkit-early-warning-system
Copy link
Collaborator

webkit-early-warning-system commented Jun 30, 2025 •
edited
Loading

EWS run on current version of this PR (hash aa607fd)

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 review from rkirsling, a team and achristensen07 June 30, 2025 13:17
@aperezdc
Copy link
Contributor Author

It looks like it may be possible to remove the subsystem struct member in general, given that Apple ports also seem to always use com.apple.WebKit as the subsystem; but that would also come with some cleanups in macros used to declare log channels, so I think that I would rather do a follow-up patch after this one gets merged 🤔

rkirsling
rkirsling approved these changes Jul 1, 2025
View reviewed changes
Copy link
Member

@rkirsling rkirsling left a comment

Choose a reason for hiding this comment

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

Makes sense; the follow-up patch sounds like a good idea.

@aperezdc aperezdc added the merge-queue Applied to send a pull request to merge-queue label Jul 1, 2025
@aperezdc
Copy link
Contributor Author

aperezdc commented Jul 1, 2025 •
edited
Loading

Makes sense; the follow-up patch sounds like a good idea.

Aye, will do, follow-up already being prepared here: #47429 😄

@aperezdc
[WPE][GTK][PlayStation] Remove unneeded WTFLogChannel::subsystem
3695ee5 
https://bugs.webkit.org/show_bug.cgi?id=295202

Reviewed by Ross Kirsling.

The WTFLogChannel::subsystem struct member always took the same value,
from the LOG_CHANNEL_WEBKIT_SUBSYSTEM macro, therefore it is possible
to remove the member and use the macro directly. This reduces the size
of the WTFLogChannel struct without affecting the produced log messages,
which avoids breaking tooling used to analyze logs.

While at it, replace fprintf() usage with SAFE_FPRINTF() to keep the
style checker happy.

* Source/WTF/wtf/Assertions.h:
* Source/WTF/wtf/Logger.h:
(WTF::Logger::log):
(WTF::Logger::logVerbose):

Canonical link: https://commits.webkit.org/296858@main
@webkit-commit-queue webkit-commit-queue force-pushed the remove-logchannel-subsys branch from aa607fd to 3695ee5 Compare July 1, 2025 07:51
@webkit-commit-queue
Copy link
Collaborator

Committed 296858@main (3695ee5): https://commits.webkit.org/296858@main

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

@webkit-commit-queue webkit-commit-queue merged commit 3695ee5 into WebKit:main Jul 1, 2025
@webkit-commit-queue webkit-commit-queue removed the merge-queue Applied to send a pull request to merge-queue label Jul 1, 2025
@aperezdc aperezdc mentioned this pull request Jul 1, 2025
Open
@aperezdc aperezdc deleted the remove-logchannel-subsys branch July 1, 2025 14:58
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@rkirsling rkirsling rkirsling approved these changes

@achristensen07 achristensen07 Awaiting requested review from achristensen07

Assignees

@aperezdc aperezdc

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
4 participants
@aperezdc @webkit-early-warning-system @webkit-commit-queue @rkirsling