Bug 165783 – Remove flex and bison build dependencies; commit generated XPath parser

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

[Alex Christensen]

Remove flex and bison build dependencies now that CSS is parsed with a non-generated parser

[Alex Christensen]

Created attachment 296965 [details]
Patch

[Daniel Bates]

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.

[Alex Christensen]

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.

[Alex Christensen]

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.

[Alex Christensen]

Comment on attachment 296965 [details]
Patch

re-requesting review

[Alex Christensen]

Comment on attachment 296965 [details]
Patch

This might not be worth it.  Needs more discussion.

[Alex Christensen]

Yep, definitely worth it.  Let's do this.

[Brent Fulgham]

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."

[Alex Christensen]

Created attachment 297123 [details]
Patch

[Alex Christensen]

Created attachment 297217 [details]
Patch

[Alex Christensen]

Created attachment 297221 [details]
Patch

[Brent Fulgham]

Comment on attachment 297221 [details]
Patch

Thank you for including the bison generator comment! r=me.

[Alex Christensen]

Created attachment 297230 [details]
Patch

[Alex Christensen]

https://trac.webkit.org/changeset/209883

[Csaba Osztrogonác]

(In reply to comment #14)
> https://trac.webkit.org/changeset/209883

It broke the WinCairo build, see build.webkit.org for details.

[Alex Christensen]

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 :(