Re: [Patch] New target to generate cscope database

[Ralf Wildenhues]

<http://thread.gmane.org/gmane.comp.sysutils.automake.patches/3587/focus=3592>

Hello Debarshi,

my apologies for dropping the ball on this for so long.
To my only defense, your copyright assignment took a while
to get through.  :-/

* Debarshi Ray wrote on Fri, May 08, 2009 at 05:58:12AM CEST:
> Updated patch: 
> http://rishi.fedorapeople.org/gnu/0001-New-target-to-generate-cscope-database.patch
> and inlined below.

Thank you!

> Changes:

> + Added cscope to AM_RECURSIVE_TARGETS. Should it be cscope and/or
> cscopelist? (Ralf)

The rule is that all public targets which depend on recursive targets
need to be listed, plus all non-public targets which depend on recursive
targets and also get invoked explicitly by some internal make recursion
alongside other targets:
  $(MAKE) $(AM_MAKEFLAGS) target othertarget[...]

Thus, anything for which we have to tell GNU make to not update `target'
in parallel, because it doesn't get to see the complete dependency tree
by virtue of recursion in one of the prerequisites.

In this case, adding 'cscope' should be sufficient.

> TODO:
> + Test cases.

Ideally, the test cases should ensure that the new targets do what you
want, and also ensure that all those nits that you have fixed in the
code are really necessary.

I will add test cases in my add-on patch below.

> Issues:
> + I am not sure if cscope can work with $(LISP). Probably not.

Have you tried?  I haven't seen any problems with including $(LISP),
which means we probably should: cscope could gain support at some point
in the future.

> + Cscope looks for #included headers not found in the source tree
> under the standard include path (usually /usr/include). More paths can
> be added using the -I option as mentioned in the man page. On the
> other hand, this behaviour can be stopped using -k. What do we want to
> do?

In case of doubt, we can let the developer and/or the user decide, by
using {AM_,}CSCOPEFLAGS.  I'm not sure what the default should be,
though.  Also, while passing $(DEFAULT_INCLUDES) $(INCLUDES) seems like
not such a bad idea, $(INCLUDES) is deprecated in favor of
$(AM_CPPFLAGS), both that one and $(CPPFLAGS) could also contain -D
flags which will make cscope barf.  Hmm.

Maybe the best for now is to neither pass -I nor -k, more so since -k is
not revertible by a later command line option.

> --- a/automake.in
> +++ b/automake.in

> @@ -3769,7 +3781,8 @@ sub handle_tags
>                                        new Automake::Location,
>                                        CONFIG    => "@config",
>                                        TAGSDIRS  => "@tag_deps",
> -                                      CTAGSDIRS => "@ctag_deps");
> +                                      CTAGSDIRS => "@ctag_deps",
> +                                      CSCOPEDIRS => "@cscope_deps");
> 
>       set_seen 'TAGS_DEPENDENCIES';
>      }
> @@ -3784,8 +3797,9 @@ sub handle_tags
>       # Otherwise, it would be possible for a top-level "make TAGS"
>       # to fail because some subdirectory failed.
>       $output_rules .= "tags: TAGS\nTAGS:\n\n";
> -     # Ditto ctags.
> +     # Ditto ctags and cscope.
>       $output_rules .= "ctags: CTAGS\nCTAGS:\n\n";
> +     $output_rules .= "cscopelist:\n\n";

This isn't sufficient: "make cscope" in an otherwise empty project will
fail.  See cscope2.test below.

>      }
>  }


> --- a/lib/am/tags.am
> +++ b/lib/am/tags.am

> +if %?TOPDIR_P%
> +
> +CSCOPE = cscope
> +.PHONY: cscope cscopeclean
> +AM_RECURSIVE_TARGETS += cscope
> +
> +cscope: cscopeclean cscope.files
> +     $(CSCOPE) -b -q $(CSCOPEFLAGS) $(AM_CSCOPEFLAGS) \
> +       -i cscope.files $(CSCOPE_ARGS)

This produces a race condition with parallel make: it is not guaranteed
that the cscopeclean rule has completed before the cscope.files rule is
started.  Why not instead just add an ...

> +cscopeclean:
> +     -rm -f cscope.files
> +
> +cscope.files: %CSCOPEDIRS% cscopelist
> +
> +endif %?TOPDIR_P%
> +
> +.PHONY: cscopelist
> +cscopelist: %CSCOPEDIRS% $(HEADERS) $(SOURCES) $(LISP)

?TOPDIR_P?      : >$@

... here?

> +     list='$(SOURCES) $(HEADERS) $(LISP)'; \
> +     for i in $$list; do \
> +       if test -f "$$i"; \
> +         then echo \"$(subdir)/$$i\"; \
> +       else \
> +         echo \"$(abs_srcdir)/$$i\"; \

This has the disadvantage of hard-coding absolute file names into
cscope.files.  We can do a bit better by using either $(srcdir)
(if it is absolute) or $(subdir)/$(srcdir) (if srcdir is relative).
The cscope 15.6 that I have tested seems smart enough to canonicalize
file names containing '../' in its output.

This should allow for use in directories with spaces in the absolute
name, too.

> +       fi; \
> +     done >> $(top_builddir)/cscope.files

I have committed the patch below on top of yours, to fixup the above
issues, add a few tests and you to THANKS, and pushed the result to the
dr-cscope branch (probably to be deleted again once this feature is
merged).  Could you (or your colleagues, or anyone else) try it out and
report issues with it?

One thing I am still not really happy about is that this only creates
one cscope database at the toplevel build directory only.  Issuing
`make cscope' in subdirs will even error out.  The former is normally
desirable, I guess, but for very large packages it might not be the
optimal solution.  OTOH, for packages consisting of a lot of
sub-packages, it might even be desirable to have less than one cscope
database per package.  Not sure yet how to best improve that.

