Bug 44802 – REGRESSION (r65351): WebCore build fails due to attempting to directly access WebKitTools/Scripts

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Mark Rowe (bdash)]

<http://trac.webkit.org/changeset/65351> has broken the ability to build WebCore in production configurations.  The change to DerivedSources.make attempts to access directories outside of the WebCore source directory.  Projects are built independently and without access to any other source trees.

This needs to be fixed as soon as is possible as this change has completely broken the ability to build production builds of WebKit on Mac OS X and Windows.  If it cannot be fixed promptly we’ll need to roll this particular change out.

[Eric Seidel (no email)]

Seems easy enough to fix.  Rolling out the change will just break your builds in other ways.

[Mark Rowe (bdash)]

A second issue is that the script involved uses third-party Python modules.  That will also need to be addressed.

[Eric Seidel (no email)]

The 3rd party python module "simplejson" is already provided in WebKitTools, which is why the tool was placed there.  When I placed the tool there, I was not aware of Apple's build limitation.  Thank you for bringing this to my attention.

Access to a json parser (which simplejson provides) is the only reason why the script was placed in WebKitTools.  One could simply check in a second copy of simplejson under WebCore to fix Apple's production build.

[Eric Seidel (no email)]

The script need not require simplejson, any way to parse json files will do.  Other solutions including moving HTMLEntityNames.json away from json, or writing our own json-like parser are possible.  When the script was written originally by Adam, he used simplejson.  I assume he chose simplejson because it was already used by many other of our scripts and already checked into our repository.

[Mark Rowe (bdash)]

(In reply to comment #3)
> The 3rd party python module "simplejson" is already provided in WebKitTools, which is why the tool was placed there.  When I placed the tool there, I was not aware of Apple's build limitation.  Thank you for bringing this to my attention.

It’s always been the case in the Mac OS X and Windows build systems that individual projects cannot access files outside of their source directory.  This is precisely the reason why delightful things such as forwarding headers exist.

> Access to a json parser (which simplejson provides) is the only reason why the script was placed in WebKitTools.  One could simply check in a second copy of simplejson under WebCore to fix Apple's production build.

Beyond being obviously unpleasant, I’m not sure that this would work.  One further constraint is that it’s not acceptable to modify the source tree in any way.  Python writes out .pyc files for modules that it imports, which I believe would violate that constraint.

[Mark Rowe (bdash)]

(In reply to comment #4)
> The script need not require simplejson, any way to parse json files will do.  Other solutions including moving HTMLEntityNames.json away from json, or writing our own json-like parser are possible.  When the script was written originally by Adam, he used simplejson.  I assume he chose simplejson because it was already used by many other of our scripts and already checked into our repository.

Looking at the input file it seems like JSON is overkill for this file.  It’s basically a list of names and corresponding values.  Something simple like CSV would be sufficient as far as I can see.

[Mark Rowe (bdash)]

Created attachment 65787 [details]
Something like this.

[Adam Barth]

We can use whatever input format is most convenient.  I used JSON just because it's easy to work with in most modern languages.

[Mark Rowe (bdash)]

So easy that it requires you install a third-party module…

I’ll flag this patch for review shortly.  I’m waiting on a couple of full world builds to complete to ensure that everything builds correctly.

[Adam Barth]

Yeah, that looks fine.  Does that address the pyc issue as well?  (Of course, the final version will need to build on other platforms too.)

[Adam Barth]

> So easy that it requires you install a third-party module…

simplejson is built into modern versions of Python.  We just need the external module to support Mr. Tiger.

> I’ll flag this patch for review shortly.  I’m waiting on a couple of full world builds to complete to ensure that everything builds correctly.

That patch is close, but there are a bunch of other build systems that need to call this script too.  If you grep for HTMLEntityNames.json, you can see how they work.

[Mark Rowe (bdash)]

Yes, it addresses the .pyc issue as there are no .py modules within the source tree that the script imports.

[Mark Rowe (bdash)]

(In reply to comment #11)
> > So easy that it requires you install a third-party module…
> 
> simplejson is built into modern versions of Python.  We just need the external module to support Mr. Tiger.

JSON support was added in Python 2.6.  A third-party module is needed even for Leopard.

> > I’ll flag this patch for review shortly.  I’m waiting on a couple of full world builds to complete to ensure that everything builds correctly.
> 
> That patch is close, but there are a bunch of other build systems that need to call this script too.  If you grep for HTMLEntityNames.json, you can see how they work.

Yep, I’m aware of that.

[Adam Barth]

> Yes, it addresses the .pyc issue as there are no .py modules within the source tree that the script imports.

Oh, cool.

[Adam Barth]

BTW, these files just moved into WebCore/html/parser.  You'll probably want to work from a revision in the past few minutes.

[Adam Barth]

Also, thanks for working on fixing this issue.

[Mark Rowe (bdash)]

Created attachment 65795 [details]
Patch

Updated patch.

[Adam Barth]

Comment on attachment 65795 [details]
Patch

Thanks.

WebCore/html/parser/create-html-entity-table:101
 +  // THIS FILE IS GENERATED BY WebCore/html/create-html-entity-table
WebCore/html/create-html-entity-table => WebCore/html/parser/create-html-entity-table

If we were more awesome, we'd generate the path automagically

[Mark Rowe (bdash)]

I noticed one further thing that needs to be included in this patch: updating the references to HTMLEntityNames.json in WebCore.xcodeproj.  While I’m there I’m going to stop copying the input file in to the framework wrapper, since copying it there is just stupid.  I’ll make these changes as I land.

[Eric Seidel (no email)]

Attachment 65795 [details] did not build on mac:
Build output: http://queues.webkit.org/results/3868031

[Mark Rowe (bdash)]

Landed in r66319.