WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED INVALID
113448
Move preprocessing of idl to pre-pass for gyp
https://bugs.webkit.org/show_bug.cgi?id=113448
Summary
Move preprocessing of idl to pre-pass for gyp
Scott Graham
Reported
2013-03-27 15:03:55 PDT
Reduces number of invocations of preprocessor from 20071 to 633.
Attachments
patch
(7.93 KB, patch)
2013-03-27 15:04 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
patch
(7.93 KB, patch)
2013-03-27 15:04 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
tidy up
(7.79 KB, patch)
2013-03-27 15:10 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
remove print
(7.71 KB, patch)
2013-03-27 15:34 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
Patch
(9.47 KB, patch)
2013-03-27 15:46 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
add note to changelog
(9.65 KB, patch)
2013-03-27 15:57 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
fix for older perl versions
(9.81 KB, patch)
2013-03-27 16:14 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
fix SVGURIReference.idl
(9.43 KB, patch)
2013-03-27 16:56 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
rebase
(9.43 KB, patch)
2013-03-28 08:55 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
rebase again
(9.24 KB, patch)
2013-03-28 09:58 PDT
,
Scott Graham
no flags
Details
Formatted Diff
Diff
Show Obsolete
(9)
View All
Add attachment
proposed patch, testcase, etc.
Scott Graham
Comment 1
2013-03-27 15:04:13 PDT
Created
attachment 195405
[details]
patch
Scott Graham
Comment 2
2013-03-27 15:04:51 PDT
Created
attachment 195406
[details]
patch
Scott Graham
Comment 3
2013-03-27 15:08:08 PDT
preprocessor.pm still does the preprocessing, and if available returns an already preprocessed idl file. This allows a few other users of preprocessor (in css, etc.) to not bother having a pre-pass. If it looks reasonable, I'll add a ChangeLog, etc.
Scott Graham
Comment 4
2013-03-27 15:10:43 PDT
Created
attachment 195409
[details]
tidy up
Tony Chang
Comment 5
2013-03-27 15:16:56 PDT
Comment on
attachment 195409
[details]
tidy up View in context:
https://bugs.webkit.org/attachment.cgi?id=195409&action=review
Nice, I like this better than my patch. Let's clean this up and land this instead.
> WebCore/WebCore.gypi:716 > + 'svg/SVGURIReference.idl',
Is this change intentional?
> WebCore/bindings/scripts/preprocessor.pm:56 > + print "$fileName was not pre-preprocessed, looking for $preprocessedFile\n";
We probably don't want this to print this. I think some other port may use $outputDirectory but without the preprocess build step, this will just spam to the console.
Tony Chang
Comment 6
2013-03-27 15:17:31 PDT
***
Bug 113442
has been marked as a duplicate of this bug. ***
Nico Weber
Comment 7
2013-03-27 15:26:38 PDT
I think this looks reasonable. It's a bit unfortunate that we still need to shell out to perl for the preprocessing step, but oh well.
Scott Graham
Comment 8
2013-03-27 15:34:49 PDT
Created
attachment 195415
[details]
remove print
Scott Graham
Comment 9
2013-03-27 15:35:17 PDT
(In reply to
comment #5
)
> (From update of
attachment 195409
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=195409&action=review
> > Nice, I like this better than my patch. Let's clean this up and land this instead. > > > WebCore/WebCore.gypi:716 > > + 'svg/SVGURIReference.idl', > > Is this change intentional?
Yes, it wasn't included in the top-level gyp list, but is referenced in other idls, so it ended up being processed a few hundred times via references. It was the only one in that situation, so I assumed the omission was unintentional. (?)
> > > WebCore/bindings/scripts/preprocessor.pm:56 > > + print "$fileName was not pre-preprocessed, looking for $preprocessedFile\n"; > > We probably don't want this to print this. I think some other port may use $outputDirectory but without the preprocess build step, this will just spam to the console.
Done.
Scott Graham
Comment 10
2013-03-27 15:46:03 PDT
Created
attachment 195420
[details]
Patch
Scott Graham
Comment 11
2013-03-27 15:46:36 PDT
with ChangeLog now.
Tony Chang
Comment 12
2013-03-27 15:48:57 PDT
Comment on
attachment 195420
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=195420&action=review
> Source/WebCore/ChangeLog:17 > + * WebCore.gypi:
Nit: Please explain why you added the idl file here.
> Source/WebCore/WebCore.gyp/WebCore.gyp:649 > 'sources': [ > # bison rule
Should we remove the preprocess flag from our calls to generate-bindings.pl?
EFL EWS Bot
Comment 13
2013-03-27 15:55:12 PDT
Comment on
attachment 195420
[details]
Patch
Attachment 195420
[details]
did not pass efl-ews (efl): Output:
http://webkit-commit-queue.appspot.com/results/17288684
WebKit Review Bot
Comment 14
2013-03-27 15:56:11 PDT
Comment on
attachment 195420
[details]
Patch
Attachment 195420
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://webkit-commit-queue.appspot.com/results/17319242
Scott Graham
Comment 15
2013-03-27 15:57:44 PDT
Created
attachment 195423
[details]
add note to changelog
Tony Chang
Comment 16
2013-03-27 15:58:37 PDT
Looks like File::Slurp and File::Basename doesn't exist on all platforms.
Scott Graham
Comment 17
2013-03-27 16:01:04 PDT
(In reply to
comment #12
)
> (From update of
attachment 195420
[details]
) > View in context:
https://bugs.webkit.org/attachment.cgi?id=195420&action=review
> > > Source/WebCore/ChangeLog:17 > > + * WebCore.gypi: > > Nit: Please explain why you added the idl file here.
Done.
> > > Source/WebCore/WebCore.gyp/WebCore.gyp:649 > > 'sources': [ > > # bison rule > > Should we remove the preprocess flag from our calls to generate-bindings.pl?
I might be confused, but I think we shouldn't because of cases like the idl above -- if someone forgets to add a subreferenced idl, this way it'll still work.
Scott Graham
Comment 18
2013-03-27 16:14:18 PDT
Created
attachment 195427
[details]
fix for older perl versions
WebKit Review Bot
Comment 19
2013-03-27 16:30:33 PDT
Comment on
attachment 195427
[details]
fix for older perl versions
Attachment 195427
[details]
did not pass chromium-ews (chromium-xvfb): Output:
http://webkit-commit-queue.appspot.com/results/17329205
Scott Graham
Comment 20
2013-03-27 16:35:19 PDT
(In reply to
comment #19
)
> (From update of
attachment 195427
[details]
) >
Attachment 195427
[details]
did not pass chromium-ews (chromium-xvfb): > Output:
http://webkit-commit-queue.appspot.com/results/17329205
Hm, apparently that's why. Sigh. I can either add it manually to the preprocess, or let it do a couple hundred extra invocations I guess.
Peter Beverloo (cr-android ews)
Comment 21
2013-03-27 16:47:42 PDT
Comment on
attachment 195427
[details]
fix for older perl versions
Attachment 195427
[details]
did not pass cr-android-ews (chromium-android): Output:
http://webkit-commit-queue.appspot.com/results/17130990
Scott Graham
Comment 22
2013-03-27 16:56:34 PDT
Created
attachment 195437
[details]
fix SVGURIReference.idl
WebKit Review Bot
Comment 23
2013-03-27 18:26:53 PDT
Comment on
attachment 195437
[details]
fix SVGURIReference.idl Rejecting
attachment 195437
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-03', 'land-attachment', '--force-clean', '--non-interactive', '--parent-command=commit-queue', 195437, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: t/git/webkit-commit-queue/Source/WebKit/chromium/v8 --revision 14075 --non-interactive --force --accept theirs-conflict --ignore-externals' in '/mnt/git/webkit-commit-queue/Source/WebKit/chromium' 55>At revision 14075. ________ 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://webkit-commit-queue.appspot.com/results/17239764
WebKit Review Bot
Comment 24
2013-03-27 18:40:06 PDT
Comment on
attachment 195437
[details]
fix SVGURIReference.idl Rejecting
attachment 195437
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-01', 'apply-attachment', '--no-update', '--non-interactive', 195437, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: 2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGenerator.pm.rej patching file Source/WebCore/bindings/scripts/IDLParser.pm patching file Source/WebCore/bindings/scripts/generate-bindings.pl patching file Source/WebCore/bindings/scripts/generate-preprocessed-idls.pl patching file Source/WebCore/bindings/scripts/preprocessor.pm Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output:
http://webkit-commit-queue.appspot.com/results/17342025
Scott Graham
Comment 25
2013-03-28 08:55:01 PDT
Created
attachment 195579
[details]
rebase
Kentaro Hara
Comment 26
2013-03-28 08:55:42 PDT
Comment on
attachment 195579
[details]
rebase here we go!
WebKit Review Bot
Comment 27
2013-03-28 08:58:06 PDT
Comment on
attachment 195579
[details]
rebase Rejecting
attachment 195579
[details]
from commit-queue. Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 195579, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue Last 500 characters of output: 2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGenerator.pm.rej patching file Source/WebCore/bindings/scripts/IDLParser.pm patching file Source/WebCore/bindings/scripts/generate-bindings.pl patching file Source/WebCore/bindings/scripts/generate-preprocessed-idls.pl patching file Source/WebCore/bindings/scripts/preprocessor.pm Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue Full output:
http://webkit-commit-queue.appspot.com/results/17342159
Scott Graham
Comment 28
2013-03-28 09:02:59 PDT
(In reply to
comment #27
)
> (From update of
attachment 195579
[details]
) > Rejecting
attachment 195579
[details]
from commit-queue. > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 195579, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue > > Last 500 characters of output: > 2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGenerator.pm.rej > patching file Source/WebCore/bindings/scripts/IDLParser.pm > patching file Source/WebCore/bindings/scripts/generate-bindings.pl > patching file Source/WebCore/bindings/scripts/generate-preprocessed-idls.pl > patching file Source/WebCore/bindings/scripts/preprocessor.pm > > Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue > > Full output:
http://webkit-commit-queue.appspot.com/results/17342159
Ah, looks like it's because of
http://trac.webkit.org/changeset/147041/trunk/Source/WebCore/bindings/scripts/CodeGenerator.pm
which I also had to modify here. I only rebased to the Chromium DEPS revision. I will wait until WebKit is rolled in Chromium and then try again.
Nico Weber
Comment 29
2013-03-28 09:40:20 PDT
(In reply to
comment #28
)
> (In reply to
comment #27
) > > (From update of
attachment 195579
[details]
[details]) > > Rejecting
attachment 195579
[details]
[details] from commit-queue. > > > > Failed to run "['/mnt/git/webkit-commit-queue/Tools/Scripts/webkit-patch', '--status-host=webkit-commit-queue.appspot.com', '--bot-id=gce-cq-04', 'apply-attachment', '--no-update', '--non-interactive', 195579, '--port=chromium-xvfb']" exit_code: 2 cwd: /mnt/git/webkit-commit-queue > > > > Last 500 characters of output: > > 2 out of 2 hunks FAILED -- saving rejects to file Source/WebCore/bindings/scripts/CodeGenerator.pm.rej > > patching file Source/WebCore/bindings/scripts/IDLParser.pm > > patching file Source/WebCore/bindings/scripts/generate-bindings.pl > > patching file Source/WebCore/bindings/scripts/generate-preprocessed-idls.pl > > patching file Source/WebCore/bindings/scripts/preprocessor.pm > > > > Failed to run "[u'/mnt/git/webkit-commit-queue/Tools/Scripts/svn-apply', '--force']" exit_code: 1 cwd: /mnt/git/webkit-commit-queue > > > > Full output:
http://webkit-commit-queue.appspot.com/results/17342159
> > Ah, looks like it's because of
http://trac.webkit.org/changeset/147041/trunk/Source/WebCore/bindings/scripts/CodeGenerator.pm
which I also had to modify here. > > I only rebased to the Chromium DEPS revision. I will wait until WebKit is rolled in Chromium and then try again.
If you're on old git, you can `git checkout master; git pull; git rebase master yourbranch` and reupload to rebase to webkit head (and then `git checkout gclient` again to get back to deps)
Scott Graham
Comment 30
2013-03-28 09:58:54 PDT
Created
attachment 195587
[details]
rebase again
WebKit Review Bot
Comment 31
2013-03-28 10:25:17 PDT
Comment on
attachment 195587
[details]
rebase again Clearing flags on attachment: 195587 Committed
r147130
: <
http://trac.webkit.org/changeset/147130
>
WebKit Review Bot
Comment 32
2013-03-28 10:25:22 PDT
All reviewed patches have been landed. Closing bug.
WebKit Review Bot
Comment 33
2013-03-28 14:54:20 PDT
Re-opened since this is blocked by
bug 113539
Tony Chang
Comment 34
2013-03-28 16:31:16 PDT
This patch is now unnecessary.
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug