GNOME Bugzilla – Bug 753838
Build system clean-ups
Last modified: 2015-09-08 16:09:40 UTC
Philip pointed out a few build system related clean-ups in https://bugzilla.gnome.org/show_bug.cgi?id=739008#c19 Let's use this bug instead of cluttering bug 739008 .
Created attachment 309622 [details] [review] build: Remove unnecessary variable initializations
Created attachment 309623 [details] [review] build: Sprinkle quotations This is a WIP. I barely know M4 so please let me know if I am on the right track.
Ccing Philip because he is the Autotools/M4 expert.
Review of attachment 309622 [details] [review]: Looks good.
Review of attachment 309623 [details] [review]: ::: configure.ac @@ +149,3 @@ dnl *** Check if we should build with http backend *** dnl ************************************************** +AC_ARG_ENABLE([http], AS_HELP_STRING([--disable-http],[build without http/dav backend])) You can have [] around AS_HELP_STRING(…) too. @@ +177,3 @@ + AC_DEFINE([HAVE_AVAHI], [], [Set if we can use avahi])] + [msg_avahi=yes], + [AM_CONDITIONAL([HAVE_AVAHI], [false])]) Might want to reindent this with spaces to make it a bit clearer that, for example, the first AM_CONDITIONAL and AC_DEFINE are in the same set of quotation marks. But if that goes against the coding style, no worries. @@ +194,3 @@ dnl *** Check for libudev *** dnl ************************* +AC_ARG_ENABLE([udev], AS_HELP_STRING([--disable-udev],[build without libudev])) AS_HELP_STRING can be quoted here too. @@ +202,3 @@ if test "x$msg_udev" = "xyes"; then + PKG_CHECK_MODULES([UDEV], [libudev]) + AC_DEFINE([HAVE_LIBUDEV], 1, [Define to 1 if libudev availible]) s/availible/available/ @@ +385,3 @@ dnl *** Check if we should build with GOA volume monitor *** dnl ******************************************************** +AC_ARG_ENABLE([goa], AS_HELP_STRING([--disable-goa],[build without GOA volume monitor])) AS_HELP_STRING can be quoted here. @@ +405,3 @@ dnl *** Check for gphoto2 *** dnl ************************* +AC_ARG_ENABLE([gphoto2], AS_HELP_STRING([--disable-gphoto2],[build without gphoto2 support])) AS_HELP_STRING. @@ +429,3 @@ + AC_DEFINE([HAVE_GPHOTO2], 1, [Define to 1 if gphoto2 is available]) + PKG_CHECK_MODULES([GPHOTO25], [libgphoto2 >= 2.5.0], + AC_DEFINE([HAVE_GPHOTO25], 1, [Define to 1 if libgphoto2 2.5 is available]), The AC_DEFINE inside PKG_CHECK_MODULES should be quoted; and if you’re clarifying indentation, here would be a good place to do so.
Note: I wouldn’t spend too much time on this, unless you’re planning on adding new features to the build system (like stuff from https://wiki.gnome.org/Projects/GnomeCommon/Migration). Build system stuff is uninteresting and a time sink. :-)
Comment on attachment 309622 [details] [review] build: Remove unnecessary variable initializations Pushed to master.
(In reply to Philip Withnall from comment #6) > Note: I wouldn’t spend too much time on this, unless you’re planning on > adding new features to the build system (like stuff from > https://wiki.gnome.org/Projects/GnomeCommon/Migration). No, I am not migrating gvfs away from gnome-common. I just wanted to address some of the issues that you had raised in bug 739008 I couldn't bring myself to completely ignore those. :) > Build system stuff > is uninteresting and a time sink. :-) Indeed.
Created attachment 309802 [details] [review] build: Sprinkle quotations Quoted a few more things as suggested by pwithnall.
(In reply to Philip Withnall from comment #5) > @@ +202,3 @@ > if test "x$msg_udev" = "xyes"; then > + PKG_CHECK_MODULES([UDEV], [libudev]) > + AC_DEFINE([HAVE_LIBUDEV], 1, [Define to 1 if libudev availible]) > > s/availible/available/ I will fix that in a subsequent commit.
Review of attachment 309802 [details] [review]: Looks good to me.
Created attachment 310911 [details] [review] build: Fix a spelling mistake
Review of attachment 310911 [details] [review]: ::: configure.ac @@ +202,3 @@ if test "x$msg_udev" = "xyes"; then PKG_CHECK_MODULES([UDEV], [libudev]) + AC_DEFINE([HAVE_LIBUDEV], 1, [Define to 1 if libudev available]) "Define to 1 if libudev is available"?
Created attachment 310915 [details] [review] build: Fix wrong grammar and spelling
(In reply to Ondrej Holy from comment #13) > Review of attachment 310911 [details] [review] [review]: > > ::: configure.ac > @@ +202,3 @@ > if test "x$msg_udev" = "xyes"; then > PKG_CHECK_MODULES([UDEV], [libudev]) > + AC_DEFINE([HAVE_LIBUDEV], 1, [Define to 1 if libudev available]) > > "Define to 1 if libudev is available"? Of course. Fixed.
Review of attachment 310915 [details] [review]: Thanks!