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 739395 - Add an option to build without uhttpmock
Add an option to build without uhttpmock
Status: RESOLVED FIXED
Product: libgdata
Classification: Platform
Component: General
0.16.x
Other Linux
: Normal normal
: ---
Assigned To: libgdata-maint
libgdata-maint
Depends on:
Blocks:
 
 
Reported: 2014-10-30 12:17 UTC by Kalev Lember
Modified: 2014-11-03 11:48 UTC
See Also:
GNOME target: ---
GNOME version: ---


Attachments
Add --disable-tests to skip the building of tests (1.56 KB, patch)
2014-10-30 12:18 UTC, Kalev Lember
needs-work Details | Review
Add --disable-tests to skip the building of tests (1.41 KB, patch)
2014-11-03 09:31 UTC, Kalev Lember
committed Details | Review

Description Kalev Lember 2014-10-30 12:17:20 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.
Comment 1 Kalev Lember 2014-10-30 12:18:43 UTC
Created attachment 289647 [details] [review]
Add --disable-tests to skip the building of tests
Comment 2 Debarshi Ray 2014-10-30 12:50:17 UTC
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."
Comment 3 Philip Withnall 2014-10-30 17:47:43 UTC
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().
Comment 4 Debarshi Ray 2014-10-30 18:40:19 UTC
(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.
Comment 5 Philip Withnall 2014-10-30 23:41:36 UTC
(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.
Comment 6 Kalev Lember 2014-11-03 09:31:55 UTC
Created attachment 289877 [details] [review]
Add --disable-tests to skip the building of tests
Comment 7 Kalev Lember 2014-11-03 09:34:56 UTC
Thanks for the comments! I've updated the patch, does it make sense to both of you now? :)
Comment 8 Philip Withnall 2014-11-03 10:26:37 UTC
Review of attachment 289877 [details] [review]:

Great, please commit to master.
Comment 9 Kalev Lember 2014-11-03 11:48:18 UTC
Attachment 289877 [details] pushed as 730dfd4 - Add --disable-tests to skip the building of tests