Thanks,
Ralf

    Fixups and tests for cscope functionality.
    
    It seems cscope is not able to take into account relative
    file names of included cscope.files files, but it is able to
    canonicalize file names containing '../' sequences.
    This patch makes the cscope references relative again, and
    fixes some corner cases.
    
    * .gitignore: Ignore files generated by `make cscope'.
    * NEWS: Reword a bit.
    * THANKS: Update.
    * automake.in (handle_tags): Use $(am__cd).  Provide default
    empty rule for the `cscope' target, for empty sources.
    * lib/am/tags.am (cscopelist): Construct relative path to files
    in $(srcdir) if $(srcdir) is relative.
    [TOPDIR_P] (cscope): Do not depend on cscope-clean.  Only invoke
    $(CSCOPE) if cscope.files is nonemtpy.
    (clean-cscope): Rename from ...
    (cscopeclean): ... this.
    (cscope.files): Depend on clean-cscope.
    (distclean-tags) [!TOPDIR_P]: No need to remove cscope files
    here.
    * tests/cscope.test, tests/cscope2.test, tests/cscope3.test: New
    tests.
    * tests/Makefile.am: Adjust.

diff --git a/NEWS b/NEWS
index 4c63fa9..ef608da 100644
--- a/NEWS
+++ b/NEWS
@@ -2,7 +2,7 @@ New in 1.11.0a:
 
 * New targets:
 
-  - New `cscope' target (builds cscope database).
+  - New `cscope' target to build a cscope database for the source tree.
 
 Bugs fixed in 1.11.0a:
 
diff --git a/automake.in b/automake.in
index b0a8951..84ed8bc 100755
--- a/automake.in
+++ b/automake.in
@@ -3753,7 +3753,7 @@ sub handle_tags
                          . "\tlist=\'\$(SUBDIRS)\'; for subdir in \$\$list; do 
\\\n"
                          # Never fail here if a subdir fails; it
                          # isn't important.
