After an evaluation, GNOME has moved from Bugzilla to GitLab. Learn more about GitLab.
No new issues can be reported in GNOME Bugzilla anymore.
To report an issue in a GNOME project, go to GNOME GitLab.
Do not go to GNOME Gitlab for: Bluefish, Doxygen, GnuCash, GStreamer, java-gnome, LDTP, NetworkManager, Tomboy.
Bug 780309 - gio/tests/appinfo build fails: gdesktopappinfo.c skipped on OS X
gio/tests/appinfo build fails: gdesktopappinfo.c skipped on OS X
Status: RESOLVED FIXED
Product: glib
Classification: Platform
Component: gio
2.52.x
Other Mac OS
: Normal critical
: ---
Assigned To: gtkdev
gtkdev
: 781694 (view as bug list)
Depends on:
Blocks:
 
 
Reported: 2017-03-20 14:47 UTC by Daniel Macks
Modified: 2017-11-20 10:33 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
build: Don't build dbus-appinfo on OSX (904 bytes, patch)
2017-03-30 21:17 UTC, Patrick Griffis (tingping)
committed Details | Review
Avoid entanglement with modules not part of build world (1.18 KB, patch)
2017-05-04 05:44 UTC, Daniel Macks
none Details | Review
Set GIO_MODULE_DIR= in test environment (1.41 KB, patch)
2017-11-20 08:34 UTC, Daniel Macks
committed Details | Review

Description Daniel Macks 2017-03-20 14:47:24 UTC
Building glib-2.52.0 on OS X 10.11 was uneventful. Then during 'make check'...

/bin/sh ../../libtool  --tag=CC   --mode=link gcc -Wall -Wstrict-prototypes -Werror=declaration-after-statement -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=pointer-arith -Werror=init-self -Werror=format-security -Werror=format=2 -Werror=missing-include-dirs -Os  -L/sw/lib -Wl,-framework,Carbon -Wl,-framework,Foundation -o appinfo appinfo.o ../../gio/libgio-2.0.la ../../gobject/libgobject-2.0.la ../../gmodule/libgmodule-2.0.la ../../glib/libglib-2.0.la  -lintl  
libtool: link: gcc -Wall -Wstrict-prototypes -Werror=declaration-after-statement -Werror=missing-prototypes -Werror=implicit-function-declaration -Werror=pointer-arith -Werror=init-self -Werror=format-security -Werror=format=2 -Werror=missing-include-dirs -Os -Wl,-framework -Wl,Carbon -Wl,-framework -Wl,Foundation -o .libs/appinfo appinfo.o  -L/sw/lib ../../gio/.libs/libgio-2.0.dylib -L/sw/lib/libpcre1 /sw/build.build/glib2-shlibs-2.52.0-1/glib-2.52.0/gobject/.libs/libgobject-2.0.dylib /sw/build.build/glib2-shlibs-2.52.0-1/glib-2.52.0/gmodule/.libs/libgmodule-2.0.dylib -lz -lresolv ../../gobject/.libs/libgobject-2.0.dylib /sw/lib/libffi.dylib ../../gmodule/.libs/libgmodule-2.0.dylib /sw/build.build/glib2-shlibs-2.52.0-1/glib-2.52.0/glib/.libs/libglib-2.0.dylib ../../glib/.libs/libglib-2.0.dylib /sw/lib/libiconv.dylib /sw/lib/libpcre1/libpcre.dylib /sw/lib/libintl.dylib -pthread
clang: warning: argument unused during compilation: '-pthread'
Undefined symbols for architecture x86_64:
  "_g_desktop_app_info_get_categories", referenced from:
      _test_from_keyfile in appinfo.o
  "_g_desktop_app_info_get_filename", referenced from:
      _test_from_keyfile in appinfo.o
  "_g_desktop_app_info_get_generic_name", referenced from:
      _test_from_keyfile in appinfo.o
  "_g_desktop_app_info_get_keywords", referenced from:
      _test_from_keyfile in appinfo.o
  "_g_desktop_app_info_get_nodisplay", referenced from:
      _test_from_keyfile in appinfo.o
  "_g_desktop_app_info_get_startup_wm_class", referenced from:
      _test_startup_wm_class in appinfo.o
  "_g_desktop_app_info_new_from_filename", referenced from:
      _test_basic in appinfo.o
      _test_launch in appinfo.o
      _test_show_in in appinfo.o
      _test_tryexec in appinfo.o
      _test_startup_wm_class in appinfo.o
      _test_supported_types in appinfo.o
      _test_locale in appinfo.o
      ...
  "_g_desktop_app_info_new_from_keyfile", referenced from:
      _test_from_keyfile in appinfo.o
