Bug 89039 - Update ANGLE in WebKit
Summary: Update ANGLE in WebKit
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebGL (show other bugs)
Version: 528+ (Nightly build)
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Max Vujovic
URL:
Keywords:
: 91586 (view as bug list)
Depends on:
Blocks:
 
Reported: 2012-06-13 14:58 PDT by Max Vujovic
Modified: 2012-07-18 09:45 PDT (History)
20 users (show)

See Also:


Attachments
Patch (1011.51 KB, patch)
2012-06-14 11:14 PDT, Max Vujovic
gns: commit-queue-
Details | Formatted Diff | Diff
Patch (1012.91 KB, patch)
2012-06-15 10:05 PDT, Max Vujovic
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch (1013.54 KB, patch)
2012-06-15 11:00 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1020.66 KB, patch)
2012-06-15 18:16 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1020.75 KB, patch)
2012-06-15 18:43 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.01 MB, patch)
2012-06-16 11:14 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.00 MB, patch)
2012-06-19 11:38 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.01 MB, patch)
2012-06-19 14:31 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.01 MB, patch)
2012-06-20 11:12 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-27 23:53 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-28 00:28 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-28 11:15 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-28 13:34 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-28 13:54 PDT, Max Vujovic
mvujovic: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-06-28 15:07 PDT, Max Vujovic
dino: review+
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff
Patch (1.64 MB, patch)
2012-07-10 11:17 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (1.23 MB, patch)
2012-07-10 13:55 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (1.23 MB, patch)
2012-07-10 15:33 PDT, Max Vujovic
mrowe: review-
Details | Formatted Diff | Diff
Patch (1.16 MB, patch)
2012-07-16 11:48 PDT, Max Vujovic
mrowe: review+
webkit-ews: commit-queue-
Details | Formatted Diff | Diff
Patch (1.16 MB, patch)
2012-07-16 15:21 PDT, Max Vujovic
no flags Details | Formatted Diff | Diff
Patch (1.16 MB, patch)
2012-07-17 10:48 PDT, Max Vujovic
webkit.review.bot: commit-queue-
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Max Vujovic 2012-06-13 14:58:45 PDT
For CSS Shaders, we need some new features in ANGLE. ANGLE is currently r1009 in WebKit (last updated in bug 81717). Let's update it to r1147.
Comment 1 Max Vujovic 2012-06-14 11:14:33 PDT
Created attachment 147620 [details]
Patch
Comment 2 Gustavo Noronha (kov) 2012-06-14 11:27:47 PDT
Comment on attachment 147620 [details]
Patch

Attachment 147620 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12961268
Comment 3 Early Warning System Bot 2012-06-14 12:10:33 PDT
Comment on attachment 147620 [details]
Patch

Attachment 147620 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12961278
Comment 4 Early Warning System Bot 2012-06-14 12:57:40 PDT
Comment on attachment 147620 [details]
Patch

Attachment 147620 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12957421
Comment 5 Max Vujovic 2012-06-15 10:05:02 PDT
Created attachment 147843 [details]
Patch

Running EWS again. Trying to fix warnings on GTK EWS from new ANGLE source.
Comment 6 Early Warning System Bot 2012-06-15 10:23:47 PDT
Comment on attachment 147843 [details]
Patch

Attachment 147843 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12965531
Comment 7 Early Warning System Bot 2012-06-15 10:40:51 PDT
Comment on attachment 147843 [details]
Patch

Attachment 147843 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12952974
Comment 8 Max Vujovic 2012-06-15 11:00:52 PDT
Created attachment 147858 [details]
Patch
Comment 9 Early Warning System Bot 2012-06-15 11:31:55 PDT
Comment on attachment 147858 [details]
Patch

Attachment 147858 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12959681
Comment 10 Early Warning System Bot 2012-06-15 11:35:48 PDT
Comment on attachment 147858 [details]
Patch

Attachment 147858 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12970014
Comment 11 Early Warning System Bot 2012-06-15 12:33:11 PDT
Comment on attachment 147858 [details]
Patch

Attachment 147858 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12959698
Comment 12 Max Vujovic 2012-06-15 18:16:49 PDT
Created attachment 147931 [details]
Patch
Comment 13 Early Warning System Bot 2012-06-15 18:29:11 PDT
Comment on attachment 147931 [details]
Patch

Attachment 147931 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12965645
Comment 14 Early Warning System Bot 2012-06-15 18:30:45 PDT
Comment on attachment 147931 [details]
Patch