-                         . "\t  test \"\$\$subdir\" = . || (cd \$\$subdir"
+                         . "\t  test \"\$\$subdir\" = . || (\$(am__cd) 
\$\$subdir"
                          . " && \$(MAKE) \$(AM_MAKEFLAGS) cscopelist); \\\n"
                          . "\tdone\n");
        push (@cscope_deps, 'cscopelist-recursive');
@@ -3799,7 +3799,7 @@ sub handle_tags
        $output_rules .= "tags: TAGS\nTAGS:\n\n";
        # Ditto ctags and cscope.
        $output_rules .= "ctags: CTAGS\nCTAGS:\n\n";
-       $output_rules .= "cscopelist:\n\n";
+       $output_rules .= "cscope cscopelist:\n\n";
     }
 }
 
diff --git a/lib/am/tags.am b/lib/am/tags.am
index 142748b..7fa75c9 100644
--- a/lib/am/tags.am
+++ b/lib/am/tags.am
@@ -141,28 +141,32 @@ GTAGS:
 if %?TOPDIR_P%
 
 CSCOPE = cscope
-.PHONY: cscope cscopeclean
+.PHONY: cscope clean-cscope
 AM_RECURSIVE_TARGETS += cscope
 
-cscope: cscopeclean cscope.files
-       $(CSCOPE) -b -q $(CSCOPEFLAGS) $(AM_CSCOPEFLAGS) \
-         -i cscope.files $(CSCOPE_ARGS)
+cscope: cscope.files
+       test ! -s cscope.files \
+         || $(CSCOPE) -b -q $(AM_CSCOPEFLAGS) $(CSCOPEFLAGS) -i cscope.files 
$(CSCOPE_ARGS)
 
-cscopeclean:
+clean-cscope:
        -rm -f cscope.files
 
-cscope.files: %CSCOPEDIRS% cscopelist
+cscope.files: clean-cscope %CSCOPEDIRS% cscopelist
 
 endif %?TOPDIR_P%
 
 .PHONY: cscopelist
 cscopelist: %CSCOPEDIRS% $(HEADERS) $(SOURCES) $(LISP)
        list='$(SOURCES) $(HEADERS) $(LISP)'; \
+       case "$(srcdir)" in \
+         [\\/]* | ?:[\\/]*) sdir="$(srcdir)" ;; \
+         *) sdir=$(subdir)/$(srcdir) ;; \
+       esac; \
        for i in $$list; do \
-         if test -f "$$i"; \
-           then echo \"$(subdir)/$$i\"; \
+         if test -f "$$i"; then \
+           echo "$(subdir)/$$i"; \
          else \
-           echo \"$(abs_srcdir)/$$i\"; \
+           echo "$$sdir/$$i"; \
          fi; \
        done >> $(top_builddir)/cscope.files
 
@@ -174,5 +178,7 @@ cscopelist: %CSCOPEDIRS% $(HEADERS) $(SOURCES) $(LISP)
 .PHONY distclean-am: distclean-tags
 
 distclean-tags:
-       -rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags cscope.out cscope.in.out \
-       cscope.po.out cscope.files
+       -rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags
+if %?TOPDIR_P%
+       -rm -f cscope.out cscope.in.out cscope.po.out cscope.files
+endif %?TOPDIR_P%
diff --git a/tests/Makefile.am b/tests/Makefile.am
index 62529a6..e974689 100644
--- a/tests/Makefile.am
+++ b/tests/Makefile.am
@@ -208,6 +208,9 @@ confsub.test \
 confvar.test \
 confvar2.test \
 copy.test \
+cscope.test \
+cscope2.test \
+cscope3.test \
 ctarget1.test \
 cxx.test \
 cxx2.test \
