WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
RESOLVED FIXED
140756
makeprop.pl script is called too many times during build
https://bugs.webkit.org/show_bug.cgi?id=140756
Summary
makeprop.pl script is called too many times during build
Chris Dumez
Reported
2015-01-21 20:41:12 PST
makeprop.pl script is called 4 times (instead of once) during build due to the following Makefile rule: CSSPropertyNames.h StyleBuilder.cpp StylePropertyShorthandFunctions.h StylePropertyShorthandFunctions.cpp : $(WEBCORE_CSS_PROPERTY_NAMES) css/makeprop.pl bindings/scripts/preprocessor.pm $(PLATFORM_FEATURE_DEFINES) $(PERL) -pe '' $(WEBCORE_CSS_PROPERTY_NAMES) > CSSPropertyNames.in $(PERL) -I$(WebCore)/bindings/scripts "$(WebCore)/css/makeprop.pl" --defines "$(FEATURE_DEFINES)" Having 4 targets here is wrong as makeprop.pl only needs to be called once to generate these 4 files. Currently, we are calling makeprop.pl once per target and therefore generating all these files 4 times. May be related to <
rdar://problem/19467942
>.
Attachments
Patch
(2.93 KB, patch)
2015-01-21 21:04 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Patch
(3.69 KB, patch)
2015-01-21 22:58 PST
,
Chris Dumez
no flags
Details
Formatted Diff
Diff
Show Obsolete
(1)
View All
Add attachment
proposed patch, testcase, etc.
Chris Dumez
Comment 1
2015-01-21 20:49:50 PST
I am thinking this may be the source of the following build error we get sometimes: CSSPropertyNames.gperf: No keywords in input file! calling gperf failed: 256 at WebCore/css/makeprop.pl line 1036. In particular, since WebCore/css/makeprop.pl is executed 4 times, I am worried that the script may be executed several times in parallel, which would explain why gperf is complaining (because another instance of makeprop.pl may be writing to gperf's its input file). makeprop.pl first generates gperf's input file then calls gperf on it at the end.
Chris Dumez
Comment 2
2015-01-21 21:04:47 PST
Created
attachment 245117
[details]
Patch
David Kilzer (:ddkilzer)
Comment 3
2015-01-21 21:59:27 PST
Comment on
attachment 245117
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=245117&action=review
> Source/WebCore/DerivedSources.make:820 > +CSSPropertyNames.h : $(WEBCORE_CSS_PROPERTY_NAMES) css/makeprop.pl bindings/scripts/preprocessor.pm $(PLATFORM_FEATURE_DEFINES)
This removes the dependencies on the other three files. I think you need some dummy rules to make sure any one missing file causes them all to be regenerated per:
http://stackoverflow.com/questions/2973445/gnu-makefile-rule-generating-a-few-targets-from-a-single-source-file
Chris Dumez
Comment 4
2015-01-21 22:10:36 PST
Comment on
attachment 245117
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=245117&action=review
>> Source/WebCore/DerivedSources.make:820 >> +CSSPropertyNames.h : $(WEBCORE_CSS_PROPERTY_NAMES) css/makeprop.pl bindings/scripts/preprocessor.pm $(PLATFORM_FEATURE_DEFINES) > > This removes the dependencies on the other three files. I think you need some dummy rules to make sure any one missing file causes them all to be regenerated per: >
http://stackoverflow.com/questions/2973445/gnu-makefile-rule-generating-a-few-targets-from-a-single-source-file
If this is an issue, this is a pre-existing one. For example, notice that CSSPropertyNames.cpp is not mentioned anywhere in this file even though it is also generated by makeprop.pl script as well. We never had any issue with CSSPropertyNames.cpp AFAIK. However, right now, makeprop.pl is called 4 times for no reason. Similarly, CSSValueKeywords.cpp is generated by makevalues.pl a few lines below but only CSSValueKeywords.h is mentioned in this file.
Chris Dumez
Comment 5
2015-01-21 22:24:12 PST
Comment on
attachment 245117
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=245117&action=review
>>> Source/WebCore/DerivedSources.make:820 >>> +CSSPropertyNames.h : $(WEBCORE_CSS_PROPERTY_NAMES) css/makeprop.pl bindings/scripts/preprocessor.pm $(PLATFORM_FEATURE_DEFINES) >> >> This removes the dependencies on the other three files. I think you need some dummy rules to make sure any one missing file causes them all to be regenerated per: >>
http://stackoverflow.com/questions/2973445/gnu-makefile-rule-generating-a-few-targets-from-a-single-source-file
> > If this is an issue, this is a pre-existing one. For example, notice that CSSPropertyNames.cpp is not mentioned anywhere in this file even though it is also generated by makeprop.pl script as well. We never had any issue with CSSPropertyNames.cpp AFAIK. However, right now, makeprop.pl is called 4 times for no reason. > > Similarly, CSSValueKeywords.cpp is generated by makevalues.pl a few lines below but only CSSValueKeywords.h is mentioned in this file.
The stack overflow solution is not perfect either: CSSPropertyNames.h : $(WEBCORE_CSS_PROPERTY_NAMES) css/makeprop.pl bindings/scripts/preprocessor.pm $(PLATFORM_FEATURE_DEFINES) makeprop.pl ... StyleBuilder.cpp : CSSPropertyNames.h noop If you delete StyleBuilder.cpp but CSSPropertyNames.h is still there, then it still won't regenerate StyleBuilder.cpp, will it? Or am I missing something.
Chris Dumez
Comment 6
2015-01-21 22:58:13 PST
Created
attachment 245122
[details]
Patch
Chris Dumez
Comment 7
2015-01-21 23:01:04 PST
Comment on
attachment 245122
[details]
Patch Hopefully this addresses your comments. I have verified manually that if I delete CSSPropertyNames.cpp, makeprop.pl correctly gets called to regenerate it. makeprop.pl is also only called once.
Chris Dumez
Comment 8
2015-01-21 23:04:14 PST
Comment on
attachment 245122
[details]
Patch View in context:
https://bugs.webkit.org/attachment.cgi?id=245122&action=review
> Source/WebCore/DerivedSources.make:826 > +.INTERMEDIATE : makeprop.intermediate
Doc:
https://www.gnu.org/software/make/manual/html_node/Special-Targets.html
This was also one of the solutions proposed on the stack overflow thread you provided:
http://stackoverflow.com/a/10609434
David Kilzer (:ddkilzer)
Comment 9
2015-01-22 11:37:09 PST
Comment on
attachment 245122
[details]
Patch r=me!
Chris Dumez
Comment 10
2015-01-22 11:38:37 PST
Comment on
attachment 245122
[details]
Patch Clearing flags on attachment: 245122 Committed
r178929
: <
http://trac.webkit.org/changeset/178929
>
Chris Dumez
Comment 11
2015-01-22 11:38:41 PST
All reviewed patches have been landed. Closing bug.
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