| Summary: | Fix WTFLogVerbose variadic parameters forwarding | ||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|
| Product: | WebKit | Reporter: | Xabier Rodríguez Calvar <calvaris> | ||||||||
| Component: | Web Template Framework | Assignee: | Xabier Rodríguez Calvar <calvaris> | ||||||||
| Status: | RESOLVED FIXED | ||||||||||
| Severity: | Normal | CC: | achristensen, benjamin, cdumez, cmarcelo, commit-queue, darin, dbates, ews-watchlist, tsavell, webkit-bug-importer | ||||||||
| Priority: | P2 | Keywords: | InRadar | ||||||||
| Version: | WebKit Nightly Build | ||||||||||
| Hardware: | Unspecified | ||||||||||
| OS: | Unspecified | ||||||||||
| Attachments: |
|
||||||||||
|
Description
Xabier Rodríguez Calvar
2019-02-21 14:17:20 PST
Created attachment 362647 [details]
Patch
Comment on attachment 362647 [details] Patch Attachment 362647 [details] did not pass mac-debug-ews (mac): Output: https://webkit-queues.webkit.org/results/11238881 New failing tests: storage/indexeddb/intversion-open-in-upgradeneeded.html storage/indexeddb/intversion-open-in-upgradeneeded-private.html storage/indexeddb/modern/blocked-open-db-requests-private.html storage/indexeddb/odd-strings-private.html Created attachment 362675 [details]
Archive of layout-test-results from ews115 for mac-highsierra
The attached test failures were seen while running run-webkit-tests on the mac-debug-ews.
Bot: ews115 Port: mac-highsierra Platform: Mac OS X 10.13.6
Comment on attachment 362647 [details]
Patch
r=me
I'm curious about what configuration led you to need to fix this bug.
Comment on attachment 362647 [details] Patch Rejecting attachment 362647 [details] from commit-queue. Failed to run "['/Volumes/Data/EWS/WebKit/Tools/Scripts/webkit-patch', '--status-host=webkit-queues.webkit.org', '--bot-id=webkit-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 362647, '--port=mac']" exit_code: 2 cwd: /Volumes/Data/EWS/WebKit Logging in as commit-queue@webkit.org... Fetching: https://bugs.webkit.org/attachment.cgi?id=362647&action=edit Fetching: https://bugs.webkit.org/show_bug.cgi?id=194920&ctype=xml&excludefield=attachmentdata Processing 1 patch from 1 bug. Processing patch 362647 from bug 194920. Fetching: https://bugs.webkit.org/attachment.cgi?id=362647 Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Parsed 2 diffs from patch file(s). patching file Source/WTF/ChangeLog Hunk #1 succeeded at 1 with fuzz 3. patching file Source/WTF/wtf/Assertions.cpp Hunk #1 FAILED at 380. Hunk #2 succeeded at 454 (offset 50 lines). Hunk #3 succeeded at 473 (offset 50 lines). 1 out of 3 hunks FAILED -- saving rejects to file Source/WTF/wtf/Assertions.cpp.rej Failed to run "[u'/Volumes/Data/EWS/WebKit/Tools/Scripts/svn-apply', '--force', '--reviewer', u'Alex Christensen']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit Full output: https://webkit-queues.webkit.org/results/11274586 (In reply to Alex Christensen from comment #4) > Comment on attachment 362647 [details] > Patch > > r=me > I'm curious about what configuration led you to need to fix this bug. Nothing too fancy, just: @@ -458,7 +458,7 @@ WTF_EXPORT_PRIVATE NO_RETURN_DUE_TO_CRASH void WTFCrashWithSecurityImplication(v #if LOG_DISABLED #define LOG(channel, ...) ((void)0) #else -#define LOG(channel, ...) WTFLog(&LOG_CHANNEL(channel), __VA_ARGS__) +#define LOG(channel, ...) WTFLogVerbose(__FILE__, __LINE__, WTF_PRETTY_FUNCTION, &LOG_CHANNEL(channel), __VA_ARGS__) #endif /* LOG_VERBOSE */ For what I'm debugging, it is easier for me to know where the prints are done. I thought of adding a compile switch to force this. Do you think it could be interesting? PS: Thx for the review, will land tomorrow morning CET. I don't think a compile switch to turn on all the logs would be good. There are ways to enable and disable each logging channel at runtime, which can accomplish the same thing. (In reply to Alex Christensen from comment #7) > I don't think a compile switch to turn on all the logs would be good. There > are ways to enable and disable each logging channel at runtime, which can > accomplish the same thing. No, I'm talking about a compile switch to do what I do in the diff I added here Created attachment 362962 [details]
Patch for landing
Comment on attachment 362962 [details] Patch for landing Clearing flags on attachment: 362962 Committed r242075: <https://trac.webkit.org/changeset/242075> All reviewed patches have been landed. Closing bug. The changes in https://trac.webkit.org/changeset/242075/webkit in conjunction with the rollout in https://trac.webkit.org/changeset/242189/webkit has caused several storage/indexeddb/ tests to crash on Mac Debug WK1 and are slowing down EWS. Marking the tests as skip causes other tests to crash. The details of this were tracked in https://bugs.webkit.org/show_bug.cgi?id=195210 and https://bugs.webkit.org/show_bug.cgi?id=195141 Can you update your change to reflect the rollout? otherwise we will need to roll r242075 out as well to fix testing. (In reply to Truitt Savell from comment #13) > Can you update your change to reflect the rollout? otherwise we will need to > roll r242075 out as well to fix testing. On it. Anyway, I don't follow why I need to fix this myself or my patch will be rolled out. The original implementation is wrong and it should be fixed as well. We can't just roll out a patch and leave things wrong, right? (In reply to Truitt Savell from comment #13) > Can you update your change to reflect the rollout? otherwise we will need to > roll r242075 out as well to fix testing. As I commented on bug 195210, it looks like bots are green now thanks to a fix by Darin, who landed r242308. |