As following a comment of Bug 87659, add GENERATE_BINDINGS macro to share the codes which use generate-bindings.pl.
Created attachment 150114 [details] Patch
Comment on attachment 150114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150114&action=review Thanks for working on this. git grep tells me that UseV8.cmake and PlatformBlackBerry.cmake also call generate-bindings.pl, it'd be good if you could generalize the macro to be able to use it in those files as well. > Source/WebCore/UseJSC.cmake:319 > + SET(WEBCORE_IDL_FILES_LIST ${WEBCORE_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) Is this change related? > Source/WebCore/UseJSC.cmake:324 > + SET(WebCoreTestSupport_IDL_FILES_LIST ${WebCoreTestSupport_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) Ditto. > Source/cmake/WebKitMacros.cmake:27 > +MACRO (GENERATE_BINDINGS _output_sources _input_files _idl_includes _destination _generator) It'd be good if you could add some documentation here. See, for example, http://trac.webkit.org/browser/trunk/Source/cmake/FindGStreamer.cmake#L54 > Source/cmake/WebKitMacros.cmake:34 > + FOREACH (f ${ARGN}) > + IF (NOT _supplemental_dependency) > + SET(_supplemental_dependency --supplementalDependencyFile ${f}) > + ENDIF () > + ENDFOREACH () So there can be at most one supplemental dependency?
(In reply to comment #2) > (From update of attachment 150114 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150114&action=review > > Thanks for working on this. git grep tells me that UseV8.cmake and PlatformBlackBerry.cmake also call generate-bindings.pl, it'd be good if you could generalize the macro to be able to use it in those files as well. > OK, I will. > > Source/WebCore/UseJSC.cmake:319 > > + SET(WEBCORE_IDL_FILES_LIST ${WEBCORE_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) > > Is this change related? > > > Source/WebCore/UseJSC.cmake:324 > > + SET(WebCoreTestSupport_IDL_FILES_LIST ${WebCoreTestSupport_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) > > Ditto. > It's because GENERATE_BINDINGS macro should not limit the IDL path to WEBCORE_DIR. As alternatives, I think that each item in IDL lists should be absolute path. I can create new bug for it because this changes are huge. > > Source/cmake/WebKitMacros.cmake:27 > > +MACRO (GENERATE_BINDINGS _output_sources _input_files _idl_includes _destination _generator) > > It'd be good if you could add some documentation here. See, for example, http://trac.webkit.org/browser/trunk/Source/cmake/FindGStreamer.cmake#L54 Thank you, I will try. > > > Source/cmake/WebKitMacros.cmake:34 > > + FOREACH (f ${ARGN}) > > + IF (NOT _supplemental_dependency) > > + SET(_supplemental_dependency --supplementalDependencyFile ${f}) > > + ENDIF () > > + ENDFOREACH () > > So there can be at most one supplemental dependency? I don't know supplemental dependency enough. But If I am right, there is one supplemental dependency file. I want that this is an optional argument.
Comment on attachment 150114 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=150114&action=review >>> Source/WebCore/UseJSC.cmake:319 >>> + SET(WEBCORE_IDL_FILES_LIST ${WEBCORE_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) >> >> Is this change related? > > It's because GENERATE_BINDINGS macro should not limit the IDL path to WEBCORE_DIR. > > As alternatives, I think that each item in IDL lists should be absolute path. > I can create new bug for it because this changes are huge. Well, it's easier to adjust the macro. If necessary, pass the base directory the script should be called from. >>> Source/cmake/WebKitMacros.cmake:34 >>> + ENDFOREACH () >> >> So there can be at most one supplemental dependency? > > I don't know supplemental dependency enough. > But If I am right, there is one supplemental dependency file. > I want that this is an optional argument. If there can be only one (please check the supplemental page in Trac), you can simply get the first argument instead of needlessly looping.
Created attachment 150698 [details] Patch
I gave this a try, works fine locally.
(In reply to comment #4) > (From update of attachment 150114 [details]) > View in context: https://bugs.webkit.org/attachment.cgi?id=150114&action=review > > >>> Source/WebCore/UseJSC.cmake:319 > >>> + SET(WEBCORE_IDL_FILES_LIST ${WEBCORE_IDL_FILES_LIST} ${WEBCORE_DIR}/${_idl}) > >> > >> Is this change related? > > > > It's because GENERATE_BINDINGS macro should not limit the IDL path to WEBCORE_DIR. > > > > As alternatives, I think that each item in IDL lists should be absolute path. > > I can create new bug for it because this changes are huge. > > Well, it's easier to adjust the macro. If necessary, pass the base directory the script should be called from. OK, I tried like you mentioned. > > >>> Source/cmake/WebKitMacros.cmake:34 > >>> + ENDFOREACH () > >> > >> So there can be at most one supplemental dependency? > > > > I don't know supplemental dependency enough. > > But If I am right, there is one supplemental dependency file. > > I want that this is an optional argument. > > If there can be only one (please check the supplemental page in Trac), you can simply get the first argument instead of needlessly looping. I checked scripts and supplemental file can be only one. I changed little bit.
Comment on attachment 150698 [details] Patch LGTM.
Comment on attachment 150698 [details] Patch Clearing flags on attachment: 150698 Committed r121857: <http://trac.webkit.org/changeset/121857>
All reviewed patches have been landed. Closing bug.
Could it be that this causes the generated sources now to be always rebuilt?
(In reply to comment #11) > Could it be that this causes the generated sources now to be always rebuilt? Oops I found typo. MAIN_DEPEND'D'ENCY. I will fix it.