Bug 165308

Summary: WebAssembly: revert patch causing odd breakage
Product: WebKit Reporter: JF Bastien <jfbastien>
Component: JavaScriptCoreAssignee: JF Bastien <jfbastien>
Status: RESOLVED FIXED    
Severity: Normal CC: commit-queue, fpizlo, ggaren, jfbastien, keith_miller, mark.lam, msaboff, saam
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Bug Depends on: 164724    
Bug Blocks: 161709, 165335, 165345    
Attachments:
Description Flags
patch
jfbastien: commit-queue+
patch
commit-queue: commit-queue-
patch none

JF Bastien
Reported 2016-12-02 08:58:35 PST
Bug #164724 seems to cause build issues which I haven't tracked down yet. WasmOps.h can't be found: ./Source/JavaScriptCore/wasm/WasmFormat.h:34:10: fatal error: 'WasmOps.h' file not found It's weird since the file is auto-generated and has been for a while. #164724 merely includes it in WasmFormat.h. I have to run out now, so I'll revert #164724 and investigate separately.
Attachments
patch (181.58 KB, patch)
2016-12-02 09:18 PST, JF Bastien 		jfbastien: commit-queue+
DetailsFormatted DiffDiff
patch (181.60 KB, patch)
2016-12-02 09:26 PST, JF Bastien 		commit-queue: commit-queue-
DetailsFormatted DiffDiff
patch (181.55 KB, patch)
2016-12-02 09:30 PST, JF Bastien 		no flags
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
JF Bastien
Comment 1 2016-12-02 09:18:12 PST
Created attachment 295951 [details] patch I had to merge around Keith's FP op patch. My local build seems to work. I'll investigate the failure later today, gotta run now.
WebKit Commit Bot
Comment 2 2016-12-02 09:20:38 PST
This patch modifies one of the wasm.json files. Please ensure that any changes in one have been mirrored to the other. You can find the wasm.json files at "Source/JavaScriptCore/wasm/wasm.json" and "JSTests/wasm/wasm.json".
JF Bastien
Comment 3 2016-12-02 09:24:07 PST
Comment on attachment 295951 [details] patch cq+ since the public build isn't broken, but some internal one is.
JF Bastien
Comment 4 2016-12-02 09:26:04 PST
Created attachment 295952 [details] patch Update "reviewed by".
Mark Lam
Comment 5 2016-12-02 09:27:25 PST
(In reply to comment #4) > Created attachment 295952 [details] > patch > > Update "reviewed by". You don't need a review to roll out a broken patch (suspected or otherwise).
WebKit Commit Bot
Comment 6 2016-12-02 09:28:42 PST
Comment on attachment 295952 [details] patch Rejecting attachment 295952 [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', 'validate-changelog', '--check-oops', '--non-interactive', 295952, '--port=mac']" exit_code: 1 cwd: /Volumes/Data/EWS/WebKit /Volumes/Data/EWS/WebKit/JSTests/ChangeLog neither lists a valid reviewer nor contains the string "Unreviewed" or "Rubber stamp" (case insensitive). Full output: http://webkit-queues.webkit.org/results/2607884
Mark Lam
Comment 7 2016-12-02 09:29:42 PST
Comment on attachment 295952 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=295952&action=review > JSTests/ChangeLog:6 > + Reviewed by NOBODY (fix build break). Change this to "Not reviewed" and you should be fine. Or I can review it.
JF Bastien
Comment 8 2016-12-02 09:30:36 PST
Created attachment 295953 [details] patch "unreviewed"
Keith Miller
Comment 9 2016-12-02 09:36:19 PST
Comment on attachment 295953 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=295953&action=review > JSTests/ChangeLog:-70 > - WebAssembly: update binary format to 0xD version > - https://bugs.webkit.org/show_bug.cgi?id=164724 > - > - Reviewed by Saam Barati. > - > - As described in the following PR: https://github.com/WebAssembly/design/pull/836 > - > - * wasm/Builder.js: > - (const._normalizeFunctionSignature): > - * wasm/Builder_WebAssemblyBinary.js: > - (const.emitters.Type): > - (const.emitters.Code): > - * wasm/LowLevelBinary.js: > - (export.default.LowLevelBinary.prototype.block_type): > - (export.default.LowLevelBinary.prototype.inline_signature_type): Deleted. > - * wasm/WASM.js: > - * wasm/js-api/test_basic_api.js: > - * wasm/self-test/test_BuilderWebAssembly.js: > - (EmptyModule): > - (CustomSection): > - * wasm/self-test/test_WASM.js: > - * wasm/wasm.json: > - > -2016-11-30 JF Bastien <jfbastien@apple.com> > - I wouldn't delete this, we keep the changelog as a historical record of everything that's been committed in the past.
WebKit Commit Bot
Comment 10 2016-12-02 09:55:56 PST
Comment on attachment 295953 [details] patch Clearing flags on attachment: 295953 Committed r209242: <http://trac.webkit.org/changeset/209242>
WebKit Commit Bot
Comment 11 2016-12-02 09:56:00 PST
All reviewed patches have been landed. Closing bug.
JF Bastien
Comment 12 2016-12-02 11:58:07 PST
> I wouldn't delete this, we keep the changelog as a historical record of > everything that's been committed in the past. Ugh my bad, I didn't think about that bit and assumed the git hooks would take care of it. I can put it back when I un-revert the revert.
Note You need to log in before you can comment on or make changes to this bug.