Bug 140756 – makeprop.pl script is called too many times during build

• Component: CSS
• Status: RESOLVED FIXED

[Chris Dumez]

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

[Chris Dumez]

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]

Created attachment 245117 [details]
Patch

[David Kilzer (:ddkilzer)]

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 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 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]

Created attachment 245122 [details]
Patch

[Chris Dumez]

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 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 on attachment 245122 [details]
Patch

r=me!

[Chris Dumez]

Comment on attachment 245122 [details]
Patch

Clearing flags on attachment: 245122

Committed r178929: <http://trac.webkit.org/changeset/178929>

[Chris Dumez]

All reviewed patches have been landed.  Closing bug.