RESOLVED FIXED 164170
[GTK] Build break by missing geoclue-2.0.
https://bugs.webkit.org/show_bug.cgi?id=164170
Summary [GTK] Build break by missing geoclue-2.0.
Hyowon Kim
Reported 2016-10-28 18:55:46 PDT
I got the following build error on my initial build for WebkitGTK. -- Checking for module 'geoclue-2.0' -- No package 'geoclue-2.0' found -- Could NOT find GEOCLUE2 (missing: VERSION_OK) -- Checking for module 'geoclue' -- No package 'geoclue' found -- Could NOT find GEOCLUE (missing: GEOCLUE_INCLUDE_DIRS GEOCLUE_LIBRARIES) CMake Error at Source/cmake/OptionsGTK.cmake:253 (message): Geoclue is needed for ENABLE_GEOLOCATION. Call Stack (most recent call first): Source/cmake/WebKitCommon.cmake:47 (include) CMakeLists.txt:137 (include)
Attachments
Patch (1.12 KB, patch)
2016-10-28 19:02 PDT, Hyowon Kim
no flags
Patch (1.15 KB, patch)
2016-10-30 04:53 PDT, Hyowon Kim
no flags
Hyowon Kim
Comment 1 2016-10-28 19:02:08 PDT
Michael Catanzaro
Comment 2 2016-10-29 08:05:08 PDT
Comment on attachment 293266 [details] Patch But we do have this function: $(aptIfElse libgeoclue-2-dev libgeoclue-dev) \ I guess it is broken for you for some reason, can you debug it? The pkg-config file should be in the -dev package, so there's no value in installing the base package explicitly.
Hyowon Kim
Comment 3 2016-10-30 04:53:11 PDT
Hyowon Kim
Comment 4 2016-10-30 04:53:58 PDT
(In reply to comment #2) > Comment on attachment 293266 [details] > Patch > > But we do have this function: > > $(aptIfElse libgeoclue-2-dev libgeoclue-dev) \ > > I guess it is broken for you for some reason, can you debug it? > > The pkg-config file should be in the -dev package, so there's no value in > installing the base package explicitly. It seems the pc file name installed by libgeoclue-2-dev is libgeoclue-2, not geoclue-2. Is my new patch an appropriate correction?
Michael Catanzaro
Comment 5 2016-10-30 08:35:57 PDT
Comment on attachment 293345 [details] Patch Ah, yes! Good catch! Seems that geoclue-2.0.pc ensures the D-Bus service exists (and it's what we use to find it in PlatformGTK.cmake). libgeoclue-2.0 is for the library and it's indeed what we're supposed to use here.
Michael Catanzaro
Comment 6 2016-10-30 08:47:31 PDT
So the reason it builds at all is simple: we do not actually use libgeoclue. Unfortunately while this commit is clearly correct -- the FindGeoClue2 module was broken -- it does mean we're now going to be unnecessarily linking against libgeoclue. :( And it's slightly tricky to fix, because we still have code to support geoclue1, and that code really does need to link to (the obsolete) libgeoclue. But we don't need to support geoclue1 anymore, since geoclue2 has been around for a long time. So I think we should remove the geoclue1 support entirely, and then we can also remove this unnecessary find module and stop linking to libgeoclue. I filed bug #164205 for this. I still give your patch r+, since the Find module was broken and that's what you were fixing.
WebKit Commit Bot
Comment 7 2016-10-30 08:59:20 PDT
Comment on attachment 293345 [details] Patch Clearing flags on attachment: 293345 Committed r208127: <http://trac.webkit.org/changeset/208127>
WebKit Commit Bot
Comment 8 2016-10-30 08:59:24 PDT
All reviewed patches have been landed. Closing bug.
Note You need to log in before you can comment on or make changes to this bug.