[webkit-reviews] review granted: [Bug 217696] Lessen the reliance on VPATH in WebCore/DerivedSources.make : [Attachment 411303] Patch

[bugzilla-daemon at webkit.org]

Darin Adler <darin at apple.com> has granted Keith Rollin <krollin at apple.com>'s
request for review:
Bug 217696: Lessen the reliance on VPATH in WebCore/DerivedSources.make
https://bugs.webkit.org/show_bug.cgi?id=217696

Attachment 411303: Patch

https://bugs.webkit.org/attachment.cgi?id=411303&action=review




--- Comment #5 from Darin Adler <darin at apple.com> ---
Comment on attachment 411303
  --> https://bugs.webkit.org/attachment.cgi?id=411303
Patch

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

I wonder if we should use $@ and $< more often to avoid repeating path names.

> Source/WebCore/DerivedSources.make:1299
> +# in ADDITIONAL_BINDING_IDLS_PATHS and the existance of the IDL file at each

existence misspelled here

> Source/WebCore/DerivedSources.make:1316
> +	       $(path)/usr/local/include/WebKitAdditions/$(idl)))))

Was there an explicit version of this /usr/local/include path already present
elsewhere in this makefile?

> Source/WebCore/DerivedSources.make:1850
> -WebCore_BUILTINS_SOURCES_LIST :
$(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py DerivedSources.make
> +WebCore_BUILTINS_SOURCES_LIST :
$(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py
$(FEATURE_AND_PLATFORM_DEFINE_DEPENDENCIES)

I don’t think this is right. I plan to eventually replace
FEATURE_AND_PLATFORM_DEFINE_DEPENDENCIES with a different approach; a single
generated file. But specifically here, is the dependency really on feature and
platform defines? Or is there some other reason to rebuild any time we touch
the makefile?

> Source/WebCore/DerivedSources.make:1853
> -WebCore_BUILTINS_DEPENDENCIES_LIST :
$(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py DerivedSources.make
> +WebCore_BUILTINS_DEPENDENCIES_LIST :
$(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py
$(FEATURE_AND_PLATFORM_DEFINE_DEPENDENCIES)

Ditto.