Bug 51806

Summary: "Tool/Scripts/run-launcher --gtk" stopped working after 74855
Product: WebKit Reporter: Koan-Sin Tan <koansin.tan>
Component: Tools / TestsAssignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Trivial CC: commit-queue, joone, pnormand, xan.lopez
Priority: P2    
Version: 528+ (Nightly build)   
Hardware: PC   
OS: All   
Attachments:
Description Flags
proposed patch
xan.lopez: review-, xan.lopez: commit-queue-
proposed patch none

Koan-Sin Tan
Reported 2011-01-02 19:40:18 PST
"Tool/Scripts/run-launcher --gtk" stopped working after 74855 (JavaScriptCore/ moved to Source/JavaScriptCore) should fix Tools/Scripts/webkitdirs.pm
Attachments
proposed patch (1.13 KB, patch)
2011-01-02 20:51 PST, Koan-Sin Tan 		xan.lopez: review-
xan.lopez: commit-queue-
DetailsFormatted DiffDiff
proposed patch (1.52 KB, patch)
2011-01-03 05:25 PST, Koan-Sin Tan 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Koan-Sin Tan
Comment 1 2011-01-02 20:51:03 PST
Created attachment 77790 [details] proposed patch this is kinda trivial
Xan Lopez
Comment 2 2011-01-03 04:23:05 PST
(In reply to comment #1) > Created an attachment (id=77790) [details] > proposed patch > > this is kinda trivial I must admit I don't get what the patch is supposed to achieve or how it's related to moving JSC to Source/. Can you explain?
Xan Lopez
Comment 3 2011-01-03 04:32:45 PST
(In reply to comment #2) > (In reply to comment #1) > > Created an attachment (id=77790) [details] [details] > > proposed patch > > > > this is kinda trivial > > I must admit I don't get what the patch is supposed to achieve or how it's related to moving JSC to Source/. Can you explain? From talking to Philippe (who also sees the brokeness), it seems that the problem is that this is called with JavaScriptCore as $libraryName, which does not exist now as a directory, so the whole thing will fail. I'm not sure what was the point of doing $dir/$foo/../.libs, but I think just $dir/.libs is OK as you suggest. I wonder if this has changed recently, our solution (and Qt, Elf,...) does not seem so robust anymore.
Xan Lopez
Comment 4 2011-01-03 04:33:21 PST
Comment on attachment 77790 [details] proposed patch The patch is good, but you need to explain a bit more what's going on in the ChangeLog. Thanks!
Philippe Normand
Comment 5 2011-01-03 04:51:01 PST
it used to work before JavascriptCore moved to Source/: /path/to/WebKitBuild/JavascriptCore/../.libs existed but now indeed: /path/to/WebkitBuild/Source/JavascriptCore/../.libs doesn't exist. Using ../../.libs would work but I agree with Xan that the proposed patch is better
Koan-Sin Tan
Comment 6 2011-01-03 04:56:14 PST
(In reply to comment #5) > it used to work before JavascriptCore moved to Source/: > > /path/to/WebKitBuild/JavascriptCore/../.libs existed > > but now indeed: > > /path/to/WebkitBuild/Source/JavascriptCore/../.libs doesn't exist. > > Using ../../.libs would work but I agree with Xan that the proposed patch is better yeah, I don't know why that was "JavascriptCore/../" which doesn't really make sense to me, so i removed it
Xan Lopez
Comment 7 2011-01-03 05:19:11 PST
*** Bug 51816 has been marked as a duplicate of this bug. ***
Koan-Sin Tan
Comment 8 2011-01-03 05:25:25 PST
Created attachment 77804 [details] proposed patch add some explanation as Xan suggested
Xan Lopez
Comment 9 2011-01-03 05:27:12 PST
Comment on attachment 77804 [details] proposed patch Thank you!
WebKit Commit Bot
Comment 10 2011-01-03 05:59:33 PST
The commit-queue encountered the following flaky tests while processing attachment 77804 [details]: http/tests/xmlhttprequest/access-control-preflight-credential-async.html bug 51817 (author: jchaffraix@webkit.org) The commit-queue is continuing to process your patch.
WebKit Commit Bot
Comment 11 2011-01-03 06:00:34 PST
Comment on attachment 77804 [details] proposed patch Clearing flags on attachment: 77804 Committed r74889: <http://trac.webkit.org/changeset/74889>
WebKit Commit Bot
Comment 12 2011-01-03 06:00:41 PST
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.