Addng a new WebCore JavaScript built-in source file does not trigger rebuild of WebCoreJSBuiltins*
Created attachment 284405 [details] Patch
I ran into this when I checked out the latest sources but they failed to link because XMLHttpRequest was not included in WebCoreJSBuiltins.cpp.
Comment on attachment 284405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284405&action=review Thanks! > Source/WebCore/DerivedSources.make:1334 > + $(PYTHON) $(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py '$(WebCore_BUILTINS_SOURCES)' $@ Why is this needed? > Source/WebCore/DerivedSources.make:1339 > +$(firstword $(WebCore_BUILTINS_WRAPPERS)): WebCore_BUILTINS_SOURCES_LIST $(BUILTINS_GENERATOR_SCRIPTS) WebCore_BUILTINS_DEPENDENCIES_LIST Cannot we just use $(WebCore_BUILTINS_SOURCES) here?
Created attachment 284444 [details] Patch for landing
(In reply to comment #4) > Created attachment 284444 [details] > Patch for landing Updated bug title in log.
Comment on attachment 284444 [details] Patch for landing Clearing flags on attachment: 284444 Committed r203661: <http://trac.webkit.org/changeset/203661>
All reviewed patches have been landed. Closing bug.
Comment on attachment 284405 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=284405&action=review >> Source/WebCore/DerivedSources.make:1334 >> + $(PYTHON) $(JavaScriptCore_SCRIPTS_DIR)/UpdateContents.py '$(WebCore_BUILTINS_SOURCES)' $@ > > Why is this needed? Because generating the wrappers depends on the *list* of JS files, not on the contents of those files, and further it needs to depend on removals as well as additions to that list. This technique gives us a file that changes when the filenames in the list change. >> Source/WebCore/DerivedSources.make:1339 >> +$(firstword $(WebCore_BUILTINS_WRAPPERS)): WebCore_BUILTINS_SOURCES_LIST $(BUILTINS_GENERATOR_SCRIPTS) WebCore_BUILTINS_DEPENDENCIES_LIST > > Cannot we just use $(WebCore_BUILTINS_SOURCES) here? No. Doing that would rebuild: 1) Too often. Any time we changed the contents of one of the sources. 2) Not often enough. Would not trigger a rebuild if we removed a file from the list.
(In reply to comment #8) > Comment on attachment 284405 [details] > Patch > > >> Source/WebCore/DerivedSources.make:1339 > >> +$(firstword $(WebCore_BUILTINS_WRAPPERS)): WebCore_BUILTINS_SOURCES_LIST $(BUILTINS_GENERATOR_SCRIPTS) WebCore_BUILTINS_DEPENDENCIES_LIST > > > > Cannot we just use $(WebCore_BUILTINS_SOURCES) here? > > No. Doing that would rebuild: > > 1) Too often. Any time we changed the contents of one of the sources. I see the point. That said, even if sources are changed, like adding a function, the header/source files will remain totally unchanged (lazy writer). So the perf loss is only the time spent to run the python generator. It does not trigger any compilation. In the future, if we remove the use of macros, we might want to update wrapper cpp files according source file content. We can revisit this if we ever go down that path Also, it might be good to think of the best way to limit the recompilation of cpp files in case of adding/removing a function. > 2) Not often enough. Would not trigger a rebuild if we removed a file from > the list. OK