Bug 194917 - build-dumprendertree, build-webkittestrunner: Build WebCore test support libraries
Summary: build-dumprendertree, build-webkittestrunner: Build WebCore test support libr...
Status: NEW
Alias: None
Product: WebKit
Classification: Unclassified
Component: Tools / Tests (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Jonathan Bedard
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-02-21 13:54 PST by Jonathan Bedard
Modified: 2022-02-09 10:14 PST (History)
4 users (show)

See Also:


Attachments
Patch (2.40 KB, patch)
2019-02-21 13:55 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff
Patch (2.65 KB, patch)
2019-02-22 08:57 PST, Jonathan Bedard
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Jonathan Bedard 2019-02-21 13:54:19 PST
build-dumprendertree and build-webkittestrunner should both build WebCore test support libraries.
Comment 1 Jonathan Bedard 2019-02-21 13:55:02 PST
<rdar://problem/48278600>
Comment 2 Jonathan Bedard 2019-02-21 13:55:46 PST
Created attachment 362640 [details]
Patch
Comment 3 Daniel Bates 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?
Comment 4 Daniel Bates 2019-02-21 16:53:15 PST
Why change directories?
Comment 5 Alexey Proskuryakov 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.
Comment 6 Jonathan Bedard 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."
Comment 7 Daniel Bates 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.
Comment 8 Daniel Bates 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.
Comment 9 Daniel Bates 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.
Comment 10 Daniel Bates 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.
Comment 11 Jonathan Bedard 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.
Comment 12 Daniel Bates 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!
Comment 13 Daniel Bates 2019-02-22 08:56:52 PST
This Bugzilla bug is invalid. The radar bug is valid.
Comment 14 Jonathan Bedard 2019-02-22 08:57:54 PST
Created attachment 362724 [details]
Patch