ld: symbol(s) not found for architecture x86_64

Those symbols are in glib/gdesktopappinfo.[ch], but glib/Makefile.am has those files controlled by "if !OS_COCOA". I see we do have gosxappinfo.[ch] on this platform, but it appears to supply different function-names.
Comment 1 Daniel Macks 2017-03-20 15:12:53 UTC
Same symptom in desktop-app-info.o
Comment 2 Daniel Macks 2017-03-22 02:47:21 UTC
Formerly, I thought the solution was simply to use the same "if !OS_COCOA" to omit these tests (skip tests that test code that is skipped). But now I see that the same problem (absence of g_desktop_app_info_* from libgio) causes gio/tests/async-splice-output-stream to fail:

/async-splice/copy-chunks-threaded-input: _1_: a
dyld: lazy symbol binding failed: Symbol not found: _g_desktop_app_info_lookup_get_type
  Referenced from: /sw/lib/gio/modules/libgiogconf.so
  Expected in: /sw/build.build/glib2-shlibs-2.52.0-1/glib-2.52.0/gio/.libs/libgio-2.0.0.dylib

dyld: Symbol not found: _g_desktop_app_info_lookup_get_type
  Referenced from: /sw/lib/gio/modules/libgiogconf.so
  Expected in: /sw/build.build/glib2-shlibs-2.52.0-1/glib-2.52.0/gio/.libs/libgio-2.0.0.dylib

Trace/BPT trap

I have /sw/lib/gio/modules/libgiogconf.so installed from gvfs, and that module is linked against the older version of libgio I have installed on my system. In the test, g_file_new_tmp() somehow triggers loading of that module from the live system, but the nature of the libtoolized test environment is that the local libs are inserted into the DYLD search path. So the module on the live system is loading the currently-being-tested local libgio instead of the one on the live system. That live one (older glib suite) *does* have these symbols. Bumping the "importance" because this is now apparently a backwards-incompatible/ABI break.
Comment 3 Matthias Neeracher 2017-03-24 06:43:57 UTC
It should be mentioned that this problem also breaks the build of glibmm 2.50.0
Comment 4 Matthias Clasen 2017-03-26 08:49:03 UTC
there is probably a bunch of "unix has desktop app info" assumptions all over the place, I'm afraid
Comment 5 Daniel Macks 2017-03-29 05:12:49 UTC
The origin seems to be Bug #734946, where a *partial* osx code was added but the *whole* previously-existing code was removed on this platform. Why wasn't this done piece-by-piece, or else with some build-time flag to control whether I want to break interface in a stable release until someone eventually writes the rest of the code? Is it that the pre-existing code is both hopeless and always-been-non-functional?
Comment 6 Matthias Clasen 2017-03-29 14:44:59 UTC
It wasn't don't piece-by-piece because there is no infrastructure to do 'a little bit of desktop file support with some native on top'.

