Add curl/curl.h configure check and improve INSTALL

[jmarshall]

This addresses a configure issue and configure option documentation omissions from INSTALL uncovered by #1261:

• Unlike the checks for other libraries, the libcurl check validated only that the link library libcurl.so/.dylib/etc is available, but did not validate that the headers are available. In the libcurl enabled applied by default? #1261 case, inconsistent LDFLAGS and CPPFLAGS settings meant that the link library was available to the linker but the headers were not available to the compiler, which should have been detected at configure time.

  (It's not recorded in the commit messages, but I think the other AC_CHECK_HEADER/AC_CHECK_LIB checks may have been written thus precisely so that such environment variable problems could be detected.)

• The default behaviour of --enable-libcurl was not documented in INSTALL, and it was not clear that
  --enable/--disable and --with/--without options come in pairs.

[jmarshall]

(This is the first elif in configure.ac, but there are already lots in the autoconf-generated parts of configure so there are no portability issues in using it.)

[jmarshall]

Non-configure builds have required libcurl since 1.10 (d5a0887); as noted in #1177 (comment), the maintainers may consider that it's time to promote libcurl to being required at build-time by default in configure-based builds too.

This would need a one-word change in configure.ac and the --enable-libcurl documentation to be inverted, something like this:

diff --git a/configure.ac b/configure.ac
index c6b09db..4391e92 100644
--- a/configure.ac
+++ b/configure.ac
@@ -95,7 +95,7 @@ AC_SYS_LARGEFILE
 AC_ARG_ENABLE([libcurl],
   [AS_HELP_STRING([--enable-libcurl],
                   [enable libcurl-based support for http/https/etc URLs])],
-  [], [enable_libcurl=check])
+  [], [enable_libcurl=yes])
 diff --git a/INSTALL b/INSTALL
index e8d0bea..1d77f7c 100644
--- a/INSTALL
+++ b/INSTALL
@@ -148,12 +148,13 @@ various features and specify further optional external requirements:
---enable-libcurl
-    Use libcurl (<http://curl.se/>) to implement network access to
-    remote files via FTP, HTTP, HTTPS, etc.  By default or with
-    --enable-libcurl=check, configure will probe for libcurl and include
-    this functionality if libcurl is available.  Use --disable-libcurl
-    to prevent this.
+--enable-libcurl=check
+--disable-libcurl
+    By default or with --enable-libcurl, HTSlib includes code that uses
+    libcurl (<http://curl.se/>) to implement network access to remote files
+    via FTP, HTTP, HTTPS, etc.  Use --enable-libcurl=check to probe for
+    libcurl and include this functionality only if libcurl is available.
+    Use --disable-libcurl to omit this functionality.

[daviesrob]

This doesn't quite work where the header can't be found, as it still defines HAVE_LIBCURL. On a test machine where I removed curl.h I got:

checking for curl/curl.h... no
checking for curl_easy_pause in -lcurl... yes
configure: WARNING: libcurl not enabled: headers not found

[ ... ]

gcc -fvisibility=hidden  -o test/test_bgzf test/test_bgzf.o libhts.a -lz -lcurl -llzma -lbz2 -lz -lm   -lpthread
/usr/bin/ld: libhts.a(hfile.o): in function `load_hfile_plugins':
/home/debian/htslib/hfile.c:1084: undefined reference to `hfile_plugin_init_libcurl'
collect2: error: ld returned 1 exit status

[ ... ]

$ grep CURL config.h
#define HAVE_LIBCURL 1

It looks like this is because the default behaviour of AC_CHECK_LIB() isn't being overridden when looking for curl_easy_pause so it defines HAVE_LIBCURL when it finds the library. Luckily the fix is easy:

diff --git a/configure.ac b/configure.ac
index c6b09db..53d03c1 100644
--- a/configure.ac
+++ b/configure.ac
@@ -353,7 +353,7 @@ libcurl=disabled
 if test "$enable_libcurl" != no; then
   libcurl_devel=ok
   AC_CHECK_HEADER([curl/curl.h], [], [libcurl_devel="headers not found"], [;])
-  AC_CHECK_LIB([curl], [curl_easy_pause], [],
+  AC_CHECK_LIB([curl], [curl_easy_pause], [:],
     [AC_CHECK_LIB([curl], [curl_easy_init],
        [libcurl_devel="library is too old (7.18+ required)"],
        [libcurl_devel="library not found"])])

Could you update the PR with this change, please, if you agree?

[jmarshall]

D'oh, you're absolutely right, thanks.

I usually test these by changing the checks to AC_CHECK_HEADER([curl/curlbork.h], …) etc, which is easier than breaking my development machine. Unfortunately my testing focussed on enable/disable and not so much on check… thanks for catching this!

[daviesrob]

Thanks for the update.

I can recommend throw-away virtual machines as very useful for tests that involve messing around with system files.

[jmarshall]

Good point again — I have a stable of long-lived VMs running different platforms, but I'm mostly unpractised with (the UI for) making short-term throwaway clones of them for hacking on like this. Which, for developers, is probably half the point of using VMs…