Bug 142284 – [Win] Missing dependency checking in WebCore/DerivedSources.make for WebCore/css/CSSValueKeywords.in

• Component: WebKit Misc.
• Status: RESOLVED FIXED

[Brent Fulgham]

The Windows build frequently fails when new features are activated (or deactivated) because of improper dependency checks. This results in a lot of manual maintenance (forcing clean bids) as well as extended periods of broken builds until someone notices.

[Brent Fulgham]

David Kilzer suggested a fix:

A clean build should fix this, but something like this (UNTESTED) might also fix it:

diff --git a/Source/WebCore/DerivedSources.make b/Source/WebCore/DerivedSources.make
index d91ff21..f782eb5 100644
--- a/Source/WebCore/DerivedSources.make
+++ b/Source/WebCore/DerivedSources.make
@@ -717,8 +717,12 @@ ifeq ($(shell $(CC) -std=gnu++11 -x c++ -E -P -dM $(SDK_FLAGS) $(FRAMEWORK_FLAGS
 endif
 
 ifeq ($(PLATFORM_FEATURE_DEFINES),)
+ifeq ($(OS),Windows*)
+PLATFORM_FEATURE_DEFINES = ../../WebKitLibraries/win/tools/vsprops/FeatureDefines.props
+else
 PLATFORM_FEATURE_DEFINES = Configurations/FeatureDefines.xcconfig
 endif
+endif
 
 ifeq ($(WTF_PLATFORM_IOS), 1)
 ADDITIONAL_BINDING_IDLS =

Why I didn’t land this as a build fix:

- I’m not sure how to fix WinCairo other than to list its FeatureDefinesCairo.props file as well.  (Probably okay.)
- Are we allowed to reference files in WebKitLibraries from Source/WebCore? Seems like bad form to reference files outside of Source/WebCore, but maybe Windows builds in production with the entire svn directory???

[Brent Fulgham]

(In reply to comment #1)
> - I’m not sure how to fix WinCairo other than to list its
> FeatureDefinesCairo.props file as well.  (Probably okay.)

Usually the two are kept in sync, so it might be reasonable to start off with just the FeatureDefines.props. Per might have ideas on how to make this better as well. But for now, I suggest we just add FeatureDefines.props and see how substantially this improves things.

> - Are we allowed to reference files in WebKitLibraries from Source/WebCore?
> Seems like bad form to reference files outside of Source/WebCore, but maybe
> Windows builds in production with the entire svn directory???

No, but FeatureDefines.props is always available in the location $WEBKIT_LIBRARIES/tools/vsprops/FeatureDefines.props, so we should be able to add this rule.

[Brent Fulgham]

Created attachment 247866 [details]
Patch

[David Kilzer (:ddkilzer)]

Comment on attachment 247866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247866&action=review

r=me!

> Source/WebCore/ChangeLog:10
> +        1. Modify DerivedSources.make to declarare a default dependency (on Windows)

Typo:  declarare

> Source/WebCore/DerivedSources.make:725
>  ifeq ($(PLATFORM_FEATURE_DEFINES),)
> +ifeq ($(OS), Windows*)
> +PLATFORM_FEATURE_DEFINES = $(WEBKIT_LIBRARIES)/tools/vsprops/FeatureDefines.props
> +else
>  PLATFORM_FEATURE_DEFINES = Configurations/FeatureDefines.xcconfig
>  endif
> +endif

Do we need this change if the build-generated-files.pl script already sets this for us?

[Brent Fulgham]

Comment on attachment 247866 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=247866&action=review

>> Source/WebCore/ChangeLog:10
>> +        1. Modify DerivedSources.make to declarare a default dependency (on Windows)
> 
> Typo:  declarare

Whoops!

>> Source/WebCore/DerivedSources.make:725
>> +endif
> 
> Do we need this change if the build-generated-files.pl script already sets this for us?

Well, it's just a fallback in case the "build-generated-files.pl" thing ever gets broken. I'm not sure it's any different than the existing default FeatureDefines.xcconf statement that it is based on.

[Brent Fulgham]

Committed r181002: <http://trac.webkit.org/changeset/181002>