Bug 156400

Summary: Touching any IDL files rebuilds all bindings in CMake Ninja build
Product: WebKit Reporter: Fujii Hironori <Hironori.Fujii>
Component: WebKit Misc.Assignee: Nobody <webkit-unassigned>
Status: RESOLVED FIXED    
Severity: Normal CC: achristensen, andersca, bfulgham, cdumez, cgarcia, commit-queue, ggaren
Priority: P2    
Version: WebKit Nightly Build   
Hardware: Unspecified   
OS: Unspecified   
See Also: https://bugs.webkit.org/show_bug.cgi?id=117708
Attachments:
Description Flags
Patch none

Description Fujii Hironori 2016-04-08 03:15:45 PDT
Touching any IDL files rebuilds all bindings in CMake Ninja build

preprocess-idls.pl has a function WriteFileIfChanged not to
update files unnecessary to be updated.

preprocess-idls.pl outputs four files supplemental_dependency.tmp,
DOMWindowConstructors.idl, WorkerGlobalScopeConstructors.idl and
DedicatedWorkerGlobalScopeConstructors.idl.  WriteFileIfChanged works
only for DedicatedWorkerGlobalScopeConstructors.idl.

It does not work due to flaky results of 'keys'.
Results of 'keys' are flaky, should be processed by 'sort'.

I tested with GTK port Ninja build:

> ./Tools/Scripts/build-webkit --gtk
> touch Source/WebCore/dom/Document.idl
Comment 1 Fujii Hironori 2016-04-08 03:28:49 PDT
Created attachment 275989 [details]
Patch
Comment 2 Brent Fulgham 2016-04-08 10:15:27 PDT
Comment on attachment 275989 [details]
Patch

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

> Source/WebCore/ChangeLog:11
> +        Sort results of 'keys'.

While this patch doesn't seem risky, I'm not sure I understand why this helps. Clearly your change makes the order of file processing consistent, which is probably a good thing, but It's not obvious to me why this stops us from rebuilding all bindings.
Comment 3 Brent Fulgham 2016-04-08 10:34:35 PDT
Comment on attachment 275989 [details]
Patch

Questions aside, I can confirm that this also fixes my Windows regeneration problem, so THANK YOU!
Comment 4 WebKit Commit Bot 2016-04-08 11:28:27 PDT
Comment on attachment 275989 [details]
Patch

Clearing flags on attachment: 275989

Committed r199237: <http://trac.webkit.org/changeset/199237>
Comment 5 WebKit Commit Bot 2016-04-08 11:28:34 PDT
All reviewed patches have been landed.  Closing bug.
Comment 6 Fujii Hironori 2016-04-10 18:50:28 PDT
Thank you for reviewing my patch.

(In reply to comment #2)
> but It's not obvious to me why this stops us
> from rebuilding all bindings.

1) supplemental_dependency.tmp depens on all IDL
2) All bindgs depnds on supplemental_dependency.tmp

In Source/WebCore/CMakeLists.txt:

> add_custom_command(
>     OUTPUT ${SUPPLEMENTAL_DEPENDENCY_FILE} ${WINDOW_CONSTRUCTORS_FILE} ${WORKERGLOBALSCOPE_CONSTRUCTORS_FILE} ${DEDICATEDWORKERGLOBALSCOPE_CONSTRUCTORS_FILE}
>     DEPENDS ${WEBCORE_DIR}/bindings/scripts/preprocess-idls.pl ${SCRIPTS_PREPROCESS_IDLS} ${WebCore_IDL_FILES} ${ObjC_Bindings_IDL_FILES} ${WebCoreTestSupport_IDL_FILES} ${WEBCORE_DIR}/CMakeLists.txt
>     COMMAND ${PERL_EXECUTABLE} -I${WEBCORE_DIR}/bindings/scripts ${WEBCORE_DIR}/bindings/scripts/preprocess-idls.pl --defines "${FEATURE_DEFINES_JAVASCRIPT}" --idlFilesList ${IDL_FILES_TMP} --supplementalDependencyFile ${SUPPLEMENTAL_DEPENDENCY_FILE} --windowConstructorsFile ${WINDOW_CONSTRUCTORS_FILE} --workerGlobalScopeConstructorsFile ${WORKERGLOBALSCOPE_CONSTRUCTORS_FILE}
> --dedicatedWorkerGlobalScopeConstructorsFile ${DEDICATEDWORKERGLOBALSCOPE_CONSTRUCTORS_FILE}
>     VERBATIM)

> GENERATE_BINDINGS(WebCore_DERIVED_SOURCES
>     "${WebCore_IDL_FILES}"
>     "${WEBCORE_DIR}"
>     "${IDL_INCLUDES}"
>     "${FEATURE_DEFINES_JAVASCRIPT}"
>     ${DERIVED_SOURCES_WEBCORE_DIR} JS JS cpp
>     ${IDL_ATTRIBUTES_FILE}
>     ${SUPPLEMENTAL_DEPENDENCY_FILE}
>     ${ADDITIONAL_BINDINGS_DEPENDENCIES})