Bug 165783 - Remove flex and bison build dependencies; commit generated XPath parser
Summary: Remove flex and bison build dependencies; commit generated XPath parser
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Alex Christensen
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-12-12 16:43 PST by Alex Christensen
Modified: 2016-12-16 10:05 PST (History)
8 users (show)

See Also:


Attachments
Patch (88.16 KB, patch)
2016-12-12 16:45 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (88.41 KB, patch)
2016-12-14 14:03 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (88.34 KB, patch)
2016-12-15 13:54 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (88.34 KB, patch)
2016-12-15 14:12 PST, Alex Christensen
no flags Details | Formatted Diff | Diff
Patch (89.58 KB, patch)
2016-12-15 15:13 PST, Alex Christensen
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Alex Christensen 2016-12-12 16:43:46 PST
Remove flex and bison build dependencies now that CSS is parsed with a non-generated parser
Comment 1 Alex Christensen 2016-12-12 16:45:24 PST
Created attachment 296965 [details]
Patch
Comment 2 Daniel Bates 2016-12-12 17:07:34 PST
Comment on attachment 296965 [details]
Patch

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

> Source/WebCore/xml/XPathGrammar.cpp:1258
> +      case 41: /* "NodeTest" */
> +#line 91 "WebCore/xml/XPathGrammar.y"
> +	{ delete (yyvaluep->nodeTest); };

I am unclear if you intentionally left the "WebCore/xml/XPathGrammar.y" comment lines and did not delete the WebCore/xml/XPathGrammar.y file so that a person can refer back to the original grammar file. Regardless, I find understanding this file and confidently making changes (even if it is once in a blue moon) strictly worse than modifying WebCore/xml/XPathGrammar.y.
Comment 3 Alex Christensen 2016-12-13 11:00:44 PST
I did intentionally leave XPathGrammar.y comments and the XPathGrammar.y file.  This file has only been touched three times since 2013, and none of them have been significant changes.  
1) Andy changed wtf::move to WTFMove. r194496
2) I made the grammar work with older versions of Bison. r189267
3) Per covered up a warning. r205011
None of these changes actually required running bison.  I still think making it easier to set up a build machine on Windows and having fewer dependencies on Linux will be more beneficial to the project than making one change per year slightly easier.
Comment 4 Alex Christensen 2016-12-13 15:48:50 PST
I really want this to happen.  I accept all responsibility for whatever maintenance burden this adds.  I really think it will decrease the maintenance burden.
Comment 5 Alex Christensen 2016-12-13 15:49:09 PST
Comment on attachment 296965 [details]
Patch

re-requesting review
Comment 6 Alex Christensen 2016-12-13 16:00:40 PST
Comment on attachment 296965 [details]
Patch

This might not be worth it.  Needs more discussion.
Comment 7 Alex Christensen 2016-12-14 13:38:50 PST
Yep, definitely worth it.  Let's do this.
Comment 8 Brent Fulgham 2016-12-14 13:55:17 PST
Comment on attachment 296965 [details]
Patch

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

Yes! r=me, assuming you can fix the GTK/EFL stuff. I think there is a file they have that gets mad if you remove other files without letting it know.

> Source/WebCore/xml/XPathGrammar.cpp:1
> +/* A Bison parser, made by GNU Bison 2.3.  */

Should we add a statement here to the effect that "This file is no longer automatically generated as part of the WebKit build. This code is in maintenance mode, and is no longer changing."
Comment 9 Alex Christensen 2016-12-14 14:03:52 PST
Created attachment 297123 [details]
Patch
Comment 10 Alex Christensen 2016-12-15 13:54:44 PST
Created attachment 297217 [details]
Patch
Comment 11 Alex Christensen 2016-12-15 14:12:35 PST
Created attachment 297221 [details]
Patch
Comment 12 Brent Fulgham 2016-12-15 14:14:10 PST
Comment on attachment 297221 [details]
Patch

Thank you for including the bison generator comment! r=me.
Comment 13 Alex Christensen 2016-12-15 15:13:13 PST
Created attachment 297230 [details]
Patch
Comment 14 Alex Christensen 2016-12-15 15:54:17 PST
https://trac.webkit.org/changeset/209883
Comment 15 Csaba Osztrogonác 2016-12-15 22:10:30 PST
(In reply to comment #14)
> https://trac.webkit.org/changeset/209883

It broke the WinCairo build, see build.webkit.org for details.
Comment 16 Alex Christensen 2016-12-16 10:05:27 PST
The WinCairo bot had been building XPathGrammar.cpp with some of my local changes.  This didn't break any actual build, it just looked like it did on my bot :(