Either you use GDesktopAppInfo, or you don't.
Comment 7 Daniel Macks 2017-03-29 15:58:37 UTC
Could we have a ./configure flag to control it?
Comment 8 Matthias Clasen 2017-03-30 14:24:35 UTC
We could, but I don't think we want to. Whats the motivation here ?
Comment 9 Daniel Macks 2017-03-30 14:32:14 UTC
Not breaking ABI. Above you can see reports that this is a visible problem for glibmm and gfvs, and even breaks glib self-testing itself (not just "we forgot to disable a test for an interface we disabled"--easy fix--but "breaking external things that use that interface breaks the test"--cannot be fixed within the test suite).
Comment 10 Patrick Griffis (tingping) 2017-03-30 14:40:30 UTC
This was fixed by commit 03c88daa733ee5efe5e78da382f3b6fc8735d44e
Comment 11 Patrick Griffis (tingping) 2017-03-30 14:54:32 UTC
As for an option, it might make sense the problem is just that OSX is sort of a grey area.

If you look at Windows you have two options being a Windows library or being a Cygwin library. In the latter it makes sense to respect the desktop spec because you are Unix.

The problem is OSX is at the same time the same as Unix but also used in a native context like bundles. So we can't go full Win32 and have a G_OS_OSX because half the Unix backends are the same. I really don't know the best solution to any of this; I think getting applications from the system is the best solution for users but obviously breaking ABI in such an unclear way sucks.
Comment 12 Patrick Griffis (tingping) 2017-03-30 21:17:01 UTC
Created attachment 349016 [details] [review]
build: Don't build dbus-appinfo on OSX

