Bug 26340 - [Qt] Build break introduced by r44550
Summary: [Qt] Build break introduced by r44550
Status: RESOLVED DUPLICATE of bug 26293
Alias: None
Product: WebKit
Classification: Unclassified
Component: JavaScriptCore (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC All
: P1 Blocker
Assignee: Joseph Ligman
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2009-06-11 20:04 PDT by Laszlo Gombos
Modified: 2009-06-15 04:58 PDT (History)
5 users (show)

See Also:


Attachments
Add JSONObject.cpp to LUT files for JavaScripCore (990 bytes, patch)
2009-06-11 20:08 PDT, Laszlo Gombos
no flags Details | Formatted Diff | Diff
Fix Qt build break by rename JSONObject in inspector (32.92 KB, patch)
2009-06-12 09:50 PDT, Joseph Ligman
no flags Details | Formatted Diff | Diff
Fix Qt build break by rename JSONObject in inspector (31.67 KB, patch)
2009-06-12 11:27 PDT, Joseph Ligman
no flags Details | Formatted Diff | Diff
Fix Qt build break by rename JSONObject in inspector (35.57 KB, patch)
2009-06-13 07:40 PDT, Joseph Ligman
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Laszlo Gombos 2009-06-11 20:04:25 PDT
I found two problems:
 1. JavaScriptCore/runtime/JSONObject.cpp needs to be listed as a LUT_FILE as well
 2. name collision between WebCore/inspector/JSONObject.cpp and the newly created JavaScriptCore/runtime/JSONObject.cpp (all .o's are generated in the same dir for QtWebKit)

I was not sure how to resolve 2. I few ideas:
 - rename one of the JSONObject.cpp (e.g. to InspectorJSONObject.cpp)
 - create JavaScriptCore library first and and link it to WebCore to avoid collision

Will submit patch for 1. (LUT_FILE); this should at least fix the standalone jsc build.
Comment 1 Laszlo Gombos 2009-06-11 20:08:14 PDT
Created attachment 31190 [details]
Add JSONObject.cpp to LUT files for JavaScripCore
Comment 2 Jan Alonzo 2009-06-12 05:17:00 PDT
Comment on attachment 31190 [details]
Add JSONObject.cpp to LUT files for JavaScripCore

> --- JavaScriptCore/JavaScriptCore.pri	(revision 44621)
> +++ JavaScriptCore/JavaScriptCore.pri	(working copy)
> @@ -69,6 +69,7 @@ include(pcre/pcre.pri)
>  
>  LUT_FILES += \
>      runtime/DatePrototype.cpp \
> +    runtime/JSONObject.cpp \
>      runtime/NumberConstructor.cpp \
>      runtime/StringPrototype.cpp \
>      runtime/ArrayPrototype.cpp \

Looks fine. r=me.
Comment 3 Laszlo Gombos 2009-06-12 08:02:19 PDT
Comment on attachment 31190 [details]
Add JSONObject.cpp to LUT files for JavaScripCore

Landed as http://trac.webkit.org/changeset/44623.
Comment 4 Laszlo Gombos 2009-06-12 08:03:09 PDT
Based on some discussions on #webkit the proposal to resolve the remaining build break is to rename WebCore/inspector/JSONObject.cpp to InspectorJSONObject.cpp (and change the class name as well).
Comment 5 Joseph Ligman 2009-06-12 09:50:09 PDT
Created attachment 31199 [details]
Fix Qt build break by rename JSONObject in inspector

I don't have all the ports so was unable to verify all builds.
Comment 6 Joseph Ligman 2009-06-12 11:27:55 PDT
Created attachment 31204 [details]
Fix Qt build break by rename JSONObject in inspector


There was a problem with the ChangeLog in the previous patch. 
I don't have all the ports so I am unable to verify all builds.
Comment 7 Dimitri Glazkov (Google) 2009-06-12 11:33:00 PDT
Did you use svn mv for the file move?
Comment 8 Joseph Ligman 2009-06-12 13:51:23 PDT
Sorry I did not. I can do that, but maybe pfeldman will have some other comments. 
Comment 9 Jan Alonzo 2009-06-12 15:30:43 PDT
Comment on attachment 31190 [details]
Add JSONObject.cpp to LUT files for JavaScripCore

Clearing review flag as the patch has landed.
Comment 10 Joseph Ligman 2009-06-13 07:40:41 PDT
Created attachment 31235 [details]
Fix Qt build break by rename JSONObject in inspector

This one uses svn move instead of just add and delete
Comment 11 Timothy Hatcher 2009-06-13 16:08:08 PDT
Now that I look at JSONObject, it is just to similar to ScriptObject. I think it can go, and just make ScriptObject support JSON stringify and creation from a JSON string. Dimitri, Pavel, do you think that would work?
Comment 12 Timothy Hatcher 2009-06-13 16:45:02 PDT

*** This bug has been marked as a duplicate of 26293 ***
Comment 13 Pavel Feldman 2009-06-14 00:48:16 PDT
(In reply to comment #11)
> Now that I look at JSONObject, it is just to similar to ScriptObject. I think
> it can go, and just make ScriptObject support JSON stringify and creation from
> a JSON string. Dimitri, Pavel, do you think that would work?
> 

I agree that there is a redundancy here. However, I am not sure that Script* is now exactly what we need, so we might need to tweak them a bit. To reduce the number of the moving parts, I'd suggest that this one is fixed as suggested (via renaming Inspector's one to a more specific name) now. In the meanwhile, I would like to learn more on the new stringify API in WebKit and find a good way of Script*, new JSON.stringify and Chromium's JSON capabilities playing together nicely.
Comment 14 Dimitri Glazkov (Google) 2009-06-14 10:39:09 PDT
I am fine with Pavel's opinion. I kinda like Timothy's suggestion though. Color me ambivalent.
Comment 15 Simon Hausmann 2009-06-15 04:52:24 PDT
Comment on attachment 31235 [details]
Fix Qt build break by rename JSONObject in inspector

Thanks Joe!
Comment 16 Simon Hausmann 2009-06-15 04:54:10 PDT
Comment on attachment 31235 [details]
Fix Qt build break by rename JSONObject in inspector

Landed in 44682. Clearning review but leaving the bug open for further discussion around the json classes. Pavel, Timothy, do you prefer a separate bugzilla entry for that?
Comment 17 Tor Arne Vestbø 2009-06-15 04:58:29 PDT
(In reply to comment #16)
> (From update of attachment 31235 [details] [review])
> Landed in 44682. Clearning review but leaving the bug open for further
> discussion around the json classes. Pavel, Timothy, do you prefer a separate
> bugzilla entry for that?

I think discussions are in https://bugs.webkit.org/show_bug.cgi?id=26293