Bug 156400 - Touching any IDL files rebuilds all bindings in CMake Ninja build
Summary: Touching any IDL files rebuilds all bindings in CMake Ninja build
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: WebKit Misc. (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Nobody
URL:
Keywords:
Depends on:
Blocks:
 
Reported: 2016-04-08 03:15 PDT by Fujii Hironori
Modified: 2016-04-10 18:50 PDT (History)
7 users (show)

See Also:


Attachments
Patch (2.67 KB, patch)
2016-04-08 03:28 PDT, Fujii Hironori
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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})