Re: [PATCH] build: avoid link failure on systems using gnulib's fcntl but not open

[Bruno Haible]

Hi,

Replying to Jim I wrote:
> >       "_rpl_open", referenced from:
> >           _grepfile in grep.o
> The situation in the 'grep' package is as follows:
>   - The main part, in lib/, has the module 'fcntl' and therefore
>     implicitly 'fcntl-h', but not the 'open' module.
>   - The tests part, in gnulib-tests/, has the module 'open', as a
>     dependency from 'dup2-tests'.
>   - The presence of the 'open' module is indicated through an
>     AC_SUBSTed variable GNULIB_OPEN. There is only one GNULIB_OPEN
>     for the entire package, and it is set to 1.
>   - Therefore the lib/fcntl.h replacement defines "#define open rpl_open".
> 
> I wouldn't say that it's a bug in gnulib. 
> It's a limitation of gnulib that you cannot decide to not use a module in
> the main part but use it in the tests. If you really wanted that, you
> would need a setup with two different configure.ac files, one for the
> main part and one for the tests.

But the user may be using open() in the main part, in lib/, without wanting
to use gnulib's 'open' module. We document for each module the list of
problems that it fixes. It's not because that dup2-tests requires an open()
function with a special workaround that the main part, in lib/, would need
it as well.

So I think it's a bug in gnulib.

The fix is to change the expansion of @GNULIB_OPEN@ in lib/fcntl.in.h
so that it expands to
  defined IN_GREP_GNULIB_TESTS
instead of to 1, and define IN_GREP_GNULIB_TESTS through
AM_CPPFLAGS in gnulib-tests/Makefile.am. This way, it will evaluate to
0 inside lib/ but to 1 when used from a file in gnulib-tests/.

