Re: even `if FALSE' *_SOURCES should be used for TAGS

[Stefano Lattarini]

On Tuesday 12 October 2010, Ralf Wildenhues wrote:
> Hello, and thanks for report and analysis!
> 
> * Stefano Lattarini wrote on Tue, Oct 12, 2010 at 05:51:58PM CEST:
> > On Monday 11 October 2010, Jan Kratochvil wrote:
> > > `make tags' will not index sources which are listed in *_SOURCES += ...
> > > in unsatisfied AM_CONDITIONAL blocks.  IMO they should be.
> 
> > I agree with you on this.  By the way, "make dist" already works OOTB
> > in this situation, so tags (and ctags, and ID, and cscope) should too.
> 
> > > Sure one can use some EXTRA_*_SOURCES out of the `if' block but this
> > > delocalizes+duplicates the information making error prone for future
> > > modifications.
> 
> > Right.
> >  
> > > OTOH AM_CONDITIONAL can be used for example if an optional sources part is
> > > / is not bundled, in such case the current automake behavior is better.
> 
> > Ralf, do you agree we should implement the behaviour requested by Jan?
> 
> Well, I see the following caveat:
> 
> 'make tags' shouldn't cause files to be built (or accessed) that 'make
> dist' wouldn't build; that probably means conditional nodist_ files
> shouldn't be tagged.
I disagree; non-distributed but unconditonally-defined sources should be
tagged IMHO (even if they are Makefile-generated), as should distributed
but conditionally-defined sources.  This is also more consistent with the
current automake documentation:
  ``If any C, C++ or Fortran 77 source code or headers are present, then
    tags and TAGS rules will be generated for the directory.  All files
    listed using the _SOURCES, _HEADERS, and _LISP primaries will be used
    to generate tags.  Note that generated source files that are not
    distributed must be declared in variables like nodist_noinst_HEADERS
    or nodist_prog_SOURCES or they will be ignored.''
Only non-distributed conditionally-defined sources should not be tagged
by default (but should be when listed in some EXTRA_*_SOURCES, or when
the condition they are defined into happens to be true).

The attached patch contains some of testcases showing the behaviour
I have in mind.  They are not perfect, but good enough ATM.

> OTOH, automake.in:handle_source_transform is the only place where
> @sources and @dist_sources get possibly different values, and I don't
> understand why that was done; IOW, why the implementation didn't just
> have $(SOURCES) = $(DIST_SOURCES)  (except for the possibility of the
> no-dist option, of course).
Maybe to take into account the possibility of having non-distributed,
Makefile-generated and unconditonally-defined sources -- for which tags
should be enabled, even if they are not distributed.  This is just a
guess of mine, though, as I've done no history digging.
> I haven't dug out why that was done.
Me neither.
> If there is no good reason, the fix would be a one-liner.

As an aside, I see that the testsuite coverage on tags functionality
is *extremely* lacking and inadequate, and I'd really like to extend
it; so I seize this opportunity to repropose my idea of a new
"tests-extend" topic branch dedicated to testsuite enhancement
and extension.

Regards,
   Stefano

From 6a87f2e645169e8486f8ba94e6c371ec93c5a667 Mon Sep 17 00:00:00 2001
From: Stefano Lattarini <address@hidden>
Date: Wed, 13 Oct 2010 15:46:11 +0200
Subject: [PATCH] New tests; add TODO comment to automake.in

---
 automake.in           |    4 ++
 tests/tagscond.test   |  117 +++++++++++++++++++++++++++++++++++++++++++++
 tests/tagsextra.test  |   70 +++++++++++++++++++++++++++
 tests/tagsnodist.test |  127 +++++++++++++++++++++++++++++++++++++++++++++++++
 4 files changed, 318 insertions(+), 0 deletions(-)
 create mode 100755 tests/tagscond.test
 create mode 100755 tests/tagsextra.test
 create mode 100755 tests/tagsnodist.test

diff --git a/automake.in b/automake.in
index 6b36ae2..9e5308f 100755
--- a/automake.in
+++ b/automake.in
@@ -2208,6 +2208,10 @@ sub handle_source_transform ($$$$%)
        $used_pfx{$xpfx} = 1
          unless $prefix =~ /EXTRA_/;
 
+       # FIXME: $varname should be added to @sources iff:
+        #   1. it is unconditionally defined, or
+        #   2. it is in EXTRA_DIST, or
+        #   3. it is distributed
        push @sources, "\$($varname)";
        push @dist_sources, shadow_unconditionally ($varname, $where)
          unless (option ('no-dist') || $prefix =~ /^nodist_/);
diff --git a/tests/tagscond.test b/tests/tagscond.test
new file mode 100755
index 0000000..1b3c772
--- /dev/null
+++ b/tests/tagscond.test
@@ -0,0 +1,117 @@
+#! /bin/sh
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that tags for conditionally defined (but distributed)
+# sources work.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AC_PROG_CC
+AM_PATH_LISPDIR
+AM_CONDITIONAL([COND], [test x"$cond" = x"yes"])
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo bar
+foo_SOURCES = main.c
+if COND
+  foo_SOURCES += foo1.c
+  bar_SOURCES = main.c bar.h
+  include_HEADERS = quux.h
+  lisp_LISP = joe.el
+else
+  foo_SOURCES += foo2.c
+  bar_SOURCES = main.c baz.h
+  include_HEADERS = zardoz.h
+  lisp_LISP = moe.el
+endif
+
+quux.h zardoz.h:
+       : > $@
+CLEANFILES = quux.h zardoz.h
+
+exp.list:
+       (case $(srcdir) in .) d=;; *) d=$(srcdir)/;; esac \
+         && echo $${d}bar.h \
+         && echo $${d}baz.h \
+          && echo $${d}foo1.c \
+          && echo $${d}foo2.c \
+          && echo $${d}main.c \
+         && echo $${d}quux.h \
+         && echo $${d}zardoz.h \
+         && echo $${d}joe.el \
+         && echo $${d}moe.el \
+        ) > $@
+MOSTLYCLEANFILES = exp.list
+
+.PHONY: test
+test: exp.list ID TAGS CTAGS
+       diff exp.list TAGS
+       diff exp.list tags
+       diff exp.list ID
+
+check-local: test
+END
+
+echo 'int main(void) { return 0; }' > main.c
+echo 'extern int i;' > foo1.c
+echo 'extern int j;' > foo2.c
+: > bar.h
+: > baz.h
+
+: > elisp-comp
+
+mkdir bin
+cat > bin/etags <<'END'
+#!/bin/sh
+for i in ${1+"$@"}; do echo "$i"; done | sort > TAGS
+END
+cat > bin/ctags <<'END'
+#!/bin/sh
+for i in ${1+"$@"}; do echo "$i"; done | sort > tags
+END
+cat > bin/mkid <<'END'
+#!/bin/sh
+case $1 in
+  -f*) out=`expr "x$1" : 'x-f\(.*\)'`; shift;;
+    *) out=ID;;
+esac
+for i in ${1+"$@"}; do echo "$i"; done | sort > "$out"
+END
+chmod a+x bin/etags bin/ctags bin/mkid
+PATH=`pwd`/bin$PATH_SEPARATOR$PATH
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+for cond in yes no; do
+  case $cond in
+    yes) anticond=no;;
+     no) anticond=yes;;
+  esac
+  ./configure cond="$cond" EMACS=no
+  $MAKE test
+  $MAKE distcheck DISTCHECK_CONFIGURE_FLAGS="cond='$cond' EMACS=no"
+  $MAKE distcheck DISTCHECK_CONFIGURE_FLAGS="cond='$anticond' EMACS=no"
+  $MAKE distclean
+done
+
+:
diff --git a/tests/tagsextra.test b/tests/tagsextra.test
new file mode 100755
index 0000000..2c502ea
--- /dev/null
+++ b/tests/tagsextra.test
@@ -0,0 +1,70 @@
+#! /bin/sh
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check interaction of EXTRA_SOURCES with tags.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AM_CONDITIONAL([COND], [false])
+AC_PROG_CC
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = main.c
+if COND
+nodist_foo_SOURCES = bad.c
+endif
+EXTRA_foo_SOURCES = bad.c
+bad.c:
+## this file should never be compiled, so make its content invalid C
+       echo '/* %{~GREP~ME~}% */ choke me' > $@
+END
+
+echo 'int main(void) { return 0; }' > main.c
+
+mkdir bin
+for p in etags ctags mkid; do
+  (echo '#!/bin/sh' && echo 'exit 0') > bin/$p
+  chmod a+x bin/$p
+done
+PATH=`pwd`/bin$PATH_SEPARATOR$PATH
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+
+for target in ID ctags CTAGS tags TAGS; do
+  rm -f bad.c
+  $MAKE $target
+  $FGREP '%{~GREP~ME~}%' bad.c
+done
+
+$MAKE
+./foo
+
+$MAKE distdir
+test ! -r $me-1.0/bar.c
+
+$MAKE distcheck
+
+:
diff --git a/tests/tagsnodist.test b/tests/tagsnodist.test
new file mode 100755
index 0000000..f07de26
--- /dev/null
+++ b/tests/tagsnodist.test
@@ -0,0 +1,127 @@
+#! /bin/sh
+# Copyright (C) 2010 Free Software Foundation, Inc.
+#
+# This program is free software; you can redistribute it and/or modify
+# it under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 2, or (at your option)
+# any later version.
+#
+# This program is distributed in the hope that it will be useful,
+# but WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with this program.  If not, see <http://www.gnu.org/licenses/>.
+
+# Check that "make TAGS" and similar do not try to create
+# non-distributed files used only in "false" conditionals.
+
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in <<'END'
+AM_CONDITIONAL([COND_OK], [:])
+AM_CONDITIONAL([COND_KO], [false])
+AC_PROG_CC
+AC_OUTPUT
+END
+
+cat > Makefile.am <<'END'
+bin_PROGRAMS = foo
+foo_SOURCES = main.c
+
+if COND_OK
+## conditionally defined, but distributed: it will be considered
+## for tags, even if it's not listed in EXTRA_SOURCES.
+foo_SOURCES += bar.c
+## non-distributed and conditionally defined: but it will be considered
+## for tags, since the conditional COND_OK heppens to be true.
+nodist_foo_SOURCES = baz.c
+else
+## non-distributed and conditionally defined: it won't be considered
+## for tags, since it's not listed in EXTRA_SOURCES and it's in the
+## `else' branch of the conditional COND_OK, which happens to be true.
+nodist_foo_SOURCES = quux.c
+endif
+
+if COND_KO
+## non-distributed and conditionally defined: won't be considered
+## for tags, since it's not listed in EXTRA_SOURCES and the conditional
+## COND_KO happensto be false.
+nodist_foo_SOURCES += zardoz.c
+endif
+
+bar.c:
+       echo 'int bar(void) { return 0; }' > $@
+baz.c:
+       echo 'int baz(void) { return 1; }' > $@
+CLEANFILES = bar.c baz.c
+
+quux.c zardoz.c:
+       @echo 'ERROR: $@ should never be crated' >&2; exit 1
+END
+
+cat > main.c <<'END'
+int main(void)
+{
+  int bar(void);
+  int baz(void);
+  return bar() * baz();
+}
+END
+
+mkdir bin
+for p in etags ctags mkid; do
+  (echo '#!/bin/sh' && echo 'exit 0') > bin/$p
+  chmod a+x bin/$p
+done
+PATH=`pwd`/bin$PATH_SEPARATOR$PATH
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+
+for target in ID ctags CTAGS tags TAGS; do
+  $MAKE $target
+  test -f bar.c
+  test -f baz.c
+  test ! -r quux.c
+  test ! -r zardoz.c
+  rm -f bar.c baz.c
+done
+
+$MAKE
+./foo
+
+$MAKE distcheck
+$MAKE clean
+rm -f ID *tags* *TAGS* # just to be sure
+
+# Try again, with no-dist option.
+
+echo 'AUTOMAKE_OPTIONS = no-dist' >> Makefile.am
+$AUTOMAKE Makefile
+
+./config.status Makefile
+
+test ! -r bar.c # sanity check
+test ! -r baz.c # likewise
+for target in ID ctags CTAGS tags TAGS; do
+  $MAKE $target
+  test -f bar.c
+  test -f baz.c
+  test ! -r quux.c
+  test ! -r zardoz.c
+  rm -f bar.c baz.c
+done
+
+$MAKE
+./foo
+
+$MAKE distdir && Exit 1 # sanity check
+
+:
-- 
1.7.1