Created attachment 371161 [details] [review]
Meson: Add missing link on libintl for some tests

Created attachment 371162 [details] [review]
Tests: gsubprocess: Do not use stdin/stdout variables

Review of attachment 371162 [details] [review]:

Yes, definitely.

Review of attachment 371161 [details] [review]:

Can you please clarify in the commit message how libintl is pulled into those tests? What function calls in what files are requiring it, and why isn’t that a problem on desktop Linux?

Comment on attachment 371162 [details] [review]
Tests: gsubprocess: Do not use stdin/stdout variables

Attachment 371162 [details] pushed as 5ccd944 - Tests: gsubprocess: Do not use stdin/stdout variables

Created attachment 371172 [details] [review]
Meson: Add missing link on libintl for some tests

This fix undefined symbol link error when building for non-glibc
platform. Applications must link on libintl, it is not a public
dependency of libglib.

Philip: This is the same as bug #794556, I don't know why we fixed only gdatatime in that bug, I think LRN stopped on the first link error and didn't see there were others.

(In reply to Philip Withnall from comment #4)
> Review of attachment 371161 [details] [review] [review]:
>
> and why isn’t that a problem on desktop Linux?

It is for _("foo"), it calls function from glibc but on non-glibc platforms it calls function from a separate library (libintl).

Created attachment 371286 [details] [review]
Fix build when pthread_getname_np is not available

On Android _setname_ is always available but _getname_ is available only
with API level >= 26.

Created attachment 371287 [details] [review]
Fix build when pthread_getname_np is not available

On Android _setname_ is always available but _getname_ is available only
with API level >= 26.

Review of attachment 371172 [details] [review]:

::: gio/tests/meson.build
@@ -264,3 @@
                    '-DTEST_LOCALE_PATH="@0@"'.format(test_mo_dir),
                  ],
-        dependencies : common_gio_tests_deps)

Right at the top of the file we already have:

if host_system == 'darwin'
common_gio_tests_deps += [libintl]
endif

Could you not extend that to cover Android too?

Review of attachment 371287 [details] [review]:

::: configure.ac
@@ +2232,3 @@
             [AC_MSG_RESULT(no)])
+        AC_CHECK_FUNC(pthread_getname_np,
+            [AC_DEFINE(HAVE_PTHREAD_GETNAME_NP,1, [Have function pthread_getname_np])])

Do you not need to heed the comment at the top of this block?

        # This is not AC_CHECK_FUNC to also work with function
        # name mangling in header files.

::: meson.build
@@ +1489,3 @@
+  if cc.has_function('pthread_getname_np', prefix : pthread_prefix, dependencies : thread_dep)
+    glib_conf.set('HAVE_PTHREAD_GETNAME_NP', 1)
+  endif

The checks above use cc.has_header_symbol. Might we need to do that here, probably wrt the previous comment about name mangling?

Created attachment 371341 [details] [review]
Meson: Add missing link on libintl in tests

This fix undefined symbol link error when building for non-glibc
platform. Applications must link on libintl, it is not a public
dependency of libglib.

On glibc platforms libintl is a not found dependency and is just ignored
by meson, so it doesn't hurt to always have it.

Created attachment 371343 [details] [review]
Fix build when pthread_getname_np is not available

On Android _setname_ is always available but _getname_ is available only
with API level >= 26.

Created attachment 371345 [details] [review]
Fix build when pthread_getname_np is not available

On Android _setname_ is always available but _getname_ is available only
with API level >= 26.

(In reply to Philip Withnall from comment #12)
> Do you not need to heed the comment at the top of this block?
> 
>         # This is not AC_CHECK_FUNC to also work with function
>         # name mangling in header files.

I don't know why, but better copy what they do above when in doubt. Patch updated. I did the same change in meson too.

> ::: meson.build
> @@ +1489,3 @@
> +  if cc.has_function('pthread_getname_np', prefix : pthread_prefix,
> dependencies : thread_dep)
> +    glib_conf.set('HAVE_PTHREAD_GETNAME_NP', 1)
> +  endif
> 
> The checks above use cc.has_header_symbol. Might we need to do that here,
> probably wrt the previous comment about name mangling?

Indded, whatever that "name mangling" meant, meson port didn't care about it... Let's just imitate what meson already has, if it breaks on some system we probably need to replace all has_header_symbol() anyway.

I tested those patch both with autotools and meson, and both for my ubuntu and cross build for android (in cerbero).

Review of attachment 371341 [details] [review]:

Sure.

Review of attachment 371345 [details] [review]:

OK.

Attachment 371341 [details] pushed as d123717 - Meson: Add missing link on libintl in tests
Attachment 371345 [details] pushed as 01e8396 - Fix build when pthread_getname_np is not available

Pushed to both master and 2.56. Thanks.