Bug 194917

Summary: build-dumprendertree, build-webkittestrunner: Build WebCore test support libraries
Product: WebKit Reporter: Jonathan Bedard <jbedard>
Component: Tools / TestsAssignee: Jonathan Bedard <jbedard>
Status: NEW    
Severity: Normal CC: ap, dbates, lforschler, webkit-bug-importer
Priority: P2 Keywords: InRadar
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
Attachments:
Description Flags
Patch
none
Patch none

Jonathan Bedard
Reported 2019-02-21 13:54:19 PST
build-dumprendertree and build-webkittestrunner should both build WebCore test support libraries.
Attachments
Patch (2.40 KB, patch)
2019-02-21 13:55 PST, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Patch (2.65 KB, patch)
2019-02-22 08:57 PST, Jonathan Bedard 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Jonathan Bedard
Comment 1 2019-02-21 13:55:02 PST
<rdar://problem/48278600>
Jonathan Bedard
Comment 2 2019-02-21 13:55:46 PST
Created attachment 362640 [details] Patch
Daniel Bates
Comment 3 2019-02-21 16:52:13 PST
Patch doesn’t look right. Why only for Cocoa platforms? Why doing it after we build drt or wktr?
Daniel Bates
Comment 4 2019-02-21 16:53:15 PST
Why change directories?
Alexey Proskuryakov
Comment 5 2019-02-21 16:54:35 PST
Why is this useful? As these libraries are built together with WebCore, I expect them to always be present. I’m probably missing some context here.
Jonathan Bedard
Comment 6 2019-02-21 17:12:39 PST
(In reply to Daniel Bates from comment #3) > Patch doesn’t look right. Why only for Cocoa platforms? Why doing it after > we build drt or wktr? Read the whole file. Other platforms, to quote the existing comment, "# GTK+, WPE, PlayStation, and Windows build everything in one shot. No need to build anything here."
Daniel Bates
Comment 7 2019-02-21 18:59:31 PST
(In reply to Daniel Bates from comment #3) > Why only for Cocoa platforms? By comment 6. > Why doing it after we build drt or wktr? Still unanswered.
Daniel Bates
Comment 8 2019-02-21 19:00:30 PST
(In reply to Daniel Bates from comment #4) > Why change directories? Okay, read more of the existing code. Answer, that’s how it’s done. The build...() seems to assume you’re in the same directory as the Xcode project.
Daniel Bates
Comment 9 2019-02-21 19:01:03 PST
(In reply to Alexey Proskuryakov from comment #5) > Why is this useful? As these libraries are built together with WebCore, I > expect them to always be present. > > I’m probably missing some context here. Read radar.
Daniel Bates
Comment 10 2019-02-21 19:03:02 PST
Actually Alexy’s question adds to my feeling that this patch doesn’t look right because this problem is only hit for Apple engineers hence the radar description. So, this fix seems misplaced.
Jonathan Bedard
Comment 11 2019-02-22 08:31:59 PST
So, either something very similar to this patch is the fix, or this isn't a bug. We run build-webkittestrunner right before running tests, by default. We don't make an attempt to build the entire WebKit stack, we never have nor should we (A no-op build of WebKit through build-webkit actually takes ~30 seconds-1 minute). Apparently there is a workflow where WebCore can be built without building the WebCore test support libraries.
Daniel Bates
Comment 12 2019-02-22 08:54:51 PST
(In reply to Jonathan Bedard from comment #11) > So, either something very similar to this patch is the fix, or this isn't a > bug. > > We run build-webkittestrunner right before running tests, by default. We > don't make an attempt to build the entire WebKit stack, we never have nor > should we (A no-op build of WebKit through build-webkit actually takes ~30 > seconds-1 minute). > Apparently there is a workflow where WebCore can be built > without building the WebCore test support libraries. This is the bug! It’s an Apple Internal thing. Fix this!
Daniel Bates
Comment 13 2019-02-22 08:56:52 PST
This Bugzilla bug is invalid. The radar bug is valid.
Jonathan Bedard
Comment 14 2019-02-22 08:57:54 PST
Created attachment 362724 [details] Patch
Note You need to log in before you can comment on or make changes to this bug.