The patch below fixes the issue with the @GNULIB_XYZ@ flags in lib/*.in.h.
The other uses of these indicator macros need to be looked at separately:
  clean-temp.c:#if GNULIB_FWRITEERROR
  clean-temp.c:#if GNULIB_CLOSE_STREAM
  clean-temp.c:#if GNULIB_FCNTL_SAFER
  clean-temp.c:#if GNULIB_FOPEN_SAFER
  clean-temp.c:#if GNULIB_FWRITEERROR
  clean-temp.c:#if GNULIB_CLOSE_STREAM
  error.c:# if GNULIB_FREOPEN_SAFER
  fchdir.c:# if GNULIB_CANONICALIZE || GNULIB_CANONICALIZE_LGPL
  fdopendir.c:# if GNULIB_DIRENT_SAFER
  fts.c:#if GNULIB_FTS
  fts.c:#if GNULIB_FTS
  fts.c:#if GNULIB_FTS
  mkstemp-safer.c:#if GNULIB_MKOSTEMP
  mkstemp-safer.c:#if GNULIB_MKOSTEMPS
  mkstemp-safer.c:#if GNULIB_MKSTEMPS
  realloc.c:#if GNULIB_MALLOC_GNU && !defined malloc
  realloc.c:#if GNULIB_REALLOC_GNU && !NEED_REALLOC_GNU && 
!SYSTEM_MALLOC_GLIBC_COMPATIBLE
  save-cwd.c:#if GNULIB_FCNTL_SAFER
  stdio-write.c:#if GNULIB_SIGPIPE
  write.c:#if GNULIB_SIGPIPE


2010-03-27  Bruno Haible  <address@hidden>

        Avoid side effects from tests-related modules on the compilation of lib.
        * m4/gnulib-common.m4 (gl_MODULE_INDICATOR_CONDITION): New macro.
        (gl_MODULE_INDICATOR_SET_VARIABLE): Use its expansion as a value.
        * gnulib-tool (func_emit_tests_Makefile_am): Accept a witness_macro
        parameter. Emit into AM_CPPFLAGS a definition of the designated C
        macro.
        (func_import): Define a witness macro. Assign it a value that depends
        on the current package. Override gl_MODULE_INDICATOR_CONDITION for the
        tests-related modules.
        (func_create_testdir): Update func_emit_tests_Makefile_am invocation.
        Reported by Jim Meyering.

--- m4/gnulib-common.m4.orig    Sat Mar 27 18:44:26 2010
+++ m4/gnulib-common.m4 Sat Mar 27 18:41:46 2010
@@ -1,4 +1,4 @@
-# gnulib-common.m4 serial 14
+# gnulib-common.m4 serial 15
 dnl Copyright (C) 2007-2010 Free Software Foundation, Inc.
 dnl This file is free software; the Free Software Foundation
 dnl gives unlimited permission to copy and/or distribute it,
@@ -37,12 +37,18 @@
 ])
 ])
 
+# gl_MODULE_INDICATOR_CONDITION
+# expands to a C preprocessor expression that evaluates to 1 or 0, depending
+# whether a gnulib module that has been requested shall be considered present
+# or not.
+AC_DEFUN([gl_MODULE_INDICATOR_CONDITION], [1])
+
 # gl_MODULE_INDICATOR_SET_VARIABLE([modulename])
 # sets the shell variable that indicates the presence of the given module to
 # a C preprocessor expression that will evaluate to 1.
 AC_DEFUN([gl_MODULE_INDICATOR_SET_VARIABLE],
 [
-  
GNULIB_[]m4_translit([$1],[abcdefghijklmnopqrstuvwxyz./-],[ABCDEFGHIJKLMNOPQRSTUVWXYZ___])=1
+  
GNULIB_[]m4_translit([$1],[abcdefghijklmnopqrstuvwxyz./-],[ABCDEFGHIJKLMNOPQRSTUVWXYZ___])=gl_MODULE_INDICATOR_CONDITION
 ])
 
 # gl_MODULE_INDICATOR([modulename])
--- gnulib-tool.orig    Sat Mar 27 18:44:26 2010
+++ gnulib-tool Sat Mar 27 18:44:16 2010
@@ -2761,7 +2761,7 @@
   echo "$files" | sed -n -e "s,^lib/,$sourcebase/,p"
 }
 
-# func_emit_tests_Makefile_am
+# func_emit_tests_Makefile_am witness_macro
 # emits the contents of tests makefile to standard output.
 # Input:
 # - local_gnulib_dir  from --local-dir
@@ -2785,6 +2785,7 @@
 # - uses_subdirs    nonempty if object files in subdirs exist
 func_emit_tests_Makefile_am ()
 {
+  witness_macro="$1"
   if test "$libtool" = true; then
     libext=la
     sed_eliminate_LDFLAGS="$sed_noop"
@@ -2913,6 +2914,9 @@
   done
   echo
   echo "AM_CPPFLAGS = \\"
+  if test -n "${witness_macro}"; then
+    echo "  address@hidden@=1 \\"
+  fi
   echo "  -I. -I\$(srcdir) \\"
   echo "  -I${testsbase_inverse} -I\$(srcdir)/${testsbase_inverse} \\"
   echo "  -I${testsbase_inverse}/${sourcebase-lib} 
-I\$(srcdir)/${testsbase_inverse}/${sourcebase-lib}"
@@ -4140,6 +4144,13 @@
     echo "  gltests_ltlibdeps="
     func_emit_initmacro_start ${macro_prefix}tests
     echo "  gl_source_base='$testsbase'"
+    # Define a tests witness macro that depends on the package.
+    # PACKAGE is defined by AM_INIT_AUTOMAKE, PACKAGE_TARNAME is defined by 
AC_INIT.
+    # See <http://lists.gnu.org/archive/html/automake/2009-05/msg00145.html>.
+    echo "  ${macro_prefix}tests_WITNESS=IN_\`echo 
\"\${PACKAGE-\$PACKAGE_TARNAME}\" | LC_ALL=C tr 'a-z' 'A-Z' | LC_ALL=C sed -e 
's/[^A-Z0-9_]/_/g'\`_GNULIB_TESTS"
+    echo "  AC_SUBST([${macro_prefix}tests_WITNESS])"
+    echo "  gl_module_indicator_condition=\$${macro_prefix}tests_WITNESS"
+    echo "  m4_pushdef([gl_MODULE_INDICATOR_CONDITION], 
[\$gl_module_indicator_condition])"
     for module in $testsrelated_modules; do
       func_verify_module
       if test -n "$module"; then
@@ -4152,6 +4163,7 @@
                 -e 's/gl_ltlibdeps/gltests_ltlibdeps/g'
       fi
     done
+    echo "  m4_popdef([gl_MODULE_INDICATOR_CONDITION])"
     func_emit_initmacro_end ${macro_prefix}tests
     # _LIBDEPS and _LTLIBDEPS variables are not needed if this library is
     # created using libtool, because libtool already handles the dependencies.
@@ -4210,7 +4222,7 @@
     func_dest_tmpfilename $testsbase/$makefile_am
     destfile="$testsbase/$makefile_am"
     modules="$testsrelated_modules"
-    func_emit_tests_Makefile_am > "$tmpfile"
+    func_emit_tests_Makefile_am "${macro_prefix}tests_WITNESS" > "$tmpfile"
     if test -f "$destdir"/$testsbase/$makefile_am; then
       if cmp "$destdir"/$testsbase/$makefile_am "$tmpfile" > /dev/null; then
         rm -f "$tmpfile"
@@ -4620,7 +4632,7 @@
     # Create $testsbase/Makefile.am.
     use_libtests=false
     destfile="$testsbase/Makefile.am"
-    func_emit_tests_Makefile_am > "$testdir/$testsbase/Makefile.am"
+    func_emit_tests_Makefile_am "" > "$testdir/$testsbase/Makefile.am"
     any_uses_subdirs="$any_uses_subdirs$uses_subdirs"
     # Create $testsbase/configure.ac.
     (echo "# Process this file with autoconf to produce a configure script."