[gnuastro-commits] master 219ed0d: Shell to call Libtool within BuildProgram set at configure time

[Mohammad Akhlaghi]

branch: master
commit 219ed0d8c93ab6a13cf4e198bbf4d6da0e5f30f0
Author: Mohammad Akhlaghi <address@hidden>
Commit: Mohammad Akhlaghi <address@hidden>

    Shell to call Libtool within BuildProgram set at configure time
    
    Some operating systems (Ubuntu in this case) set the default shell to dash,
    it is a minimalist POSIX compliant shell. Therefore it doesn't have many
    features that are available in Bash (including the `+=' operator). On the
    other hand, older versions of Libtool (2.4.2 in this case) don't check for
    the availability of the `+=' operator by the shell before using it,
    therefore BuildProgram crashes in such cases.
    
    Newer versions of libtool do address this problem (checked with version
    2.4.6), but we don't want to force the user to modify their installation as
    much as possible: only if there is no other way.
    
    To fix this, BuildProgram now doesn't call Libtool directly. It first calls
    the shell to run Libtool (specified with the `GAL_CONFIG_GNULIBTOOL_SHELL'
    macro which is set at configure time), and then runs Libtool with the
    appropriate shell.
    
    The `GAL_CONFIG_GNULIBTOOL_SHELL' macro is found during configure time. It
    first checks `sh', then `bash', then `zsh' (other shells can be added
    later) and sets the first one where Libtool operates without any error.
    
    This bug was reported by Aaron Watkins.
    
    This fixes bug #54430.
---
 NEWS                         |   2 +-
 bin/buildprog/buildprog.c    |   7 +--
 bootstrap.conf               |   1 +
 configure.ac                 | 110 ++++++++++++++++++++++++++++++-------------
 developer-build              |   2 +-
 doc/announce-acknowledge.txt |   1 +
 tests/Makefile.am            |   4 --
 tests/buildprog/simpleio.sh  |  10 +---
 8 files changed, 85 insertions(+), 52 deletions(-)

diff --git a/NEWS b/NEWS
index 8f4f3c9..8525518 100644
--- a/NEWS
+++ b/NEWS
@@ -66,7 +66,7 @@ GNU Astronomy Utilities NEWS                          -*- 
outline -*-
   bug #54346: Non '-I' or non '-L' strings in CPPFLAGS or LDFLAGS cause crash.
   bug #54358: Arithmetic's where, not ignoring blank values in condition.
   bug #54406: Insufficient sanity checks in mode symmetricity calculation.
-
+  bug #54430: BuildProgram fails with older Libtool versions on dash.
 
 
 
diff --git a/bin/buildprog/buildprog.c b/bin/buildprog/buildprog.c
index 92f956d..298f86b 100644
--- a/bin/buildprog/buildprog.c
+++ b/bin/buildprog/buildprog.c
@@ -102,9 +102,10 @@ buildprog(struct buildprogparams *p)
     if( asprintf(&fullla, "%s/libgnuastro.la", LIBDIR)<0 )
       error(EXIT_FAILURE, 0, "%s: asprintf allocation", __func__);
 
-  /* Put the command to run into a string. */
-  if( asprintf(&command, "%s %s %s%s --mode=link gcc %s %s %s %s %s %s %s "
-               "-I%s %s -o %s",
+  /* Write the full Libtool command into a string (to run afterwards). */
+  if( asprintf(&command, "%s -c \"%s %s %s%s --mode=link gcc %s %s "
+               "%s %s %s %s %s -I%s %s -o %s\"",
+               GAL_CONFIG_GNULIBTOOL_SHELL,
                GAL_CONFIG_GNULIBTOOL_EXEC,
                p->cp.quiet ? "--quiet" : "",
                p->tag      ? "--tag="   : "",
diff --git a/bootstrap.conf b/bootstrap.conf
index dc8120b..9c6d47b 100644
--- a/bootstrap.conf
+++ b/bootstrap.conf
@@ -226,6 +226,7 @@ gnulib_modules="
     gpl-3.0
     mbstok_r
     inttypes
+    system-posix
     git-version-gen
 "
 
diff --git a/configure.ac b/configure.ac
index 95f6d0e..bd3264f 100644
--- a/configure.ac
+++ b/configure.ac
@@ -278,6 +278,9 @@ AC_SEARCH_LIBS([wcspih], [wcs], [],
     [AC_MSG_ERROR([WCSLIB not found, cannot continue.])])
 
 
+
+
+
 # These are secondary tests for more fine-grained control in libraries that
 # have already been checked. We don't need to add them to the LIBS
 # variable, so we are using AC_CHECK_LIB for these tests.
@@ -311,27 +314,6 @@ AC_SUBST(HAVE_PTHREAD_BARRIER, [$has_pthread_barrier])
 
 
 
-
-# Gnulib checks:
-gl_INIT
-
-
-
-
-
-# Gnulib checks for the proper name for the C99 equivalent `restrict'
-# keyword and puts it in the `ac_cv_c_restrict' variable. If none exists,
-# it will put a `no' inside of this variable. As described in the output
-# `bootstrapped/m4/gnulib-common.m4', this is only necessary until Autoconf
-# 2.70 is released. Afterwards, we can use AC_C_RESTRICT.
-AS_IF([test "x$ac_cv_c_restrict" = "xno"],
-      [gal_restrict_replace=], [gal_restrict_replace=$ac_cv_c_restrict])
-AC_SUBST(RESTRICT_REPLACEMENT, [$gal_restrict_replace])
-
-
-
-
-
 # To add the help2man:
 AC_CHECK_PROG(has_help2man, help2man, [yes], [no])
 AM_CONDITIONAL([COND_HASHELP2MAN], [test "x$has_help2man" = "xyes"])
@@ -379,8 +361,11 @@ AC_SUBST(HAVE_LIBGIT2, [$has_libgit2])
 # Check the libtool executable on the system. Note that Gnuastro also ships
 # with a version of Libtool. We don't want Gnuastro's Libtool, here we want
 # to see if the system has libtool independent of Gnuastro so BuildProgram
-# can use it later.
+# can use it later. We also need to check some shells to run libtool within
+# them.
 AC_CHECK_PROG(has_libtool, libtool, [yes], [no])
+AC_CHECK_PROG(has_bash,    bash,    [yes], [no])
+AC_CHECK_PROG(has_zsh,     zsh,     [yes], [no])
 
 # If Libtool is present, make sure it is GNU Libtool
 AS_IF([test "x$has_libtool" = "xyes"],
@@ -399,10 +384,46 @@ AS_IF([test "x$has_gnulibtool" = "xyes"],
                [has_gnulibtool=yes; gnulibtool_exec=glibtool],
                [has_gnulibtool=no; anywarnings=yes] ) ])
 
-# Write the test results in the Makefiles and `config.h'.
-AM_CONDITIONAL([COND_HASGNULIBTOOL], [test "x$has_gnulibtool" = "xyes"])
-AC_DEFINE_UNQUOTED([GAL_CONFIG_GNULIBTOOL_EXEC], ["$gnulibtool_exec"],
-                   [The executable to call GNU Libtool])
+# Older versions of GNU Libtool have problems with the `dash' shell (a
+# minimalist shell) and will crash (due to not having the `+=' operator),
+# see bug #54430. So we need to check if the system's libtool and shell
+# (called by `sh' as in C's `system' function within BuildProgram) can work
+# with each other. If not, we need to search for Bash and tell BuildProgram
+# to use Bash instead of the default. But older versions of Bash also don't
+# support this operator, so we'll have to check with Bash is well.
+libtool_shell="none"
+AS_IF([test "x$has_gnulibtool" = "xyes"],
+      [
+        AC_MSG_CHECKING(for shell to use with libtool)
+        cprog=libtool_shell_test.c
+        echo "#include <stdio.h>"                                 > $cprog
+        echo "int main(void){printf(\"success\\n\"); return 0;}" >> $cprog
+        ltargs="--quiet --tag=CC --mode=link $CC $cprog -O3 -o test"
+        AS_IF( sh -c "$gnulibtool_exec $ltargs" > /dev/null 2>&1,
+               [libtool_shell="sh"],
+               [AS_IF([test "x$has_bash" = "xyes"],
+                      [AS_IF(bash -c "$gnulibtool_exec $ltargs" > /dev/null 
2>&1,
+                             [libtool_shell="bash"]) ],
+                      [AS_IF([test "x$has_zsh" = "xyes"],
+                             [AS_IF(zsh -c "$gnulibtool_exec $ltargs" > 
/dev/null 2>&1,
+                                    [libtool_shell="zsh"]) ])
+                       ] )
+               ] )
+        rm $cprog
+        AC_MSG_RESULT($libtool_shell)
+      ])
+
+# If a good shell to call Libtool could be found, then Libtool is usable
+# within a program (BuildProgram in this case).
+AS_IF([test "x$libtool_shell" = "xnone"],
+      [usable_libtool=no; anywarnings=yes],
+      [
+        usable_libtool=yes
+        AC_DEFINE_UNQUOTED([GAL_CONFIG_GNULIBTOOL_SHELL], ["$libtool_shell"],
+                           [Shell program to use with GNU Libtool])
+        AC_DEFINE_UNQUOTED([GAL_CONFIG_GNULIBTOOL_EXEC], ["$gnulibtool_exec"],
+                           [The executable to call GNU Libtool])
+      ])
 
 
 
@@ -430,6 +451,12 @@ AM_CONDITIONAL([COND_HASGHOSTSCRIPT], [test 
"x$has_ghostscript" = "xyes"])
 
 
 
+# Gnulib checks:
+gl_INIT
+
+
+
+
 
 # Check if Gnulib tests should be done:
 AC_ARG_ENABLE([gnulibcheck],
@@ -442,6 +469,19 @@ AM_CONDITIONAL([COND_GNULIBCHECK], [test 
$enable_gnulibcheck = yes])
 
 
 
+# Gnulib checks for the proper name for the C99 equivalent `restrict'
+# keyword and puts it in the `ac_cv_c_restrict' variable. If none exists,
+# it will put a `no' inside of this variable. As described in the output
+# `bootstrapped/m4/gnulib-common.m4', this is only necessary until Autoconf
+# 2.70 is released. Afterwards, we can use AC_C_RESTRICT.
+AS_IF([test "x$ac_cv_c_restrict" = "xno"],
+      [gal_restrict_replace=], [gal_restrict_replace=$ac_cv_c_restrict])
+AC_SUBST(RESTRICT_REPLACEMENT, [$gal_restrict_replace])
+
+
+
+
+
 # Set the one general parameters:
 AC_DEFINE_UNQUOTED([CONF_POSTFIX], [".conf"], [Configuration file post fix.])
 AC_DEFINE_UNQUOTED([USERCONFIG_DIR], [".local/etc"], [User data dir.])
@@ -616,7 +656,7 @@ AS_IF([test $ayes = true ],
 
 # BuildProgram depends on the presence of GNU Libtool, if it isn't present,
 # then don't build it.
-AS_IF([test "x$has_gnulibtool" = "xno"], [enable_buildprog=no])
+AS_IF([test "x$usable_libtool" = "xno"], [enable_buildprog=no])
 
 
 
@@ -749,8 +789,8 @@ AS_IF([test x$enable_guide_message = xyes],
                AS_ECHO(["    version controlled directory to help in 
reproducibility."])
                AS_ECHO([]) ])
 
-        AS_IF([test "x$has_gnulibtool" = "xno"],
-              [AS_ECHO(["  - GNU Libtool, could not be found in your search 
path."])
+        AS_IF([test "x$usable_libtool" = "xno"],
+              [AS_ECHO(["  - GNU Libtool can't be used on this system (see 
below)."])
                AS_ECHO(["    Gnuastro's BuildProgram uses GNU libtool to link 
your source code"])
                AS_ECHO(["    with the various libraries (Gnuastro's 
dependencies). Therefore"])
                AS_ECHO(["    BuildProgram will not be built or installed. 
Please note that"])
@@ -759,12 +799,14 @@ AS_IF([test x$enable_guide_message = xyes],
                AS_ECHO(["    internal implementation of GNU Libtool to build 
its self. This"])
                AS_ECHO(["    warning is only to let you know that BuildProgram 
will not be"])
                AS_ECHO(["    part of this build. The executable names searched 
were \`libtool'"])
-               AS_ECHO(["    and \`glibtool'."])
+               AS_ECHO(["    and \`glibtool'. The shells searched were \`sh', 
\`bash' and \`zsh'."])
                AS_ECHO([])
-               AS_IF([test "x$has_libtool" = "xyes"],
-                     [AS_ECHO(["    -- A libtool implementation was found, but 
it isn't GNU."])
-                      AS_ECHO([])
-                     ])
+               AS_IF([test "x$has_gnulibtool" = "xyes"],
+                     [AS_ECHO(["    -- GNU Libtool is present, but couldn't be 
run in tested shells."])
+                      AS_ECHO([])],
+                     [AS_IF([test "x$has_libtool" = "xyes"],
+                            [AS_ECHO(["    -- A libtool implementation was 
found, but it isn't GNU."])
+                             AS_ECHO([]) ]) ])
               ])
 
         AS_IF([test "x$has_ghostscript" = "xno"],
diff --git a/developer-build b/developer-build
index e188c4d..f6769cc 100755
--- a/developer-build
+++ b/developer-build
@@ -1,4 +1,4 @@
-#! /bin/sh
+#! /bin/bash
 
 # This script will configure and build Gnuastro in parallel inside another
 # directory (to keep the source and build directories separate). By default
diff --git a/doc/announce-acknowledge.txt b/doc/announce-acknowledge.txt
index 49ab4ba..33c1470 100644
--- a/doc/announce-acknowledge.txt
+++ b/doc/announce-acknowledge.txt
@@ -11,3 +11,4 @@ Juan Molina Tobar
 Dmitrii Oparin
 Teymoor Saifollahi
 Jenny Sorce
+Aaron Watkins
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 42e913c..e3c7379 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -51,9 +51,6 @@ endif
 if COND_HASLIBJPEG
   MAYBE_HASLIBJPEG = "yes"
 endif
-if COND_HASGNULIBTOOL
-  MAYBE_HASGNULIBTOOL = "yes"
-endif
 if COND_HASCXX
   MAYBE_CXX_PROGS    = versioncxx
   MAYBE_CXX_TESTS    = lib/versioncxx.sh
@@ -202,7 +199,6 @@ export mkdir_p="$(MKDIR_P)";                             \
 export topsrc=$(top_srcdir);                             \
 export topbuild=$(top_builddir);                         \
 export haslibjpeg=$(MAYBE_HASLIBJPEG);                   \
-export hasgnulibtool=$(MAYBE_HASGNULIBTOOL);             \
 export hasghostscript=$(MAYBE_HASGHOSTSCRIPT);           \
 export check_with_program=$(MAYBE_CHECK_WITH_PROGRAM);
 
diff --git a/tests/buildprog/simpleio.sh b/tests/buildprog/simpleio.sh
index 69baaf8..1f3874c 100755
--- a/tests/buildprog/simpleio.sh
+++ b/tests/buildprog/simpleio.sh
@@ -41,18 +41,10 @@ source=$topsrc/tests/$prog/simpleio.c
 #
 #   - The input data was not made (for example the test that created the
 #     data file failed).
-#
-#   - Gnuastro ships with its own version of Libtool for the building of
-#     the libraries and programs. But here, we want to test the user's
-#     libtool (and how it works with BuildProgram). So if libtool wasn't
-#     found at configure time, we need to skip this test.
 if [ ! -f $execname ]; then echo "$execname not created.";  exit 77; fi
 if [ ! -f $img      ]; then echo "$img does not exist.";    exit 77; fi
 if [ ! -f $source   ]; then echo "$source does not exist."; exit 77; fi
-if [ "x$hasgnulibtool" != "xyes" ];then
-    echo "GNU libtool not present.";
-    exit 77;
-fi
+