Skip another test that fails to build.
Comment 13 Daniel Macks 2017-04-03 07:28:57 UTC
I get a total of ~20 separate io/tests/ test-program failures analogous to Comment #2. Although the "cause" is broken ABI, the way that becomes visible is that giomodule is loading modules from the live system even though we're only in a testing environment, which is additionally circular against any older installed version of glib. Would a more proper test harness actually just have giomodule ignore the live system's modules altogether? One route would be to set the GIO_MODULE_DIR env var to a defined value that does not contain any modules, short-circuiting get_gio_module_dir()'s attempts to load whatever it finds.
Comment 14 Daniel Macks 2017-04-04 04:11:41 UTC
(In reply to Daniel Macks from comment #13)
> I get a total of ~20 separate io/tests/ test-program failures analogous to
> Comment #2. Although the "cause" is broken ABI, the way that becomes visible
> is that giomodule is loading modules from the live system even though we're
> only in a testing environment, which is additionally circular against any
> older installed version of glib. Would a more proper test harness actually
> just have giomodule ignore the live system's modules altogether? One route
> would be to set the GIO_MODULE_DIR env var to a defined value that does not
> contain any modules, short-circuiting get_gio_module_dir()'s attempts to
> load whatever it finds.

Indeed, 'make check TESTS_ENVIRONMENT=GIO_MODULE_DIR=' clears a ton of problems. I have no idea how a blank (but defined!) value there is actually evaluated (whethe there could be interference for out-of-tree building with other random things in the local dir, for example).
Comment 15 Daniel Macks 2017-04-23 20:47:45 UTC
Bug 781548 will allow me to avoid breaking ABI.(In reply to Matthias Clasen from comment #8)
> We could, but I don't think we want to. Whats the motivation here ?

Seems like Bug 781548 is it.
Comment 16 Philip Withnall 2017-04-25 09:22:46 UTC
*** Bug 781694 has been marked as a duplicate of this bug. ***
Comment 17 Philip Chimento 2017-04-26 04:14:06 UTC
My build failure was fixed by attachment 349016 [details] [review] and it's practically the same patch that I wrote on bug 781694, so this is a vote for committing it; I'm not a GLib reviewer though.

Can we backport it to the 2.52 branch too please?
Comment 18 Matthias Clasen 2017-04-27 13:51:43 UTC
Review of attachment 349016 [details] [review]:

sure
Comment 19 Matthias Clasen 2017-04-27 13:51:51 UTC
Review of attachment 349016 [details] [review]:

sure
Comment 20 Patrick Griffis (tingping) 2017-04-28 01:52:42 UTC
Comment on attachment 349016 [details] [review]
build: Don't build dbus-appinfo on OSX

Attachment 349016 [details] pushed as d4bfee1 - build: Don't build dbus-appinfo on OSX
Comment 21 Philip Chimento 2017-04-28 05:04:22 UTC
Thanks, I appreciate it!
Comment 22 Daniel Macks 2017-04-30 07:42:29 UTC
(In reply to Matthias Neeracher from comment #3)
> It should be mentioned that this problem also breaks the build of glibmm
> 2.50.0

Looks like that got filed at Bug #781947
Comment 23 Daniel Macks 2017-05-04 05:44:11 UTC
Created attachment 351022 [details] [review]
Avoid entanglement with modules not part of build world

Per Comment #14
Comment 24 Kjell Ahlstedt 2017-07-03 10:58:32 UTC
(In reply to Patrick Griffis (tingping) from comment #11)
> The problem is OSX is at the same time the same as Unix but also used in a
> native context like bundles. So we can't go full Win32 and have a G_OS_OSX
> because half the Unix backends are the same.

Why not? The tests would not be quite as clean as with G_OS_WIN32, more like
the tests with defined(HAVE_COCOA) in giomodule.c.
It would still be helpful to glibmm and other modules if HAVE_COCOA in config.h
could be replaced by a preprocessor macro in glibconfig.h, named G_OS_OSX or
G_OS_COCOA or something else. Then other modules could test for the existence
of this macro instead of repeating the configure test done by glib.
Comment 25 Patrick Griffis (tingping) 2017-07-03 18:35:07 UTC
(In reply to Kjell Ahlstedt from comment #24)
> (In reply to Patrick Griffis (tingping) from comment #11)
> > The problem is OSX is at the same time the same as Unix but also used in a
> > native context like bundles. So we can't go full Win32 and have a G_OS_OSX
> > because half the Unix backends are the same.
> 
> Why not? The tests would not be quite as clean as with G_OS_WIN32, more like
> the tests with defined(HAVE_COCOA) in giomodule.c.
> It would still be helpful to glibmm and other modules if HAVE_COCOA in
> config.h
> could be replaced by a preprocessor macro in glibconfig.h, named G_OS_OSX or
> G_OS_COCOA or something else. Then other modules could test for the existence
> of this macro instead of repeating the configure test done by glib.

Well, it wouldn't be an awful thing to do. As you say it is just a less clean case.

For example all usage of it would be like:

  #ifdef G_OS_UNIX
  #ifndef G_OS_OSX
  #include <gio-unix.h>
  #endif
  #endif

It just doesn't have the clean separation that WIN32 does. I do believe it would still have value and no real harm.
Comment 26 Philip Withnall 2017-10-05 11:49:02 UTC
Review of attachment 351022 [details] [review]:

This is a good catch. Setting GIO_MODULE_DIR to the empty string is the correct way to disable module loading from the system path.

Once this patch is updated and pushed, is there anything more to do in this bug?

::: gio/tests/Makefile.am
@@ +18,3 @@
 
+# avoid trying to load modules installed on build machine
+TESTS_ENVIRONMENT=GIO_MODULE_DIR=

I think this should be:
```
AM_TESTS_ENVIRONMENT += GIO_MODULE_DIR=
```

The AM_ prefix is needed because TESTS_ENVIRONMENT is reserved for the user to set, and we need to use += rather than = because glib-tap.mk already initialises AM_TESTS_ENVIRONMENT for us.

The corresponding change also needs to be made to the gio/tests/meson.build file.
Comment 27 Daniel Macks 2017-11-20 08:34:47 UTC
Created attachment 364029 [details] [review]
Set GIO_MODULE_DIR= in test environment

This is purely voodoo on the meson.build as I don't have a platform where I can test it.
Comment 28 Philip Withnall 2017-11-20 10:23:05 UTC
Review of attachment 364029 [details] [review]:

Looks like the right voodoo to me, thanks.