Attachment 147931 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12970097
Comment 15 Max Vujovic 2012-06-15 18:43:27 PDT
Created attachment 147934 [details]
Patch
Comment 16 Early Warning System Bot 2012-06-15 18:54:35 PDT
Comment on attachment 147934 [details]
Patch

Attachment 147934 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12970102
Comment 17 Early Warning System Bot 2012-06-15 18:55:57 PDT
Comment on attachment 147934 [details]
Patch

Attachment 147934 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12959775
Comment 18 Gustavo Noronha (kov) 2012-06-15 18:57:02 PDT
Comment on attachment 147934 [details]
Patch

Attachment 147934 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/12956868
Comment 19 Max Vujovic 2012-06-16 11:14:32 PDT
Created attachment 147977 [details]
Patch
Comment 20 Early Warning System Bot 2012-06-16 11:33:07 PDT
Comment on attachment 147977 [details]
Patch

Attachment 147977 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12964827
Comment 21 Early Warning System Bot 2012-06-16 11:37:20 PDT
Comment on attachment 147977 [details]
Patch

Attachment 147977 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12970262
Comment 22 Early Warning System Bot 2012-06-16 12:32:32 PDT
Comment on attachment 147977 [details]
Patch

Attachment 147977 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12965785
Comment 23 Max Vujovic 2012-06-19 11:38:14 PDT
Created attachment 148375 [details]
Patch
Comment 24 Early Warning System Bot 2012-06-19 12:49:58 PDT
Comment on attachment 148375 [details]
Patch

Attachment 148375 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/12972874
Comment 25 Early Warning System Bot 2012-06-19 13:27:45 PDT
Comment on attachment 148375 [details]
Patch

Attachment 148375 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/12988176
Comment 26 Max Vujovic 2012-06-19 14:31:29 PDT
Created attachment 148426 [details]
Patch
Comment 27 Max Vujovic 2012-06-20 11:12:38 PDT
Created attachment 148608 [details]
Patch
Comment 28 Max Vujovic 2012-06-27 23:53:04 PDT
Created attachment 149888 [details]
Patch
Comment 29 Early Warning System Bot 2012-06-28 00:17:47 PDT
Comment on attachment 149888 [details]
Patch

Attachment 149888 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13107074
Comment 30 Max Vujovic 2012-06-28 00:28:00 PDT
Created attachment 149894 [details]
Patch
Comment 31 Early Warning System Bot 2012-06-28 00:49:00 PDT
Comment on attachment 149894 [details]
Patch

Attachment 149894 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13100965
Comment 32 Early Warning System Bot 2012-06-28 00:56:07 PDT
Comment on attachment 149894 [details]
Patch

Attachment 149894 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13099946
Comment 33 Early Warning System Bot 2012-06-28 01:45:49 PDT
Comment on attachment 149894 [details]
Patch

Attachment 149894 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13101987
Comment 34 Max Vujovic 2012-06-28 11:15:32 PDT
Created attachment 149975 [details]
Patch
Comment 35 Early Warning System Bot 2012-06-28 12:26:09 PDT
Comment on attachment 149975 [details]
Patch

Attachment 149975 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13110126
Comment 36 Max Vujovic 2012-06-28 13:34:31 PDT
Created attachment 149996 [details]
Patch
Comment 37 Gustavo Noronha (kov) 2012-06-28 13:49:32 PDT
Comment on attachment 149996 [details]
Patch

Attachment 149996 [details] did not pass gtk-ews (gtk):
Output: http://queues.webkit.org/results/13107209
Comment 38 Max Vujovic 2012-06-28 13:54:10 PDT
Created attachment 150006 [details]
Patch
Comment 39 Max Vujovic 2012-06-28 15:07:45 PDT
Created attachment 150022 [details]
Patch
Comment 40 Max Vujovic 2012-06-28 15:56:45 PDT
Comment on attachment 150022 [details]
Patch

This patch is ready for review.

I'm updating ANGLE to r1170.

When trying to update to ANGLE r1147, I was getting build warnings on the QT and GTK bots. I fixed the warnings and pushed the fixes to ANGLE in r1170.

However, between r1147 and r1170, ANGLE enabled its new preprocessor. This caused a new build issue on QT, which I fixed in this patch by renaming two files. 

To avoid chasing a moving target, I plan on fixing this issue in ANGLE after this patch. The issue and fix is described the ChangeLog, pasted below:

