GNOME Bugzilla – Bug 739395
Add an option to build without uhttpmock
Last modified: 2014-11-03 11:48:24 UTC
It turns out RHEL 7 doesn't have uhttpmock which makes it difficult to build libgdata there. Would be good to have an option to build libgdata without tests to be able to build it even on systems where uhttpmock isn't available.
Created attachment 289647 [details] [review] Add --disable-tests to skip the building of tests
Review of attachment 289647 [details] [review]: I am not a libgdata maintainer, but it looks good to me, Kalev, apart from a few minor points. ::: configure.ac @@ +151,3 @@ +AC_MSG_CHECKING(whether to build tests) +AC_ARG_ENABLE([tests], Nitpick. We should not quote 'tests' and put everything on the same line to match the rest of the file. @@ +153,3 @@ +AC_ARG_ENABLE([tests], + AS_HELP_STRING([--enable-tests], [Enable building of tests]), + [enable_tests=$enableval], [enable_tests=yes]) The [enable_tests=$enableval] is redundant because both variables will have the same value. See https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Package-Options.html : "The option's argument is available to the shell commands action-if-given in the shell variable enableval, which is actually just the value of the shell variable named enable_feature, with any non-alphanumeric characters in feature changed into ‘_’. You may use that variable instead, if you wish."
Review of attachment 289647 [details] [review]: Thanks! A few additional comments. ::: Makefile.am @@ +3,3 @@ +if ENABLE_TESTS +SUBDIRS += gdata/tests +endif You need to list all the directories in DIST_SUBDIRS too. ::: configure.ac @@ +151,3 @@ +AC_MSG_CHECKING(whether to build tests) +AC_ARG_ENABLE([tests], Actually, please keep the quoting. The quoting in the rest of the file is terrible and I’m slowly moving away from it. Please quote the AC_MSG_CHECKING() argument. @@ +160,3 @@ + dnl **************************** + dnl Check for uhttpmock + dnl **************************** Please either remove the ***** block, or keep it above the AC_MSG_CHECKING().
(In reply to comment #3) > Review of attachment 289647 [details] [review]: > > Thanks! A few additional comments. > > ::: Makefile.am > @@ +3,3 @@ > +if ENABLE_TESTS > +SUBDIRS += gdata/tests > +endif > > You need to list all the directories in DIST_SUBDIRS too. I am far from being an Autotools expert, but I think you can get away by not listing it. I did a ./autogen.sh --disable-tests && make && make dist, and tar -tf on the resulting tarball showed me the files in gdata/tests.
(In reply to comment #4) > I am far from being an Autotools expert, but I think you can get away by not > listing it. I did a ./autogen.sh --disable-tests && make && make dist, and tar > -tf on the resulting tarball showed me the files in gdata/tests. Hmm, weird. I guess I’m out of date with autotools then — I was going by the two methods in http://www.gnu.org/software/automake/manual/html_node/Conditional-Subdirectories.html, so I guess automake has got cleverer about detecting appends to SUBDIRS and including them in DIST_SUBDIRS.
Created attachment 289877 [details] [review] Add --disable-tests to skip the building of tests
Thanks for the comments! I've updated the patch, does it make sense to both of you now? :)
Review of attachment 289877 [details] [review]: Great, please commit to master.
Attachment 289877 [details] pushed as 730dfd4 - Add --disable-tests to skip the building of tests