Bug 94290 – Pre-process CSSGrammar.y before running through bison.

• Status: RESOLVED
• Resolution: FIXED
• Component: New Bugs

[Shane Stephens]

[WIP] Pre-process CSSGrammar.y before running through bison.

[Shane Stephens]

Created attachment 158988 [details]
Patch

[Peter Beverloo (cr-android ews)]

Comment on attachment 158988 [details]
Patch

Attachment 158988 [details] did not pass cr-android-ews (chromium-android):
Output: http://queues.webkit.org/results/13506976

[Tony Chang]

Comment on attachment 158988 [details]
Patch

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

> Source/WebCore/ChangeLog:8
> +        * DerivedSources.make:

Chromium doens't use DerivedSources.make.  You'll have to update WebCore.gyp/Webcore.gyp too.

> Source/WebCore/ChangeLog:11
> +        * css/makegrammar.pl:

Did you mean to include makegrammar.pl here? I don't see any changes to this file.

[Alexey Proskuryakov]

Neither the patch nor the bug explain the rationale. Please do provide it.

[Tony Chang]

In https://bugs.webkit.org/show_bug.cgi?id=94099#c4 , Antti asked that all code related to CSS hierarchies be wrapped with #ifs.

Antti, I assume you wanted this for the changes to CSSGrammar.y too, right? Is this what you had in mind?

[Antti Koivisto]

Right, I think it would be useful to have feature defines in grammar too.

[Pablo Flouret]

Created attachment 167800 [details]
Patch

[Pablo Flouret]

