GNOME Bugzilla – Bug 735974
build error: hard dependency on openat()
Last modified: 2014-09-11 15:30:58 UTC
gnome-desktop has a hard dependency on libgsystem through one call to the function gs_file_read_noatime. libgsystem has a hard dependency on openat() since commit d63409a3d4. Extract of log message: "Modern UNIX come with a variety of filesystem API suffixed with "at", like openat(), linkat(), etc. The reason for their existence is multiple." Unfortunately NetBSD 6 isn't modern enough... This makes it impossible to compile gnome-desktop on NetBSD 6.
Created attachment 285258 [details] [review] don't call gs_file_read_noatime() on systems lacking openat() Also, TESTS_ENVIRONMENT and EXTRA_DIST are set in glib-tap.mk, so use += in tests/Makefile.am.
The plan of record is bug 708453
"The plan" is slightly orthogonal: if I copy the relevant parts of libgsystem into gnome-desktop, it would still not be possible to build gnome-desktop on a system which doesn't provide openat(). Feel free to assign "The plan" bug to me, but in the meantime, I think this build fix should go in, especially given the imminent release. (BTW when testing, I grabbed a vanilla gnome-desktop-3.13.91, on a "modern" box which does have openat(), and it fails a distcheck. I think that "The plan" would probably fix that...)
ping? It would be nice for the upcoming gnome release to provide a gnome-desktop which compiles on systems which lack openat() [as per the attached patch].
Review of attachment 285258 [details] [review]: ::: configure.ac @@ +134,3 @@ +AC_CHECK_FUNCS([openat]) +AM_CONDITIONAL([USE_LIBGSYSTEM], [test $ac_cv_func_openat = yes]) Minor, but I tend to use "x" before variables to reduce the risk from typos. Or quotes. ::: docs/reference/gnome-desktop3/Makefile.am @@ +73,2 @@ # e.g. GTKDOC_LIBS=$(top_builddir)/gtk/$(gtktargetlib) +AM_CPPFLAGS=$(GNOME_DESKTOP_CFLAGS) -I$(top_srcdir)/libgnome-desktop -DGNOME_DESKTOP_USE_UNSTABLE_API Would prefer this type of thing as a separate patch too. ::: libgnome-desktop/Makefile.am @@ +18,3 @@ $(DISABLE_DEPRECATED_CFLAGS) +if USE_LIBGSYSTEM So...this "depend on libgsystem only on Linux" feels weird to me, but I can't say I'm opposed. Since you have a tested patch, that seems OK to me. @@ +153,3 @@ endif +EXTRA_DIST += \ Please don't fix multiple issues in one patch. I assume you're doing this because you saw a warning? It's probably correct, if so, please refactor into a separate "build: fix automake warnings" patch or so.
Thanks for the review - as the solution to problem involved autotools, I fixed those first before I could check that what I wrote was OK, hence the trivial extras. One comment: this isn't a "depend on libgsystem only on Linux" - it is a depend on libgsystem only on systems which provide openat(), such as NetBSD 7! At least, that is the intention... I'll just split the patch in two then...
Created attachment 285921 [details] [review] previous with autotools fix split out
Review of attachment 285921 [details] [review]: ::: libgnome-desktop/Makefile.am @@ +19,3 @@ +if USE_LIBGSYSTEM +AM_CPPFLAGS += -I$(srcdir)/libgsystem I don't think that hunk is needed, we'll survive just fine with including this directory even if we don't use libgsystem. ::: libgnome-desktop/gnome-desktop-thumbnail.c @@ +497,3 @@ g_warning ("Unable to create an input stream for %s: %s", uri, error->message); g_clear_error (&error); + g_object_unref (file); Whitespace change.
Fair enough about the AM_CPPFLAGS, I can remove it. Is "fixing" the white space not good? What would you like me to do?
(In reply to comment #9) > Fair enough about the AM_CPPFLAGS, I can remove it. > > Is "fixing" the white space not good? What would you like me to do? You can include it in a separate patch, but it's not relevant to the fix you're doing. Things would be different if we used Python ;) You can include the patch in this same bug though, no need to open a new one.
Created attachment 285927 [details] [review] previous without AM_CPPFLAGS hunk, and whitespace change split out
Created attachment 285928 [details] [review] whitespace part of previous
Review of attachment 285927 [details] [review]: Looks good.
Review of attachment 285928 [details] [review]: Fine.
Thank you! Committed as: https://git.gnome.org/browse/gnome-desktop/commit/?id=b9547595ac9fa2a08318589e060778de03897242 https://git.gnome.org/browse/gnome-desktop/commit/?id=f209a414dbec435e2c00d7f52424a3d1dafc4e4c