Moved library setup code to WEBKIT_FRAMEWORK macro.
Created attachment 271750 [details] Patch
Created attachment 271785 [details] Patch
Created attachment 271798 [details] Patch
Created attachment 271855 [details] Patch
Created attachment 271860 [details] Patch
Comment on attachment 271860 [details] Patch I think the value of moving this duplicate code to a macro probably outweighs the value of being explicit, so I'm OK with this. I want to give Alex and Martin a chance to chime in, though. I'm not sure about getting rid of WebKit2_DERIVED_SOURCES, because I think we might want to set different CPPFLAGS there to silence some warnings we currently have. And Alex has a patch introducing WebCore_DERIVED_SOURCES in bug #151399.
(In reply to comment #6) > I think the value of moving this duplicate code to a macro probably > outweighs the value of being explicit, so I'm OK with this. It seems to me that there is established set of "standard" variables for each WebKit target that configure it. So this patch would not only reduce code size, but would also help to make target behavior more consistent in future. I'm planning to submit similar patches for binaries and non-framework libraries. Also, it would simplify addition of new configurable properties, e.g. in Qt port we would like to override precompiled header files via variables like WebCore_PRECOMPILED_HEADER. I want to give > Alex and Martin a chance to chime in, though. > > I'm not sure about getting rid of WebKit2_DERIVED_SOURCES, because I think > we might want to set different CPPFLAGS there to silence some warnings we > currently have. And Alex has a patch introducing WebCore_DERIVED_SOURCES in > bug #151399. Note that he adds WebCore_DERIVEDSOURCES, not WebCore_DERIVED_SOURCES, breaking "convention" of WebKit2. If we really need to use different flags on derived sources, I will add ${${_target}_DERIVED_SOURCES} to source list of target.
Created attachment 271864 [details] Patch
Restored DERIVED_SOURCES.
Comment on attachment 271864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271864&action=review > Source/WebKit2/CMakeLists.txt:-789 > - set_target_properties(WebKit2 PROPERTIES OUTPUT_NAME ${WebKit2_OUTPUT_NAME}) I was under the impression set_target_properties only worked after the target had been made with add_library already. This is only used on Windows to my knowledge, and Windows EWS would report a successful build with different output names, which we would not want.
Comment on attachment 271864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271864&action=review >> Source/WebKit2/CMakeLists.txt:-789 >> - set_target_properties(WebKit2 PROPERTIES OUTPUT_NAME ${WebKit2_OUTPUT_NAME}) > > I was under the impression set_target_properties only worked after the target had been made with add_library already. This is only used on Windows to my knowledge, and Windows EWS would report a successful build with different output names, which we would not want. GTK+ port uses WebKit2_OUTPUT_NAME, and I've verified that it works.
Comment on attachment 271864 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=271864&action=review >>> Source/WebKit2/CMakeLists.txt:-789 >>> - set_target_properties(WebKit2 PROPERTIES OUTPUT_NAME ${WebKit2_OUTPUT_NAME}) >> >> I was under the impression set_target_properties only worked after the target had been made with add_library already. This is only used on Windows to my knowledge, and Windows EWS would report a successful build with different output names, which we would not want. > > GTK+ port uses WebKit2_OUTPUT_NAME, and I've verified that it works. ok, sounds good. I'll take a close look at the output after this lands, but I don't anticipate any problems, then.
Comment on attachment 271864 [details] Patch Clearing flags on attachment: 271864 Committed r196947: <http://trac.webkit.org/changeset/196947>
All reviewed patches have been landed. Closing bug.