Bug 74972 - Enable the [Supplemental] IDL on Gtk
Summary: Enable the [Supplemental] IDL on Gtk
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebCore Misc. (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Kentaro Hara
URL:
Keywords:
Depends on:
Blocks: 72138
  Show dependency treegraph
 
Reported: 2011-12-20 16:20 PST by Kentaro Hara
Modified: 2011-12-27 14:32 PST (History)
7 users (show)

See Also:


Attachments
WIP patch to see if build passes (83.86 KB, patch)
2011-12-20 16:32 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
Patch (83.97 KB, patch)
2011-12-21 01:45 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
WIP patch to see if build succeeds (84.01 KB, patch)
2011-12-21 17:28 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
WIP patch to see if build succeeds (84.02 KB, patch)
2011-12-21 17:36 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff
WIP patch to see if build succeeds (30.50 KB, patch)
2011-12-26 06:40 PST, Kentaro Hara
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Kentaro Hara 2011-12-20 16:20:34 PST
We have enabled the [Supplemental] IDL on Chromium and AppleWebKit, and are planning to enable it on all platforms (Meta bug 72138). In this bug, we enable it on Gtk.
Comment 1 Kentaro Hara 2011-12-20 16:32:30 PST
Created attachment 120112 [details]
WIP patch to see if build passes
Comment 2 Martin Robinson 2011-12-20 16:40:33 PST
Comment on attachment 120112 [details]
WIP patch to see if build passes

View in context: https://bugs.webkit.org/attachment.cgi?id=120112&action=review

> GNUmakefile.am:54
> +BINDING_IDLS :=

Would prefer something a bit more descriptive. Would dom_binding_idls be more accurate?
Comment 3 Kentaro Hara 2011-12-20 16:42:59 PST
(In reply to comment #2)
> (From update of attachment 120112 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=120112&action=review
> 
> > GNUmakefile.am:54
> > +BINDING_IDLS :=
> 
> Would prefer something a bit more descriptive. Would dom_binding_idls be more accurate?

Sure, I'll fix it. Since this patch is too big, I'll land it step by step after I confirm that it passes all builds.
Comment 4 Collabora GTK+ EWS bot 2011-12-21 01:30:12 PST
Comment on attachment 120112 [details]
WIP patch to see if build passes

Attachment 120112 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10991600
Comment 5 Kentaro Hara 2011-12-21 01:45:23 PST
Created attachment 120160 [details]
Patch
Comment 6 Collabora GTK+ EWS bot 2011-12-21 12:47:53 PST
Comment on attachment 120160 [details]
Patch

Attachment 120160 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10999061
Comment 7 Kentaro Hara 2011-12-21 17:28:21 PST
Created attachment 120249 [details]
WIP patch to see if build succeeds
Comment 8 WebKit Review Bot 2011-12-21 17:31:17 PST
Attachment 120249 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
From git://git.webkit.org/WebKit
   e4a13f1..3779062  master     -> origin/master
Partial-rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc ...
Currently at 103476 = e4a13f19647c218b1fc1ce0ecc69c5ed16a4d7f4
r103477 = 3779062662ed4d6798ce8e6d2adb880cc2b2704a
Done rebuilding .git/svn/refs/remotes/origin/master/.rev_map.268f45cc-cd09-0410-ab3c-d52691b4dbfc
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167534 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 9 Kentaro Hara 2011-12-21 17:36:20 PST
Created attachment 120250 [details]
WIP patch to see if build succeeds
Comment 10 WebKit Review Bot 2011-12-21 17:53:38 PST
Attachment 120250 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/update-webkit']" exit_code: 9

Updating OpenSource
First, rewinding head to replay your work on top of it...
Applying: Inform the scrolling coordinator when scrollbar layers come and go
Using index info to reconstruct a base tree...
<stdin>:474806: trailing whitespace.
        [Chromium] DatabaseTrackerChromium: iterating DatabaseSet races with Database disposal on worker thread 
<stdin>:474827: trailing whitespace.
        Nothing to test, just removing redundant code. Correct behavior tested by 
<stdin>:475346: trailing whitespace.
    
warning: 3 lines add whitespace errors.
Falling back to patching base and 3-way merge...
warning: too many files (created: 167534 deleted: 3), skipping inexact rename detection
Auto-merging LayoutTests/ChangeLog
CONFLICT (content): Merge conflict in LayoutTests/ChangeLog
Auto-merging LayoutTests/platform/chromium/test_expectations.txt
CONFLICT (content): Merge conflict in LayoutTests/platform/chromium/test_expectations.txt
Auto-merging Source/WebCore/ChangeLog
CONFLICT (content): Merge conflict in Source/WebCore/ChangeLog
Auto-merging Source/WebCore/page/ScrollingCoordinator.h
CONFLICT (content): Merge conflict in Source/WebCore/page/ScrollingCoordinator.h
Auto-merging Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
CONFLICT (content): Merge conflict in Source/WebCore/page/mac/ScrollingCoordinatorMac.mm
Auto-merging Source/WebCore/platform/ScrollView.cpp
Auto-merging Source/WebCore/rendering/RenderLayerCompositor.cpp
CONFLICT (content): Merge conflict in Source/WebCore/rendering/RenderLayerCompositor.cpp
Auto-merging Source/WebKit2/ChangeLog
CONFLICT (content): Merge conflict in Source/WebKit2/ChangeLog
Auto-merging Tools/ChangeLog
CONFLICT (content): Merge conflict in Tools/ChangeLog
Auto-merging Tools/Scripts/build-webkit
Auto-merging Tools/Scripts/webkitdirs.pm
CONFLICT (content): Merge conflict in Tools/Scripts/webkitdirs.pm
Failed to merge in the changes.
Patch failed at 0001 Inform the scrolling coordinator when scrollbar layers come and go

When you have resolved this problem run "git rebase --continue".
If you would prefer to skip this patch, instead run "git rebase --skip".
To restore the original branch and stop rebasing run "git rebase --abort".

rebase refs/remotes/origin/master: command returned error: 1

Died at Tools/Scripts/update-webkit line 158.


If any of these errors are false positives, please file a bug against check-webkit-style.
Comment 11 Gustavo Noronha (kov) 2011-12-21 23:06:41 PST
Comment on attachment 120250 [details]
WIP patch to see if build succeeds

Attachment 120250 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/10996336
Comment 12 Kentaro Hara 2011-12-26 06:40:05 PST
Created attachment 120554 [details]
WIP patch to see if build succeeds
Comment 13 Adam Barth 2011-12-26 11:43:58 PST
It builds!
Comment 14 Kentaro Hara 2011-12-26 16:06:39 PST
[Summary]

- It builds, but the last patch is a first step for the ideal approach.
- I would like to commit the last patch, and then refactor GNUmakefile.list.am for the ideal approach.

[Details]

Current GNUmakefile.list.am:

    webcore_built_sources += \
        DerivedSources/WebCore/JSA.cpp \
        DerivedSources/WebCore/JSA.h \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....


The last patch:

    webcore_built_sources += \
        DerivedSources/WebCore/JSA.cpp \
        DerivedSources/WebCore/JSA.h \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....


The ideal approach (which I have tried but failed in the previous patches): 

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


As you can see, the last patch is verbose since we need to describe *.idl, JS*.h and JS*.cpp in GNUmakefile.list.am. I have tried to build the ideal approach in my local GTK environment for one week, but the build is still failing, as described below. Anyway, I think that the last patch is a first step for the ideal approach. We can remove verbose JS*.h and JS*.cpp from GNUmakefile.list.am incrementally after committing it.


[Build failure]

Although all compiles succeed, the link fails: http://queues.webkit.org/results/10996336

[I] This succeeds (i.e. move one file to dom_binding_idls):

    webcore_built_sources += \
        DerivedSources/WebCore/JSB.cpp \
        DerivedSources/WebCore/JSB.h \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


[II] This also succeeds (i.e. move two files to dom_binding_idls):

    webcore_built_sources += \
        DerivedSources/WebCore/JSC.cpp \
        DerivedSources/WebCore/JSC.h \
        ....

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))

[III] But this fails in the link process (i.e. move all files to dom_binding_idls):

    webcore_built_sources +=

    dom_binding_idls += \
        WebCore/aaa/A.idl \
        WebCore/bbb/B.idl \
        WebCore/ccc/C.idl \
        ....

    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.cpp, $(basename $(notdir $(dom_binding_idls))))
    webcore_built_sources += $(patsubst %, DerivedSources/WebCore/JS%.h, $(basename $(notdir $(dom_binding_idls))))


In [I], [II] and [III], I confirmed that the set of generated JS*.h and JS*.cpp and their contents are exactly same. Although http://queues.webkit.org/results/10996336 indicates that JS*.h and JS*.cpp are compiled but not linked (I don't know why), interestingly, this link failure happens for (not all but) a part of JS*.h and JS*.cpp deterministically. I investigated the common points of IDLs from which those failing JS*.h and JS*.cpp are generated, but could not find anything particular. I am not sure but this might be a problem of the link order.

### Another difficulty is that it takes two hours for each clobber build in my local GTK environemnt:-)
Comment 15 Adam Barth 2011-12-27 09:53:28 PST
How bizarre.  The approach in your patch is how a number of the other build systems work, so it seems like a fine intermediate state.
Comment 16 WebKit Review Bot 2011-12-27 14:32:02 PST
Comment on attachment 120554 [details]
WIP patch to see if build succeeds

Clearing flags on attachment: 120554

Committed r103729: <http://trac.webkit.org/changeset/103729>
Comment 17 WebKit Review Bot 2011-12-27 14:32:09 PST
All reviewed patches have been landed.  Closing bug.