For libwebrtc compilation we are just searching the libraries and the errors when they are not found are not very informative without this.
Created attachment 337066 [details] Patch
Created attachment 337069 [details] Patch
Created attachment 337072 [details] Patch
Comment on attachment 337072 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=337072&action=review But they're clearly only used by libwebrtc, so these don't belong outside Source/ThirdParty/libwebrtc. > Source/cmake/OptionsGTK.cmake:240 > + find_package(Vpx 1.7.0) > + if (NOT LIBVPX_FOUND) > + message(FATAL_ERROR "vpx is needed for USE_LIBWEBRTC.") > + endif () > + find_package(LibEvent) > + if (NOT LIBEVENT_FOUND) > + message(FATAL_ERROR "libevent is needed for USE_LIBWEBRTC.") > + endif () I think you want to do something like this, but in Source/ThirdParty/libwebrtc/CMakeLists.txt instead.
(In reply to Michael Catanzaro from comment #4) > Comment on attachment 337072 [details] > Patch > > View in context: > https://bugs.webkit.org/attachment.cgi?id=337072&action=review > > But they're clearly only used by libwebrtc, so these don't belong outside > Source/ThirdParty/libwebrtc. > > > Source/cmake/OptionsGTK.cmake:240 > > + find_package(Vpx 1.7.0) > > + if (NOT LIBVPX_FOUND) > > + message(FATAL_ERROR "vpx is needed for USE_LIBWEBRTC.") > > + endif () > > + find_package(LibEvent) > > + if (NOT LIBEVENT_FOUND) > > + message(FATAL_ERROR "libevent is needed for USE_LIBWEBRTC.") > > + endif () > > I think you want to do something like this, but in > Source/ThirdParty/libwebrtc/CMakeLists.txt instead. That is an option, but I saw libraries in thirdparty directory were doing it here and carlos recommended to use optionsgtk, so not sure about it :-). Maybe Carlos can comment about his preferences.
I don't care where the checks are as along as the error message is understandable, with the current one it's not obvious what is missing and why that is required.
Created attachment 337152 [details] Patch
I moved the detection to the libwebrtc directory, even if it seems others are doing it in OptionsGTK I agree with Michael it is better to add them in the libwebrtc/CMakeList.txt. Thanks for the comments!
Comment on attachment 337152 [details] Patch Can't you move the find modules, too? What other ThirdParty modules have checks for their dependencies in OptionsGTK.cmake? That shouldn't be. I don't see any.
Created attachment 337257 [details] Patch for landing
(In reply to Michael Catanzaro from comment #9) > Comment on attachment 337152 [details] > Patch > > Can't you move the find modules, too? > I did, they are in a cmake directory inside libwebrtc directory. > What other ThirdParty modules have checks for their dependencies in > OptionsGTK.cmake? That shouldn't be. I don't see any. Your are right, just checked more carefully and what I thought they were thirdparties now they are system dependencies, sorry about that. Thanks for the review!
Comment on attachment 337257 [details] Patch for landing Clearing flags on attachment: 337257 Committed r230297: <https://trac.webkit.org/changeset/230297>
All reviewed patches have been landed. Closing bug.
<rdar://problem/39201847>