GNOME Bugzilla – Bug 619908
[PATCH] Probably wrong pc file
Last modified: 2018-05-22 12:55:41 UTC
Is there a reason that a seperate champlain-memphis.pc file exists and that mephis is missing from the Requires section in champlain-0.6.pc? Especially that libchamplain-0.6.so is being linked to libmemphis: deejay1@sulaco:~/Desktop$ ldd /usr/lib/libchamplain-0.6.so | grep memphis libmemphis-0.2.so.0 => /usr/lib/libmemphis-0.2.so.0 (0x00007f12c60dd000)
Created attachment 162270 [details] [review] patch I guess this patch is ok
Uh, no, the way it's done is intentional. The thing is that libchamplain can be compiled without memphis support (--disable-memphis or you just don't have libmemphis installed when compiling libchamplain) and for applications there needs to be a way to determine whether the installed library is compiled with or without memphis. So if the user application requires memphis, it should use PKG_CHECK_MODULES(DEPS, [ champlain-memphis ]) in its configure.ac, if memphis support isn't required, it should use PKG_CHECK_MODULES(DEPS, [ champlain ]) I really want to avoid dependency explosion for libchamplain especially if it should depend on non-standard and non-mature libraries like libmemphis (the rest of the dependencies is usually already installed by default in your linux distribution). Actually I would recommend for linux distributions to install libchamplain binaries without libmemphis support - I think in the current state it's rather too experimental. I did this the same way as cairo does (you can have cairo installed with different rendering backends; in our case the backend is the map source). If you have a better idea how to achieve the goal to have some parts of libchamplain optional, please tell me.
I thought it might be something in that way, but in that case you have to make sure that the library is buildable without memphis support, which it currently isn't (fails on building the docs) otherwise distributions will include memphis, especially that memphis features are described at http://library.gnome.org/devel/libchamplain/unstable/ And yes, I know that cairo uses some wierd autofoo for that :/
Well I think it's not to upto the application that will use libchamplain to know if it's compiled with memphis support. If libchamplain is compiled with memphis and the application uses champlain.pc it will fails to build due to the missing library (memphis) at linking time. What use trying to do require call to dlopen and loading memphis at runtime AFAIK
@Łukasz: Good point, will fix the docs generation. I'll leave the full documentation available at the docs pages, but you're right that it should be somehow pointed out that the part of the interface is optional. @Laurent: Not sure if I understand your reply correctly: "If libchamplain is compiled with memphis and the application uses champlain.pc it will fails to build due to the missing library (memphis) at linking time." If memphis support is enabled for libchamplain then memphis has to be installed in the system, otherwise libchamplain wouldn't compile. If memphis is disabled for libchamplain, then champlain.pc will not be installed and you'll get error during compile-time if your application requires it. Of course it's up to the distributions to provide a consistent set of libraries (e.g. have memphis-enabled libchamplain distributed together with memphis).
Created attachment 162326 [details] [review] patch v2 As your public headers include memphis, you need to add it to the requires of you lib. If you really want to reduce needed library please see my new patch
I don't get it. How is it supposed to work? How will the user application be informed whether memphis-enabled libchamplain is installed? Currently if the user compiles libchamplain without memphis support, the memphis-related headers don't get installed. Yes, the inclusion of memphis.h in champlain-memphis-tile-source.h is unfortunate - forward declaration of MemphisRule should be enough here - I'll patch this. This also causes some problems for bindings, see http://mail.gnome.org/archives/libchamplain-list/2010-May/msg00038.html point 3 and should be fixed by hiding memphis completely in 0.8.
Laurent, can you address the questions in comment #7? TIA!
Jiri, I presume patch 16232 is rejected in its current form?
Oups I never answer. My point was that a library shouldn't change its public API when a flag is passed to the configure
@Tobias: do you mean the Laurent's patch? The one you link to is something totally unrelated to libchamplain. @Laurent: I took a very direct inspiration from cairo in this case. In cairo, you can have different backends (pdf, png, svn, ...) which may / may not be present. Their presence is determined during configure automatically but can be overridden by flags. Each backend has its own .pc file (e.g. cairo-pdf.pc). For each backend there is a macro determining whether the backend is present (e.g. CAIRO_HAS_PDF_SURFACE). I tried to achieve the very same thing in libchamplain - make the backends presence optional in libchamplain (and not having hard-dependency on libmemphis for example). It's slightly different in 0.8 (and 0.10) than it was in 0.6 - the optional backends are renderers now instead of map sources. So similarly to cairo, you can specify whether the library is compiled with libmemphis backend support (--enable-memphis), you have a separate .pc file for memphis-enabled libchamplain (champlain-memphis.pc) and a macro determining whether memphis support is available (CHAMPLAIN_HAS_MEMPHIS). Please let me know if you think there's a better way achieve this. (Thanks for cleaning up libchamplain's bugzilla by the way.)
-- GitLab Migration Automatic Message -- This bug has been migrated to GNOME's GitLab instance and has been closed from further activity. You can subscribe and participate further through the new bug through this link to our GitLab instance: https://gitlab.gnome.org/GNOME/libchamplain/issues/14.