Long time ago JSC related stuff were moved to a separated UseJSC.cmake from CMakeLists.txt to be able build with V8 instead of JSC - http://trac.webkit.org/changeset/81515 After Blink fork, JSC is the only one JavaScript engine in WebKit, so UseJSC.cmake can be moved back to CMakeLists.txt
Created attachment 227943 [details] Patch
Personally I prefer to keep UseJSC.cmake in order to reduce CMakeLists.txt workload. Besides someone might to wanna use V8 in future again. However, many people wanna back to it to CMakeLists.txt, I won't object to back to there.
Comment on attachment 227943 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=227943&action=review I agree with this change, but r- now due to some minor things. > Source/WebCore/CMakeLists.txt:2838 > + list(APPEND WebCore_SOURCES > + bindings/js/JSCustomSQLStatementErrorCallback.cpp > + bindings/js/JSSQLResultSetRowListCustom.cpp > + bindings/js/JSSQLTransactionCustom.cpp > + bindings/js/JSSQLTransactionSyncCustom.cpp > + ) Please move them after Modules/webdatabase/WorkerGlobalScopeWebDatabase.cpp - few lines above. > Source/WebCore/CMakeLists.txt:2875 > + bindings/js/JSWebGLRenderingContextCustom.cpp We don't need one more APPEND, just add it before html/canvas/ANGLEInstancedArrays.cpp. > Source/WebCore/CMakeLists.txt:2976 > + bindings/js/JSAudioTrackCustom.cpp > + bindings/js/JSAudioTrackListCustom.cpp > + bindings/js/JSTextTrackCueCustom.cpp > + bindings/js/JSTextTrackCustom.cpp > + bindings/js/JSTextTrackListCustom.cpp > + bindings/js/JSTrackCustom.cpp > + bindings/js/JSTrackEventCustom.cpp > + bindings/js/JSVideoTrackCustom.cpp > + bindings/js/JSVideoTrackListCustom.cpp > + ) > + > + list(APPEND WebCore_SOURCES ditto, just add them before html/track/AudioTrack.cpp > Source/WebCore/CMakeLists.txt:-3193 > - > -include(${WEBCORE_DIR}/UseJSC.cmake) > - I prefer moving the line 285 - end part of UseJSC.cmake to this place instead of splitting into several pieces.
(In reply to comment #2) > Personally I prefer to keep UseJSC.cmake in order to reduce CMakeLists.txt workload. Besides someone might to wanna use V8 in future again. However, many people wanna back to it to CMakeLists.txt, I won't object to back to there. This patch doesn't change workload at all, now CMakeLists.txt simple includes UseJSC.cmake unconditionally. After the Blink fork the probability of adding back V8 to WebKit as JS engine is near 0%, I'm sure Apple would have strong objection against it if somebody dreamt about V8 again. :) I think having only one cmake file for WebCore reduces the maintenance cost.
(In reply to comment #4) > (In reply to comment #2) > > Personally I prefer to keep UseJSC.cmake in order to reduce CMakeLists.txt workload. Besides someone might to wanna use V8 in future again. However, many people wanna back to it to CMakeLists.txt, I won't object to back to there. > > This patch doesn't change workload at all, now CMakeLists.txt > simple includes UseJSC.cmake unconditionally. > > After the Blink fork the probability of adding back V8 to WebKit as > JS engine is near 0%, I'm sure Apple would have strong objection > against it if somebody dreamt about V8 again. :) > > I think having only one cmake file for WebCore reduces the maintenance cost. If so, I don't wanna object to back to CMakeLists.txt. :)
I've realized, that UseJSC.cmake contained a lot of unnecessary conditions and duplicated files, so I've removed them before continuing to work on this merge issue. Bug report of mentioned issue: https://bugs.webkit.org/show_bug.cgi?id=131438
Created attachment 229030 [details] Patch
Comment on attachment 229030 [details] Patch LGTM
Comment on attachment 229030 [details] Patch Clearing flags on attachment: 229030 Committed r167071: <http://trac.webkit.org/changeset/167071>
All reviewed patches have been landed. Closing bug.
*** Bug 114131 has been marked as a duplicate of this bug. ***