"""
Update ANGLE to r1170, with the following modifications:

Rename ANGLE/src/compiler/preprocessor/new/Diagnostic.cpp to DiagnosticBase.cpp.
Rename ANGLE/src/compiler/preprocessor/new/DirectiveHandler.cpp to DirectiveHandlerBase.cpp.

With the introduction of ANGLE's new preprocessor, there were two files named Diagnostic.cpp in ANGLE under different folders. This caused problems on the QT build when their object files, both named Diagnostic.o, tried to go in the same folder. Renaming one of them to DirectiveHandlerBase.cpp avoids this conflict. The same situation occurred with DirectiveHandler.cpp. I will try to get their names changed in ANGLE for future updates.
"""
Comment 41 Dean Jackson 2012-07-01 15:58:14 PDT
Comment on attachment 150022 [details]
Patch

rs=me. I remember when I did this I had to be careful choosing the version of Bison that processed the OS X port, but since the mac EWS is ok I'm crossing my fingers here.
Comment 42 WebKit Review Bot 2012-07-01 16:50:34 PDT
Comment on attachment 150022 [details]
Patch

Rejecting attachment 150022 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 11947 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
47>At revision 11947.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13130003
Comment 43 Max Vujovic 2012-07-01 23:03:49 PDT
(In reply to comment #41)
> (From update of attachment 150022 [details])
> rs=me. I remember when I did this I had to be careful choosing the version of Bison that processed the OS X port, but since the mac EWS is ok I'm crossing my fingers here.

Thanks, Dean! You used Bison 2.3 last time. I'm using Bison 2.4.2 this time. I haven't had any build or test issues yet.

However, it looks like the review bot didn't like the tabs in two new Bison generated files. I'll fix that up next week, and watch the build bots then. (I'm traveling with limited internet access this week).
Comment 44 Max Vujovic 2012-07-10 11:17:30 PDT
Created attachment 151493 [details]
Patch

In this new patch, I resolved a conflict from bug 90506 in WebCore/Target.pri. Let's run the bots again and make sure it works.
Comment 45 Max Vujovic 2012-07-10 13:55:38 PDT
Created attachment 151520 [details]
Patch

To pass the commit-queue, created an svn patch with the allow-tabs property set on the new files in the preprocessor/new folder. This property is already set on the existing files in ANGLE.
Comment 46 Max Vujovic 2012-07-10 15:33:38 PDT
Created attachment 151538 [details]
Patch

Updated the ChangeLog date and Reviewed By in the patch for the commit.
Comment 47 Alexandru Chiculita 2012-07-10 15:37:39 PDT
Comment on attachment 151538 [details]
Patch

Setting the cq+ flag, as the patch was just rebased since it was reviewed last time.
Comment 48 Mark Rowe (bdash) 2012-07-10 15:57:38 PDT
Comment on attachment 151538 [details]
Patch

This contains code generated by Bison that is licensed under GPL v3. Please generate it with a version of Bison that uses a license that the WebKit project is ok with (e.g., Bison v2.3).
Comment 49 Max Vujovic 2012-07-10 16:05:24 PDT
(In reply to comment #48)
> (From update of attachment 151538 [details])
> This contains code generated by Bison that is licensed under GPL v3. Please generate it with a version of Bison that uses a license that the WebKit project is ok with (e.g., Bison v2.3).

Sorry about that- didn't know about the licensing issue. ANGLE uses Bison 2.4.2 nowadays. I'll try to change this patch to use Bison 2.3 like before.
Comment 50 Max Vujovic 2012-07-11 10:36:17 PDT
(In reply to comment #49)
> (In reply to comment #48)
> > (From update of attachment 151538 [details] [details])
> > This contains code generated by Bison that is licensed under GPL v3. Please generate it with a version of Bison that uses a license that the WebKit project is ok with (e.g., Bison v2.3).
> 
> Sorry about that- didn't know about the licensing issue. ANGLE uses Bison 2.4.2 nowadays. I'll try to change this patch to use Bison 2.3 like before.

Hi Mark,

I found out that the license on the Bison 2.4.2 generated files isn't actually GPL v3. It's GPL v3 with the Bison exception. The exception means that you can use Bison under the license of your choice as long as you aren't using it to make a parser generator. This means the Bison generated code in this patch can be GPL v2. Ken Russell pointed this out to me.

Here's the Bison exception, lifted from the License in the ExpressionParser.cpp, which was generated with Bison 2.4.2:
"""
/* As a special exception, you may create a larger work that contains
   part or all of the Bison parser skeleton and distribute that work
   under terms of your choice, so long as that work isn't itself a
   parser generator using the skeleton or a modified version thereof
   as a parser skeleton.  Alternatively, if you modify or redistribute
   the parser skeleton itself, you may (at your option) remove this
   special exception, which will cause the skeleton and the resulting
   Bison output files to be licensed under the GNU General Public
   License without this special exception.
   
   This special exception was added by the Free Software Foundation in
   version 2.2 of Bison.  */
"""

If that's ok with you, could you revert the r-?

Thank you,
Max
Comment 51 Mark Rowe (bdash) 2012-07-11 11:06:33 PDT
It's still not ok.
Comment 52 Kenneth Russell 2012-07-11 11:26:53 PDT
(In reply to comment #51)
> It's still not ok.

Mark, could you please describe more clearly why not? The code in question is *not* being licensed under GPLv3 -- it is being licensed under the WebKit project's terms, per the Bison exception clause.

In parallel, we'll look into using the older Bison directive for making a reentrant parser -- but I think it would be a mistake to artificially limit the version of Bison that can be used within WebKit.
Comment 53 Mark Rowe (bdash) 2012-07-11 11:40:11 PDT
Let me start off by pointing out that I'm not a lawyer, and the reason for my objecting so strongly comes from having no desire to discuss this with lawyers when it's so trivial to avoid having a need to do so.  That said, the file quite clearly states at the top of it:

   This program is free software: you can redistribute it and/or modify
    it under the terms of the GNU General Public License as published by
   the Free Software Foundation, either version 3 of the License, or
   (at your option) any later version.

The fact that there is an exception allowing distribution under terms of our choice doesn't change this fact. 

On an unrelated note, it's not obvious to me why these generated files are checked in rather than being generated during the build process.
Comment 54 Kenneth Russell 2012-07-11 11:49:12 PDT
(In reply to comment #53)
> Let me start off by pointing out that I'm not a lawyer, and the reason for my objecting so strongly comes from having no desire to discuss this with lawyers when it's so trivial to avoid having a need to do so.  That said, the file quite clearly states at the top of it:
> 
>    This program is free software: you can redistribute it and/or modify
>     it under the terms of the GNU General Public License as published by
>    the Free Software Foundation, either version 3 of the License, or
>    (at your option) any later version.
> 
> The fact that there is an exception allowing distribution under terms of our choice doesn't change this fact. 

I'm not a lawyer either, and hate to point this out, but the current sources say the same thing, only with GPL v2 instead of v3. To the best of my knowledge the WebKit project is currently relying on the exception clause.

> On an unrelated note, it's not obvious to me why these generated files are checked in rather than being generated during the build process.

It makes things easier for developers consuming the ANGLE sources.
Comment 55 Mark Rowe (bdash) 2012-07-11 11:58:50 PDT
(In reply to comment #54)
> (In reply to comment #53)
> > Let me start off by pointing out that I'm not a lawyer, and the reason for my objecting so strongly comes from having no desire to discuss this with lawyers when it's so trivial to avoid having a need to do so.  That said, the file quite clearly states at the top of it:
> > 
> >    This program is free software: you can redistribute it and/or modify
> >     it under the terms of the GNU General Public License as published by
> >    the Free Software Foundation, either version 3 of the License, or
> >    (at your option) any later version.
> > 
> > The fact that there is an exception allowing distribution under terms of our choice doesn't change this fact. 
> 
> I'm not a lawyer either, and hate to point this out, but the current sources say the same thing, only with GPL v2 instead of v3. To the best of my knowledge the WebKit project is currently relying on the exception clause.

That's fine. It's version three of the GPL that we want nothing to do with.

> 
> > On an unrelated note, it's not obvious to me why these generated files are checked in rather than being generated during the build process.
> 
> It makes things easier for developers consuming the ANGLE sources.
Comment 56 Dean Jackson 2012-07-12 04:03:25 PDT
(In reply to comment #54)

> > On an unrelated note, it's not obvious to me why these generated files are checked in rather than being generated during the build process.
> 
> It makes things easier for developers consuming the ANGLE sources.

Outside of the resolution here, I think we should look into this. We use bison to process other grammars in the build process. I'm not sure ANGLE is that much more difficult.

The most annoying part will be setting this up for all builds.
Comment 57 Joshua Netterfield 2012-07-12 09:19:34 PDT
> Source/ThirdParty/ANGLE/src/compiler/osinclude.h:-20
> -      defined(__GLIBC__) || defined(__GNU__) || defined(__QNX__)

QNX systems are POSIX. I'll submit a bug to ANGLE, but in the meantime could you continue to recognize QNX as POSIX?
Comment 58 Max Vujovic 2012-07-12 09:50:24 PDT
(In reply to comment #56)
> (In reply to comment #54)
> 
> > > On an unrelated note, it's not obvious to me why these generated files are checked in rather than being generated during the build process.
> > 
> > It makes things easier for developers consuming the ANGLE sources.
> 
> Outside of the resolution here, I think we should look into this. We use bison to process other grammars in the build process. I'm not sure ANGLE is that much more difficult.
> 
> The most annoying part will be setting this up for all builds.

It would be nice to use Bison and Flex in the WebKit ANGLE build.

I've made the following umbrella bug:
Bug 91105 - [ANGLE] Use Bison and Flex during ANGLE build

And child bugs for each platform using ANGLE:
Bug 91107 - [ANGLE] On Mac, use Bison and Flex during ANGLE build
Bug 91108 - [ANGLE] On QT, use Bison and Flex during ANGLE build
Bug 91109 - [ANGLE] On GTK, use Bison and Flex during ANGLE build
Comment 59 Max Vujovic 2012-07-12 09:51:42 PDT
(In reply to comment #57)
> > Source/ThirdParty/ANGLE/src/compiler/osinclude.h:-20
> > -      defined(__GLIBC__) || defined(__GNU__) || defined(__QNX__)
> 
> QNX systems are POSIX. I'll submit a bug to ANGLE, but in the meantime could you continue to recognize QNX as POSIX?

Sure, I can do that. Thank you for filing the bug on ANGLE.
Comment 60 Max Vujovic 2012-07-16 11:48:47 PDT
Created attachment 152576 [details]
Patch

I've uploaded a new patch with the following changes:

(1) Use Bison 2.3 instead of Bison 2.4.2 to generate ExpressionParser.cpp and glslang_tab.cpp. I had to modify ExpressionParser.y to make it compatible with Bison 2.3. The changes have been contributed back to ANGLE in r1224.

(2) Continue to recognize QNX as POSIX in ANGLE. This has been contributed back to ANGLE in r1223.

These changes are also noted in the ChangeLog entries.

Mark, does this work for you?
Comment 61 Mark Rowe (bdash) 2012-07-16 12:00:14 PDT
Comment on attachment 152576 [details]
Patch

Thanks for making those changes.
Comment 62 Max Vujovic 2012-07-16 12:05:06 PDT
(In reply to comment #61)
> (From update of attachment 152576 [details])
> Thanks for making those changes.

No problem. Thanks for the review :)
Comment 63 Early Warning System Bot 2012-07-16 12:33:23 PDT
Comment on attachment 152576 [details]
Patch

Attachment 152576 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13245973
Comment 64 Early Warning System Bot 2012-07-16 12:42:50 PDT
Comment on attachment 152576 [details]
Patch

Attachment 152576 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13262067
Comment 65 Early Warning System Bot 2012-07-16 13:26:24 PDT
Comment on attachment 152576 [details]
Patch

Attachment 152576 [details] did not pass qt-wk2-ews (qt):
Output: http://queues.webkit.org/results/13256224
Comment 66 Early Warning System Bot 2012-07-16 13:38:48 PDT
Comment on attachment 152576 [details]
Patch

Attachment 152576 [details] did not pass qt-ews (qt):
Output: http://queues.webkit.org/results/13255224
Comment 67 Max Vujovic 2012-07-16 15:21:07 PDT
Created attachment 152623 [details]
Patch

Patch to fix the QT EWS failure.

QT EWS fails on the Bison 2.3 generated files because two macros aren't defined on QT: YYENABLE_NLS and YYLTYPE_IS_TRIVIAL.

In CSSGrammar.y, we fix this Bison 2.3 issue by adding the lines:
#define YYENABLE_NLS 0
#define YYLTYPE_IS_TRIVIAL 1

I've added the same lines to ANGLE's grammar files (glslang.y and ExpressionParser.y) and ran Bison 2.3 again.

I'll work on contributing this fix back to ANGLE.

As a side note, Bison 2.4.2 fixes this issue by first checking if these macros are defined and then checking their value. Here's the original patch to Bison: http://lists.gnu.org/archive/html/grub-devel/2010-01/msg00340.html
Comment 68 Max Vujovic 2012-07-17 10:48:48 PDT
Created attachment 152784 [details]
Patch

Added Dean and Mark as the reviewers in the ChangeLog for landing.
Comment 69 Alexandru Chiculita 2012-07-17 11:03:21 PDT
Comment on attachment 152784 [details]
Patch

Trying the CQ again.
Comment 70 WebKit Review Bot 2012-07-17 12:01:32 PDT
Comment on attachment 152784 [details]
Patch

Rejecting attachment 152784 [details] from commit-queue.

Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=queues.webkit.org', '-..." exit_code: 2

Last 500 characters of output:
queue/Source/WebKit/chromium/third_party/skia/include --revision 4600 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
44>At revision 4600.

________ running '/usr/bin/python tools/clang/scripts/update.py --mac-only' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'

________ running '/usr/bin/python gyp_webkit' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium'
Updating webkit projects from gyp files...

Full output: http://queues.webkit.org/results/13278214
Comment 71 Adam Barth 2012-07-17 12:07:03 PDT
    The following files contain tab characters:

        trunk/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/ExpressionParser.cpp
        trunk/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/Tokenizer.cpp

    Please use spaces instead to indent.
    If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
 at /usr/lib/git-core/git-svn line 570
Comment 72 Max Vujovic 2012-07-17 12:14:18 PDT
(In reply to comment #71)
>     The following files contain tab characters:
> 
>         trunk/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/ExpressionParser.cpp
>         trunk/Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/Tokenizer.cpp
> 
>     Please use spaces instead to indent.
>     If you must commit a file with tabs, use svn propset to set the "allow-tabs" property.
>  at /usr/lib/git-core/git-svn line 570

That's strange because I already set "allow-tabs" on those files in the patch. I'll investigate further. Thanks, Adam.
Comment 73 Alexandru Chiculita 2012-07-17 13:21:16 PDT
svn-apply was ignoring the propsets for allow-tabs inside the patch, so I landed it manually on behalf of Max.

Manually committed r122870: http://trac.webkit.org/changeset/122870
Comment 74 Daniel Bates 2012-07-17 16:51:50 PDT
(In reply to comment #73)
> svn-apply was ignoring the propsets for allow-tabs inside the patch, so I landed it manually on behalf of Max.
> 
> Manually committed r122870: http://trac.webkit.org/changeset/122870

Filed bug #91556 to teach svn-{apply, unapply} about the allow-tabs property.
Comment 75 Peter Gal 2012-07-18 00:32:19 PDT
This patch resulted a build fail on the Qt linux debug bots.

http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Debug/builds/23917

error msg:

../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp: In member function 'void pp::MacroExpander::ungetToken(const pp::Token&)':
.../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp:129: error: comparison of unsigned expression >= 0 is always true


it seems the problem is the 'assert(context->index >= 0);' line, the index is of type size_t which is unsigned . Maybe we can remove this line or something else?
Comment 76 Kristóf Kosztyó 2012-07-18 00:52:44 PDT
(In reply to comment #75)
> This patch resulted a build fail on the Qt linux debug bots.
> 
> http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Debug/builds/23917
> 
> error msg:
> 
> ../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp: In member function 'void pp::MacroExpander::ungetToken(const pp::Token&)':
> .../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp:129: error: comparison of unsigned expression >= 0 is always true
> 
> 
> it seems the problem is the 'assert(context->index >= 0);' line, the index is of type size_t which is unsigned . Maybe we can remove this line or something else?

I made this fix in r122925 (http://trac.webkit.org/changeset/122925)
Comment 77 Dongseong Hwang 2012-07-18 01:49:21 PDT
*** Bug 91586 has been marked as a duplicate of this bug. ***
Comment 78 Max Vujovic 2012-07-18 09:45:15 PDT
(In reply to comment #76)
> (In reply to comment #75)
> > This patch resulted a build fail on the Qt linux debug bots.
> > 
> > http://build.webkit.sed.hu/builders/x86-64%20Linux%20Qt%20Debug/builds/23917
> > 
> > error msg:
> > 
> > ../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp: In member function 'void pp::MacroExpander::ungetToken(const pp::Token&)':
> > .../Source/ThirdParty/ANGLE/src/compiler/preprocessor/new/MacroExpander.cpp:129: error: comparison of unsigned expression >= 0 is always true
> > 
> > 
> > it seems the problem is the 'assert(context->index >= 0);' line, the index is of type size_t which is unsigned . Maybe we can remove this line or something else?
> 
> I made this fix in r122925 (http://trac.webkit.org/changeset/122925)

Thank you for the build fix. I'll get this fixed in ANGLE. I've created an ANGLE bug for the issue:
http://code.google.com/p/angleproject/issues/detail?id=349