(In reply to comment #7)
> Created an attachment (id=167800) [details]
> Patch

I could only test this on the mac and chromium ports, so if this works correctly on the first try i'll probably be very surprised :-)

If there's someone from a port not on the cc list that i should cc, let me know.

[Early Warning System Bot]

Comment on attachment 167800 [details]
Patch

Attachment 167800 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14245172

[Early Warning System Bot]

Comment on attachment 167800 [details]
Patch

Attachment 167800 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14229665

[Tony Chang]

Comment on attachment 167800 [details]
Patch

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

Overall, looks great once the ews bots all go green.  I think it would also be OK to use makegrammar.pl in the Chromium build, but this is also fine.

> Source/WebCore/WebCore.gyp/WebCore.gyp:1082
> +            '../css/CSSGrammar.y.in',

Shouldn't CSSGrammar.y.includes be an input too?

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:50
> +inputFile = sys.argv[1]
> +outputDir = sys.argv[2]

Nit: python style variable naming is input_file.

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:54
> +    # featureFlags = ["-D", "ENABLE_FEATURE1=1", "-D", "ENABLE_FEATURE2=0", ...]

Nit: Remove testing comment?

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:57
> +gccExe = "gcc"

gcc_exe and other variables below.

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:61
> +(inputRoot, inputExt) = os.path.splitext(inputFile)

Nit: () on the left side aren't necessary.

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:64
> +output = subprocess.Popen(['ls', '-l'], stdout=subprocess.PIPE).communicate()[0]

Remove testing command?

[Pablo Flouret]

Created attachment 167817 [details]
Patch

Trying to fix qt/gtk build errors.

[Pablo Flouret]

If i understand my own patch correctly, the targets don't really depend on CSSGrammar.y.includes, so changing that file won't trigger a re-generation of CSSGrammar.y, which i imagine could be annoying.

If anyone knows how to do this for each build system i can modify the patch, or do it on a subsequent one.

(PS: i hate build systems)

[Pablo Flouret]

(In reply to comment #11)
> (From update of attachment 167800 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=167800&action=review
> 
> Overall, looks great once the ews bots all go green.  I think it would also be OK to use makegrammar.pl in the Chromium build, but this is also fine.

I thought about using makegrammar.pl here as well, but it would need some shoehorning. The python way goes a bit better with the gyp ways, i think.

> > Source/WebCore/WebCore.gyp/WebCore.gyp:1082
> > +            '../css/CSSGrammar.y.in',
> 
> Shouldn't CSSGrammar.y.includes be an input too?

Yeah, i guess, it was easier like this in terms of parsing the arguments in the script, but i could make that more robust. Should i make the script more generic to handle multiple .y.in files, or should that bridge be crossed if it's ever needed? (I imagine XPathGrammar.y doesn't see much action?)

> > Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:50
> > +inputFile = sys.argv[1]
> > +outputDir = sys.argv[2]
> 
> Nit: python style variable naming is input_file.

All the other action_* and rule_* scripts use this style, should i change it anyway?

[Tony Chang]

(In reply to comment #14)
> (In reply to comment #11)
> > Shouldn't CSSGrammar.y.includes be an input too?
> 
> Yeah, i guess, it was easier like this in terms of parsing the arguments in the script, but i could make that more robust. Should i make the script more generic to handle multiple .y.in files, or should that bridge be crossed if it's ever needed? (I imagine XPathGrammar.y doesn't see much action?)

I wouldn't bother making the script handle multiple .y.in files. Like you said, this stuff doesn't change that often.

> > > Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:50
> > > +inputFile = sys.argv[1]
> > > +outputDir = sys.argv[2]
> > 
> > Nit: python style variable naming is input_file.
> 
> All the other action_* and rule_* scripts use this style, should i change it anyway?

The other scripts were written before we had much python code in WebKit and had converged on PEP8 style.  We should try to use PEP8 style in new files.

[Pablo Flouret]

Created attachment 167860 [details]
Patch

[Tony Chang]

Comment on attachment 167860 [details]
Patch

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

LGTM assuming all the ews bots go green.  The feedback on the python script are minor, so feel free to ignore.

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:62
> +if len(arguments) == 0:
> +    exit_with_usage(1)

Nit: "if not len(arguments):" would be more like WebKit style.

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:76
> +gcc_exe = "gcc"
> +if "CC" in os.environ:
> +    gcc_exe = os.environ["CC"]

Nit: This could just be gcc_exe = os.environ.get("CC", "gcc").

> Source/WebCore/WebCore.gyp/scripts/action_preprocessgrammar.py:86
> +output_file = open(os.path.join(output_dir, os.path.basename(input_root)), "w")

Nit: Using 'with' for files would automatically close it for you.  It's a bit more idiomatic.

[Pablo Flouret]

Looks like they're all green. If you want we can wait overnight to see if anyone from the other ports has any comments. Otherwise cq+ away.

[WebKit Review Bot]

Comment on attachment 167860 [details]
Patch

Clearing flags on attachment: 167860

Committed r130937: <http://trac.webkit.org/changeset/130937>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Csaba Osztrogonác]

(In reply to comment #19)
> (From update of attachment 167860 [details])
> Clearing flags on attachment: 167860
> 
> Committed r130937: <http://trac.webkit.org/changeset/130937>

It broke the Qt Win build:
	perl C:/WebKitBuildSlave/szeged-windows-1/qt-windows-32bit-release/build/Source/WebCore/css/makegrammar.pl --grammar C:\WebKitBuildSlave\szeged-windows-1\qt-windows-32bit-release\build\Source\WebCore\css\CSSGrammar.y.in --outputDir generated --extraDefines "UNICODE WIN32 ENABLE_3D_RENDERING=1 ENABLE_BLOB=1 ENABLE_CHANNEL_MESSAGING=1 ENABLE_CSS_BOX_DECORATION_BREAK=1 ENABLE_CSS_COMPOSITING=1 ENABLE_CSS_EXCLUSIONS=1 ENABLE_CSS_FILTERS=1 ENABLE_CSS_IMAGE_SET=1 ENABLE_CSS_REGIONS=1 ENABLE_CSS_STICKY_POSITION=1 ENABLE_DATALIST_ELEMENT=1 ENABLE_DETAILS_ELEMENT=1 ENABLE_FAST_MOBILE_SCROLLING=1 ENABLE_FILTERS=1 ENABLE_FTPDIR=1 ENABLE_FULLSCREEN_API=1 ENABLE_GESTURE_EVENTS=1 ENABLE_ICONDATABASE=1 ENABLE_IFRAME_SEAMLESS=1 ENABLE_INPUT_TYPE_COLOR=1 ENABLE_INSPECTOR=1 ENABLE_INSPECTOR_SERVER=1 ENABLE_JAVASCRIPT_DEBUGGER=1 ENABLE_LEGACY_NOTIFICATIONS=1 ENABLE_LEGACY_VIEWPORT_ADAPTION=1 ENABLE_LEGACY_VENDOR_PREFIXES=1 ENABLE_METER_ELEMENT=1 ENABLE_NOTIFICATIONS=1 ENABLE_PAGE_VISIBILITY_API=1 ENABLE_PROGRESS_ELEMENT=1 ENABLE_REQUEST_ANIMATION_FRAME=1 ENABLE_SHARED_WORKERS=1 ENABLE_SMOOTH_SCROLLING=1 ENABLE_SQL_DATABASE=1 ENABLE_SVG=1 ENABLE_SVG_FONTS=1 ENABLE_TOUCH_ADJUSTMENT=1 ENABLE_TOUCH_EVENTS=1 ENABLE_WEB_SOCKETS=1 ENABLE_WEB_TIMING=1 ENABLE_WORKERS=1 WTF_USE_TILED_BACKING_STORE=1 HAVE_QTQUICK=1 HAVE_QTPRINTSUPPORT=1 HAVE_QSTYLE=1 HAVE_QTTESTLIB=1 ENABLE_NETSCAPE_PLUGIN_API=1 PLUGIN_ARCHITECTURE_UNSUPPORTED=1 WTF_USE_3D_GRAPHICS=1 ENABLE_WEBGL=1 ENABLE_CSS_SHADERS=1 ENABLE_ORIENTATION_EVENTS=1 ENABLE_DEVICE_ORIENTATION=1 ENABLE_TOUCH_SLIDER=1 ENABLE_ACCELERATED_2D_CANVAS=0 ENABLE_ANIMATION_API=0 ENABLE_BATTERY_STATUS=0 ENABLE_CSP_NEXT=0 ENABLE_CSS_GRID_LAYOUT=0 ENABLE_CSS_HIERARCHIES=0 ENABLE_CSS_IMAGE_ORIENTATION=0 ENABLE_CSS_IMAGE_RESOLUTION=0 ENABLE_CSS_VARIABLES=0 ENABLE_CSS3_TEXT_DECORATION=0 ENABLE_DASHBOARD_SUPPORT=0 ENABLE_DATAGRID=0 ENABLE_DATA_TRANSFER_ITEMS=0 ENABLE_DIRECTORY_UPLOAD=0 ENABLE_DOWNLOAD_ATTRIBUTE=0 ENABLE_FILE_SYSTEM=0 ENABLE_GAMEPAD=0 ENABLE_GEOLOCATION=0 ENABLE_HIGH_DPI_CANVAS=0 ENABLE_INDEXED_DATABASE=0 ENABLE_INPUT_SPEECH=0 ENABLE_INPUT_TYPE_DATE=0 ENABLE_INPUT_TYPE_DATETIME=0 ENABLE_INPUT_TYPE_DATETIMELOCAL=0 ENABLE_INPUT_TYPE_MONTH=0 ENABLE_INPUT_TYPE_TIME=0 ENABLE_INPUT_TYPE_WEEK=0 ENABLE_LEGACY_CSS_VENDOR_PREFIXES=0 ENABLE_LINK_PREFETCH=0 ENABLE_LINK_PRERENDER=0 ENABLE_MATHML=0 ENABLE_MEDIA_SOURCE=0 ENABLE_MEDIA_STATISTICS=0 ENABLE_MEDIA_STREAM=0 ENABLE_MHTML=0 ENABLE_MICRODATA=0 ENABLE_MUTATION_OBSERVERS=0 ENABLE_NAVIGATOR_CONTENT_UTILS=0 ENABLE_NETWORK_INFO=0 ENABLE_QUOTA=0 ENABLE_SCRIPTED_SPEECH=0 ENABLE_SHADOW_DOM=0 ENABLE_STYLE_SCOPED=0 ENABLE_SVG_DOM_OBJC_BINDINGS=0 ENABLE_TEXT_AUTOSIZING=0 ENABLE_TEXT_NOTIFICATIONS_ONLY=0 ENABLE_TOUCH_ICON_LOADING=0 ENABLE_UNDO_MANAGER=0 ENABLE_VIBRATION=0 ENABLE_VIDEO=0 ENABLE_VIDEO_TRACK=0 ENABLE_WEB_AUDIO=0 ENABLE_XSLT=0" --symbolsPrefix cssyy
The system cannot find the path specified.
generated\\CSSGrammar.y:86.1: syntax error, unexpected end of file
Died at C:/WebKitBuildSlave/szeged-windows-1/qt-windows-32bit-release/build/Source/WebCore/css/makegrammar.pl line 88.

[WebKit Review Bot]

Re-opened since this is blocked by bug 98946

[Tony Chang]

Actually, I think this is just using the wrong slash for paths.  Let me try to fix.

[Pablo Flouret]

(In reply to comment #23)
> Actually, I think this is just using the wrong slash for paths.  Let me try to fix.

If that's not it then this might do it:

diff --git a/Source/WebCore/css/makegrammar.pl b/Source/WebCore/css/makegrammar.pl
index 32f757e..4b587ea 100644
--- a/Source/WebCore/css/makegrammar.pl
+++ b/Source/WebCore/css/makegrammar.pl
@@ -25,6 +25,7 @@ use Config;
 use File::Basename;
 use File::Spec;
 use Getopt::Long;
+use IO::File;
 
 require Config;
 
@@ -58,21 +59,20 @@ if ($suffix eq ".y.in") {
     my $grammarFileOutPath = File::Spec->join($outputDir, "$filename.y");
     my $featureFlags = "-D " . join(" -D ", split(" ", $extraDefines));
 
-    system($gcc . " -E -P -x c $grammarFilePath $featureFlags > $grammarFileOutPath.processed" );
+    my $processed = new IO::File;
+    open $processed, "$gcc -E -P -x c $grammarFilePath $featureFlags|";
 
     open GRAMMAR, ">$grammarFileOutPath" or die;
     open INCLUDES, "<$grammarFileIncludesPath" or die;
-    open PROCESSED, "<$grammarFileOutPath.processed" or die;
 
     while (<INCLUDES>) {
         print GRAMMAR;
     }
-    while (<PROCESSED>) {
+    while (<$processed>) {
         print GRAMMAR;
     }
 
     close GRAMMAR;
-    unlink("$grammarFileOutPath.processed");
     $grammarFilePath = $grammarFileOutPath;
 }

[Zoltan Arvai]

If it helps here is the content of CSSGrammar.y that read by makegrammar.pl line 88:
https://gist.github.com/3868405

[Tony Chang]

Reverted r130937 for reason:

Breaks Qt build

Committed r130962: <http://trac.webkit.org/changeset/130962>

[Tony Chang]

This is what we learned:

Qt uses --preprocessor \"$${QMAKE_MOC} -E\" in other places that we use the proprocessor.  My last attempt to fix the build (http://trac.webkit.org/changeset/130957) used that, but the output from moc -E is different from gcc (namely, it doesn't maintain leading spaces).

I'm not sure what the right fix for this is.  Is there a way we can get moc to behave more like gcc?  Alternately, maybe we can use cl.exe on Qt Win?

[Pablo Flouret]

(In reply to comment #27)
> This is what we learned:
> 
> Qt uses --preprocessor \"$${QMAKE_MOC} -E\" in other places that we use the proprocessor.  My last attempt to fix the build (http://trac.webkit.org/changeset/130957) used that, but the output from moc -E is different from gcc (namely, it doesn't maintain leading spaces).
> 
> I'm not sure what the right fix for this is.  Is there a way we can get moc to behave more like gcc?  Alternately, maybe we can use cl.exe on Qt Win?

Why did this not fail on chromium win? Is it using cygwin or requiring gcc around?

Alternatively, i could modify the script to do very dumb preprocessing of just #if ENABLE_* directives, since doing anything fancier than that in the .y file would probably break anyway...

[Adam Barth]

This caused trouble for the ChromiumOS build as well.

[Tony Chang]

(In reply to comment #28)
> (In reply to comment #27)
> > This is what we learned:
> > 
> > Qt uses --preprocessor \"$${QMAKE_MOC} -E\" in other places that we use the proprocessor.  My last attempt to fix the build (http://trac.webkit.org/changeset/130957) used that, but the output from moc -E is different from gcc (namely, it doesn't maintain leading spaces).
> > 
> > I'm not sure what the right fix for this is.  Is there a way we can get moc to behave more like gcc?  Alternately, maybe we can use cl.exe on Qt Win?
> 
> Why did this not fail on chromium win? Is it using cygwin or requiring gcc around?

I suspect gcc is installed.

> Alternatively, i could modify the script to do very dumb preprocessing of just #if ENABLE_* directives, since doing anything fancier than that in the .y file would probably break anyway...

I think it would be better to figure out how to use cl.exe on the various Windows platforms, unless the Qt guys have another suggestion.

Adam, do you have a link to the chromiumos failure? I don't see any chromiumos bots on either build.webkit.org or http://build.chromium.org/p/chromium.webkit/

[Adam Barth]

We didn't see the problem until the patch reached the main waterfall:

http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28daisy%29/builds/3657/steps/cbuildbot/logs/stdio

chromeos-chrome-24.0.1292.0_alpha-r1: Traceback (most recent call last):
chromeos-chrome-24.0.1292.0_alpha-r1:   File "scripts/action_preprocessgrammar.py", line 82, in <module>
chromeos-chrome-24.0.1292.0_alpha-r1:     stdout=subprocess.PIPE)
chromeos-chrome-24.0.1292.0_alpha-r1:   File "/usr/lib64/python2.6/subprocess.py", line 623, in __init__
chromeos-chrome-24.0.1292.0_alpha-r1:     errread, errwrite)
chromeos-chrome-24.0.1292.0_alpha-r1:   File "/usr/lib64/python2.6/subprocess.py", line 1141, in _execute_child
chromeos-chrome-24.0.1292.0_alpha-r1:     raise child_exception
chromeos-chrome-24.0.1292.0_alpha-r1: OSError: [Errno 2] No such file or directory
chromeos-chrome-24.0.1292.0_alpha-r1: make: *** [c/Release/obj/gen/webkit/CSSGrammar.y] Error 1
chromeos-chrome-24.0.1292.0_alpha-r1: make: *** Waiting for unfinished jobs....
chromeos-chrome-24.0.1292.0_alpha-r1: emake failed

[Tony Chang]

(In reply to comment #31)
> We didn't see the problem until the patch reached the main waterfall:
> 
> http://build.chromium.org/p/chromium.chromiumos/builders/ChromiumOS%20%28daisy%29/builds/3657/steps/cbuildbot/logs/stdio

Hmm, that error means it didn't find the compiler (it's an arm compile, the lines above use armv7a-cros-linux-gnueabi-g++).  I'm not sure if the CC environment variable was set.  It doesn't look like it, so I'm not sure how make knows to use armv7a-cros-linux-gnueabi-g++.

The other .py scripts used by the chromium build just call the .pl file and the .pl scripts mostly use Source/WebCore/bindings/scripts/preprocessor.pm.  Maybe we can continue to use that method rather than trying to write new code.

[Pablo Flouret]

(In reply to comment #32)
> 
> The other .py scripts used by the chromium build just call the .pl file and the .pl scripts mostly use Source/WebCore/bindings/scripts/preprocessor.pm.  Maybe we can continue to use that method rather than trying to write new code.

applyPreprocessor() essentially does the same that's being done in makegrammar.pl, so it wouldn't solve Qt woes anyway.

I'm wondering if just handling simple #if ENABLE_* directives in makegrammar.pl isn't the right thing to do after all. Anything other than that should probably pass through to the CSSGrammar.y, there would be no need to separate into .y.in and .y.includes, we could get rid of action_preprocessgrammar.py, no fumbling to find a preprocessor in every platform under the sun, etc.

What do you think?

[Simon Hausmann]

(In reply to comment #27)
> This is what we learned:
> 
> Qt uses --preprocessor \"$${QMAKE_MOC} -E\" in other places that we use the proprocessor.  My last attempt to fix the build (http://trac.webkit.org/changeset/130957) used that, but the output from moc -E is different from gcc (namely, it doesn't maintain leading spaces).
> 
> I'm not sure what the right fix for this is.  Is there a way we can get moc to behave more like gcc?  Alternately, maybe we can use cl.exe on Qt Win?

I've looked a bit into this and the issue seems to be the following:

The bison grammar contains things like

  $$ = parser->createFloatingSelector();

Moc is tokenizing the input and due to the way it works it is tokenizing this in a way that it _usually_ works, which is as a C++ parser, because of its actual purpose. In C++ the dollar sign is not a valid character for identifiers, and therefore this causes an error (that is silently discarded).

When moc is used in preprocessor-only mode however it should not do that. I'm working on a fix for that. Locally applied I can build with the original patch that passes --preprocessor to makegrammar.pl.

[Simon Hausmann]

(In reply to comment #34)
[...]
> When moc is used in preprocessor-only mode however it should not do that. I'm working on a fix for that. Locally applied I can build with the original patch that passes --preprocessor to makegrammar.pl.

Fix uploaded into Qt Gerrit:

    https://codereview.qt-project.org/#change,37022

[Pablo Flouret]

Created attachment 168471 [details]
Patch

[Pablo Flouret]

(In reply to comment #36)
> Created an attachment (id=168471) [details]
> Patch

Cleaned up makegrammar.pl a bit and made it take the preprocessor and bison executables as arguments whenever possible, so provided the moc fix is in the bots i think now it should work correctly everywhere (including chromiumOS, since it should've failed the first time because the preprocessor wasn't an argument, i believe; we still need to keep an eye, of course).

Also got rid of action_preprocessgrammar.py, used makegrammar.pl on chromium here as well.

[Pablo Flouret]

Created attachment 168475 [details]
Rebased patch

[Gyuyoung Kim]

Comment on attachment 168475 [details]
Rebased patch

Attachment 168475 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14292008

[Pablo Flouret]

Created attachment 168479 [details]
Patch

Some debugging cruft was in the way.

[Tony Chang]

Comment on attachment 168479 [details]
Patch

Thanks! Re-using preprocess.pm and using the same perl script on Chromium seems more likely to work.

Maybe someone from Qt can try landing this and making sure the bots don't go red.

[Adam Barth]

Can you hold off landing this for a while?  The tree has been a disaster recently and I haven't been able to get a roll in all day.

[Tony Chang]

(In reply to comment #42)
> Can you hold off landing this for a while?  The tree has been a disaster recently and I haven't been able to get a roll in all day.

Yeah, I was hoping that after the ews bubbles all go green, someone from Qt can try landing it during their Monday morning (i.e., not today).

[Gyuyoung Kim]

Comment on attachment 168479 [details]
Patch

Attachment 168479 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14294026

[Early Warning System Bot]

Comment on attachment 168479 [details]
Patch

Attachment 168479 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/14291031

[Early Warning System Bot]

Comment on attachment 168479 [details]
Patch

Attachment 168479 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/14296041

[Simon Hausmann]

(In reply to comment #43)
> (In reply to comment #42)
> > Can you hold off landing this for a while?  The tree has been a disaster recently and I haven't been able to get a roll in all day.
> 
> Yeah, I was hoping that after the ews bubbles all go green, someone from Qt can try landing it during their Monday morning (i.e., not today).

The Qt fix went in, but it's not through all the stage of the Qt CI system yet. I hope that the change will be available in the final repo by middle of next week. It will require anyone building the Qt port of WebKit to update the Qt version, so could we perhaps aim for landing this end of next week? Or is that too late for you guys?

[Simon Hausmann]

(In reply to comment #47)
> (In reply to comment #43)
> > (In reply to comment #42)
> > > Can you hold off landing this for a while?  The tree has been a disaster recently and I haven't been able to get a roll in all day.
> > 
> > Yeah, I was hoping that after the ews bubbles all go green, someone from Qt can try landing it during their Monday morning (i.e., not today).
> 
> The Qt fix went in, but it's not through all the stage of the Qt CI system yet. I hope that the change will be available in the final repo by middle of next week. It will require anyone building the Qt port of WebKit to update the Qt version, so could we perhaps aim for landing this end of next week? Or is that too late for you guys?

The good news is that unexpectedly the update went through very fast and from the Qt side things are ready. I'll send a heads-up for the upcoming update to the webkit-qt mailing list and leave it as Ossy's discretion when to do the update (whenever it suits you best time wise :)

[Csaba Osztrogonác]

I'm updating the Linux bots today, Zoltán will update the Windows bot today or tomorrow, so we will be able to land it soon.

[Tony Chang]

FWIW, it's fine to wait until later in the week to land this.  I don't think it's blocking anything that time critical.

[Pablo Flouret]

Created attachment 168764 [details]
Rebased patch

[Pablo Flouret]

(In reply to comment #51)
> Created an attachment (id=168764) [details]
> Rebased patch

The bots were pretty wonky on friday, new ews run to be sure everything is still green.

[Csaba Osztrogonác]

I updated the Qt 5 on the Linux bots. We are waiting for the update on Qt-ARM bot and the update on Qt-Windows bot. I think the guys will finish it tomorrow.

[Gyuyoung Kim]

Comment on attachment 168764 [details]
Rebased patch

Attachment 168764 [details] did not pass efl-ews (efl):
Output: http://queues.webkit.org/results/14288842

[Pablo Flouret]

Comment on attachment 168764 [details]
Rebased patch

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

> Source/cmake/WebKitMacros.cmake:156
> +        COMMAND ${PERL_EXECUTABLE} -I ${WEBCORE_DIR}/bindings/scripts ${WEBCORE_DIR}/css/makegrammar.pl ${_input} --outputDir ${DERIVED_SOURCES_WEBCORE_DIR} --extraDefines "${_features}" --preprocessor "${CODE_GENERATOR_PREPROCESSOR}" --bison "${BISON_EXECUTABLE}" --symbolsPrefix ${_prefix} ${_input}

EFL build fails due to the (erroneous) extra ${_input} here.

[Pablo Flouret]

Created attachment 168823 [details]
Patch for landing

Got rid of the extra ${_input}

[Csaba Osztrogonác]

We updated all Qt bots, so it is safe to land the patch now.

[Pablo Flouret]

Comment on attachment 168823 [details]
Patch for landing

Tony, feel free to cq+ whenever you think is best. I can keep an eye on the bots to see if any problems come up.

[Tony Chang]

Comment on attachment 168823 [details]
Patch for landing

Clearing flags on attachment: 168823

Committed r131477: <http://trac.webkit.org/changeset/131477>

[Pablo Flouret]

Qt MIPS missing the moc update?

[Csaba Osztrogonác]

(In reply to comment #60)
> Qt MIPS missing the moc update?

Hmm ... it seems yes. Gergely, could you update the Qt 5 on the MIPS bot?

[Csaba Osztrogonác]

(In reply to comment #61)
> (In reply to comment #60)
> > Qt MIPS missing the moc update?
> 
> Hmm ... it seems yes. Gergely, could you update the Qt 5 on the MIPS bot?

FYI: http://lists.webkit.org/pipermail/webkit-qt/2012-October/003182.html

[Gergely Kis]

(In reply to comment #62)
> (In reply to comment #61)
> > (In reply to comment #60)
> > > Qt MIPS missing the moc update?
> > 
> > Hmm ... it seems yes. Gergely, could you update the Qt 5 on the MIPS bot?
> 
> FYI: http://lists.webkit.org/pipermail/webkit-qt/2012-October/003182.html

Working on the update right now, thanks for the heads up.