diff --git a/tests/cscope.test b/tests/cscope.test
new file mode 100755
index 0000000..5beee5b
--- /dev/null
+++ b/tests/cscope.test
@@ -0,0 +1,108 @@
+#! /bin/sh
+# Copyright (C) 2009  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 3, 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/>.
+
+# Test cscope functionality.
+
+required=
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_CONFIG_FILES([sub/Makefile])
+AC_PROG_CC
+AC_PROG_CXX
+AC_PROG_FC
+AM_PROG_GCJ
+AM_PATH_LISPDIR
+AC_OUTPUT
+END
+
+mkdir sub sub/subsub
+
+cat >Makefile.am <<'END'
+SUBDIRS = sub
+bin_PROGRAMS = foo
+foo_SOURCES = foo.c bar.cpp baz.f90
+lisp_LISP = foo.el
+EXTRA_DIST = foo.el
+END
+
+cat >sub/Makefile.am <<'END'
+bin_PROGRAMS = bar
+bar_SOURCES = subsub/dist.c
+nodist_bar_SOURCES = subsub/gen.c
+subsub/gen.c:
+       $(MKDIR_P) subsub
+       echo 'int generated_subsub () { return 0; }' > $@
+CLEANFILES = subsub/gen.c
+END
+
+echo 'int foo_func () { return 0; }' > foo.c
+echo 'int main () { return 0; }' > bar.cpp
+cat > baz.f90 <<'END'
+      subroutine baz
+      end
+END
+: >foo.el
+echo 'int main () { return 0; }' > sub/subsub/dist.c
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE --add-missing
+
+mkdir build
+cd build
+../configure || Exit 77
+
+$MAKE -n cscope
+
+: ${CSCOPE=cscope}
+( $CSCOPE --version ) >/dev/null 2>&1 || Exit 77
+
+$MAKE cscope 2>stderr
+cat stderr >&2
+grep 'cannot find file' stderr && Exit 1
+
+test -f cscope.files
+$FGREP foo.c cscope.files
+$FGREP bar.cpp cscope.files
+$FGREP sub/subsub/dist.c cscope.files
+$FGREP sub/subsub/gen.c cscope.files
+
+cp cscope.files cscope.files1
+$MAKE cscope
+diff cscope.files cscope.files1
+
+$MAKE distclean
+test ! -f cscope.files
+test ! -f cscope.out
+test ! -f cscope.in.out
+test ! -f cscope.po.out
+
+
+`pwd`/../configure || Exit 77
+
+$MAKE cscope 2>stderr
+cat stderr >&2
+grep 'cannot find file' stderr && Exit 1
+$MAKE distclean
+
+cd ..
+./configure
+$MAKE distcheck
+
+:
diff --git a/tests/cscope2.test b/tests/cscope2.test
new file mode 100755
index 0000000..801457f
--- /dev/null
+++ b/tests/cscope2.test
@@ -0,0 +1,36 @@
+#! /bin/sh
+# Copyright (C) 2009  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 3, 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/>.
+
+# The `cscope' target should not fail if there are no sources.
+
+required=
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_OUTPUT
+END
+: >Makefile.am
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE cscope
+
+:
diff --git a/tests/cscope3.test b/tests/cscope3.test
new file mode 100755
index 0000000..04e85c3
--- /dev/null
+++ b/tests/cscope3.test
@@ -0,0 +1,41 @@
+#! /bin/sh
+# Copyright (C) 2009  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 3, 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/>.
+
+# The `cscope' target should not fail if there are no sources in a subdir.
+# In practice this means `cscope' should not be invoked if cscope.files
+# is empty.
+
+required=cscope
+. ./defs || Exit 1
+
+set -e
+
+cat >> configure.in << 'END'
+AC_CONFIG_FILES([sub/Makefile])
+AC_OUTPUT
+END
+mkdir sub
+echo 'SUBDIRS = sub' >Makefile.am
+: >sub/Makefile.am
+
+$ACLOCAL
+$AUTOCONF
+$AUTOMAKE
+
+./configure
+$MAKE cscope
+
+: