RESOLVED FIXED 44802
REGRESSION (r65351): WebCore build fails due to attempting to directly access WebKitTools/Scripts
https://bugs.webkit.org/show_bug.cgi?id=44802
Summary REGRESSION (r65351): WebCore build fails due to attempting to directly access...
Mark Rowe (bdash)
Reported 2010-08-27 16:07:44 PDT
<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.
Attachments
Something like this. (178.97 KB, patch)
2010-08-27 16:52 PDT, Mark Rowe (bdash)
no flags
Patch (186.18 KB, patch)
2010-08-27 17:39 PDT, Mark Rowe (bdash)
abarth: review+
Eric Seidel (no email)
Comment 1 2010-08-27 16:15:24 PDT
Seems easy enough to fix. Rolling out the change will just break your builds in other ways.
Mark Rowe (bdash)
Comment 2 2010-08-27 16:18:18 PDT
A second issue is that the script involved uses third-party Python modules. That will also need to be addressed.
Eric Seidel (no email)
Comment 3 2010-08-27 16:22:30 PDT
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)
Comment 4 2010-08-27 16:25:09 PDT
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)
Comment 5 2010-08-27 16:28:46 PDT
(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)
Comment 6 2010-08-27 16:30:59 PDT
(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)
Comment 7 2010-08-27 16:52:28 PDT
Created attachment 65787 [details] Something like this.
Adam Barth
Comment 8 2010-08-27 16:53:53 PDT
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)
Comment 9 2010-08-27 16:57:42 PDT
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
Comment 10 2010-08-27 16:58:20 PDT
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
Comment 11 2010-08-27 17:00:00 PDT
> 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)
Comment 12 2010-08-27 17:00:58 PDT
Yes, it addresses the .pyc issue as there are no .py modules within the source tree that the script imports.
Mark Rowe (bdash)
Comment 13 2010-08-27 17:02:04 PDT
(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
Comment 14 2010-08-27 17:03:55 PDT
> Yes, it addresses the .pyc issue as there are no .py modules within the source tree that the script imports. Oh, cool.
Adam Barth
Comment 15 2010-08-27 17:05:11 PDT
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
Comment 16 2010-08-27 17:07:29 PDT
Also, thanks for working on fixing this issue.
Mark Rowe (bdash)
Comment 17 2010-08-27 17:39:43 PDT
Created attachment 65795 [details] Patch Updated patch.
Adam Barth
Comment 18 2010-08-27 17:46:24 PDT
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)
Comment 19 2010-08-27 17:50:40 PDT
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)
Comment 20 2010-08-27 18:03:06 PDT
Mark Rowe (bdash)
Comment 21 2010-08-28 19:35:06 PDT
Landed in r66319.
Note You need to log in before you can comment on or make changes to this bug.