GNOME Bugzilla – Bug 794556
gdatetime test fails to link in meson build
Last modified: 2018-04-11 14:00:06 UTC
Link failure poits to calls to libintl functions from within glib. My understanding is that this is due to tests being linked to a static version of glib without actually accompanying it with the proper -lintl lib. This is a quick and dirty fix for it.
Created attachment 369947 [details] [review] Link gdatetime test to libintl
Review of attachment 369947 [details] [review]: If we're linking GLib against libintl then we need to add it to the GLib dependencies; this way, linking against GLib will link against libintl, and the tests will link correctly. ::: meson.build @@ +1666,3 @@ if libintl.found() glib_conf.set('INTLLIBS', '-lintl') + intl_deps = [libintl] This should really go into the dependencies for GLib, if we're linking against it.
*** Bug 794768 has been marked as a duplicate of this bug. ***
Created attachment 370257 [details] [review] Add libintl to glib dependency object This allows glib/tests/gdatetime to compile successfully, since it uses libintl for setlocale() and bind_textdomain_codeset() (for OSes where libintl is not part of C runtime). My original report was a bit impresice: glib/tests/gdatetime.c actually uses libintl by itself (i thought that the error messages were referring to glib/gdatetime.c). So yes, it needs the same libintl that glib itself uses.
Actually I think the first patch was better. libintl is an internal dependency of glib, otherwise it would have been added into the pc file in Libs: instead of Libs.private. Some apps needs to use libintl directly and should link to it themself instead of relying on glib to pull it. There are many places in meson.build where we do: dependencies : [libintl, libglib_dep]
Review of attachment 369947 [details] [review]: There is no static version of glib because Meson does not even support to build both shared and static yet. Glib link to libintl because it uses it internally, but apps that use glib but does not use symbols from libintl directly doesn't need to link to libintl. Here gdatetime test actually seems to use libintl directly and thus needs to link to it explicitly. ::: glib/tests/meson.build @@ +113,3 @@ endif + if test_name == 'gdatetime' + deps += intl_deps deps += libintl ::: meson.build @@ +1666,3 @@ if libintl.found() glib_conf.set('INTLLIBS', '-lintl') + intl_deps = [libintl] You don't need intl_deps, you can directly pass libintl object to 'dependencies' keyword. It's already done in many places
Review of attachment 370257 [details] [review]: Does the dependency list of libglib_dep then become equal to the dependency list of libglib? Or is https://github.com/mesonbuild/meson/issues/1426 causing other dependencies to be dropped? libintl is already listed in the dependencies for libglib (line 240). This makes me wonder if we need to set libglib_dep.dependencies to libglib.dependencies until https://github.com/mesonbuild/meson/issues/1426 is fixed. In any case, the comment above libglib_dep.dependencies needs to be updated for this patch.
Created attachment 370758 [details] [review] Link gdatetime test to libintl gdatetime testcase uses glib (which uses libintl), but *alsi* calls libintl functions on its own, as part of the testing process. Therefore it must be linked to libintl like any other program that uses it. This is a re-iteration of the first patch, attachment 369947 [details] [review], but it uses libintl variable as a dependency instead of adding a new variable. Also, commit message is changed to reflect the reality better.
Review of attachment 370758 [details] [review]: Looks good to me, but I'm not glib reviewer.
Review of attachment 370758 [details] [review]: Sure. But what about my questions in comment #7?
(In reply to Philip Withnall from comment #7) > Review of attachment 370257 [details] [review] [review]: > > Does the dependency list of libglib_dep then become equal to the dependency > list of libglib? glib is linked like this: > libglib = library('glib-2.0', > ... > link_with : [charset_lib, gnulib_lib], > dependencies : [pcre, thread_dep, libintl, librt] + libiconv + platform_deps, > ... libglib_dep is declared like this: > libglib_dep = declare_dependency( > link_with : libglib, > # thread_dep doesn't get pulled in from libglib atm, > # see https://github.com/mesonbuild/meson/issues/1426 > dependencies : [thread_dep], > ... I don't see how dependencies of these two could be equal, even with libintl added to the dependencies of libglib_dep. > Or is https://github.com/mesonbuild/meson/issues/1426 > causing other dependencies to be dropped? I don't know. I didn't submit that issue, nor did i experience the errors that this issue caused before meson.build was fixed to work around it. > libintl is already listed in the dependencies for libglib (line 240). This > makes me wonder if we need to set libglib_dep.dependencies to > libglib.dependencies until https://github.com/mesonbuild/meson/issues/1426 > is fixed. Semantically declare_dependency() is an equivalent of dependency(). The "dependencies:" parameter is, therefore, an equivalent of the...um...no idea. If we were building statically, that would be Requires.private from pkg-config. But for dynamic builds...um...Requires (the non-private kind)? I'm not sure. > > In any case, the comment above libglib_dep.dependencies needs to be updated > for this patch. Since i'm not touching that anymore, the update of that comment is going to be someone else's problem.
Attachment 370758 [details] pushed as 24e80aa - Link gdatetime test to libintl