Re: [Patch] New target to generate cscope database

[Ralf Wildenhues]

Hello Debarshi,

thank you for your work on this!

* Debarshi Ray wrote on Wed, May 06, 2009 at 11:17:19AM CEST:
> I was trying to make Automake generate a rule for creating cscope
> tags:

> I found a discussion involving Jesse Barnes
> (http://sources.redhat.com/ml/automake/2004-07/msg00051.html) which
> seemed inconclusive. I have tried this patch with Automake 1.10.2 and
> the msmtp source base and it seems to work, but my knowledge of
> Automake's internals and Perl is zero.

Did you take any code from Jesse's patch?  In that case, the ChangeLog
entry should list Jesse as author as well, and we might even need
copyright papers from Jesse, too.

A nit-picky review follows; I will fix up all those bits that you won't,
but the more you can do, the faster the whole thing will be done.  :-)

First off, a few questions:

Does cscope offer enough additional functionality over, say,
Exuberant Ctags or gtags, to warrant being supported?  Do many people
use it?

What happens if sources other than C, Lex, or Yacc are listed in
cscope.files?

What if files are listed more than once in cscope.files?  If that is a
problem, you can reuse the awk construct that is used in the CTAGS and
other rules, but that will eliminate duplicates stemming from one
Makefile.am only.

The cscope.1 manpages I'm looking at allows to pass -Iincdir arguments,
which directs cscope to search for included headers in 'incdir'.  What
exactly is this used for, and would it still be beneficial to use even
if all headers in the package are listed in *_HEADERS or *_SOURCES
files?  Or would it even be necessary to pass suitable -I arguments in
order to prevent cscope from using older, installed or incompatible
versions of some header files?

The HACKING file from the git tree lists several bits missing from
this patch, namely tests ensuring that the new functionality works as
desired (also see tests/README), a NEWS file entry.

For example, presence of C, Lex, Yacc, and other sources should be
tested.

> From: Debarshi Ray <address@hidden>
> Date: Wed, 6 May 2009 14:37:39 +0530
> Subject: [PATCH] New target to generate cscope database.
> 
> * automake.in (handle_tags): Handle cscope.
> * doc/automake.texi (Tags): Document cscope.
> * lib/am/tags.am (CSCOPE): New macro.
> (cscope): New target.
> (cscope.file): Likewise.
> (cscopelist): Likewise.
> (distclean-tags): Remove `cscope.out', `cscope.in.out', `cscope.po.out'
> and `cscope.files'.

> * Makefile.in: Regenerate.

FYI, in Automake we typically don't list any regenerated files in the
ChangeLog entry.  They are only tracked in version control because that
allows to see easily how changes to the sources affect the generated
files.

> diff --git a/automake.in b/automake.in
> index 20ef3bd..b5133b1 100755
> --- a/automake.in
> +++ b/automake.in
> @@ -3724,6 +3724,7 @@ sub handle_tags
>  {
>      my @tag_deps = ();
>      my @ctag_deps = ();
> +    my @cscope_deps = ();
>      if (var ('SUBDIRS'))
>      {
>       $output_rules .= ("tags-recursive:\n"
> @@ -3747,6 +3748,17 @@ sub handle_tags
>       push (@ctag_deps, 'ctags-recursive');
>       &depend ('.PHONY', 'ctags-recursive');
>       &depend ('.MAKE', 'ctags-recursive');
> +
> +     $output_rules .= ("cscopelist-recursive:\n"
> +                       . "\tlist=\'\$(SUBDIRS)\'; for subdir in \$\$list; do 
> \\\n"
> +                       # Never fail here if a subdir fails; it
> +                       # isn't important.
> +                       . "\t  test \"\$\$subdir\" = . || (cd \$\$subdir"
> +                       . " && \$(MAKE) \$(AM_MAKEFLAGS) cscopelist); \\\n"
> +                       . "\tdone\n");
> +     push (@cscope_deps, 'cscopelist-recursive');
> +     &depend ('.PHONY', 'cscopelist-recursive');
> +     &depend ('.MAKE', 'cscopelist-recursive');
>      }

Side note: the {tags,ctags,cscopelist}-recursive rules should be
factorized so their text appears only once in the output.  That can be
done as a separate change, however.

>      if (&saw_sources_p (1)
> @@ -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";
>      }
>  }
> 

> diff --git a/doc/automake.texi b/doc/automake.texi
> index 09a5dd2..240fa84 100644
> --- a/doc/automake.texi
> +++ b/doc/automake.texi
> @@ -312,7 +312,7 @@ Support for Test Suites
> 
>  Miscellaneous Rules
> 
> -* Tags::                        Interfacing to etags and mkid
> +* Tags::                        Interfacing to cscope, etags and mkid
>  * Suffixes::                    Handling new file extensions
>  * Multilibs::                   Support for multilibs.
> 
> @@ -9302,7 +9302,7 @@ the @code{AM_INIT_AUTOMAKE} macro in 
> @file{configure.ac}.
>  There are a few rules and variables that didn't fit anywhere else.
> 
>  @menu
> -* Tags::                        Interfacing to etags and mkid
> +* Tags::                        Interfacing to cscope, etags and mkid
>  * Suffixes::                    Handling new file extensions
>  * Multilibs::                   Support for multilibs.
>  @end menu
> @@ -9364,6 +9364,13 @@ Automake will also generate an @code{ID} rule
> that will run
>  directory-by-directory basis.
>  @trindex id
> 
> +Similarly, the @code{cscope} rule will create a list of all the source
> +files in the tree and run @command{cscope} to build an inverted index
> +database. The variable @code{CSCOPE} is the name of the program to
> +invoke (by default @command{cscope}); and @code{CSCOPEFLAGS} and
> address@hidden are similar in meaning to their CTAGS
> +counterparts.
> +
>  Finally, Automake also emits rules to support the
>  @uref{http://www.gnu.org/software/global/, GNU Global Tags program}.
>  The @code{GTAGS} rule runs Global Tags and puts the


> diff --git a/lib/am/tags.am b/lib/am/tags.am
> index f6661e2..8c8fc52 100644
> --- a/lib/am/tags.am
> +++ b/lib/am/tags.am
> @@ -134,6 +134,29 @@ GTAGS:
>         && gtags -i $(GTAGS_ARGS) "$$here"
> 
> 
> +## ------- ##
> +## cscope  ##
> +## ------- ##
> +
> +if %?TOPDIR_P%
> +
> +CSCOPE = cscope
> +.PHONY: cscope
> +cscope: cscope.files
> +     cd $(top_builddir) && $(CSCOPE) -b -R -q -i cscope.files $(CSCOPE_ARGS)

Why are you using -R here, if the cscope.files file already lists all
files recursively?  Isn't that duplicate, and won't it pick up all
source files found in the tree, as opposed to only those belonging to
the project (and not backup copies or so)?

"cscope" should be added to AM_RECURSIVE_TARGETS here.

I suppose we should test that the cscope rule does not pick up arbitrary
other files from the source tree.

Should we pass $(AM_CSCOPEFLAGS) $(CSCOPEFLAGS) to $(CSCOPE) too?
If yes, that should be documented in the manual, too.

> +cscope.files: %CSCOPEDIRS% cscopelist
> +
> +endif %?TOPDIR_P%
> +
> +.PHONY: cscopelist
> +cscopelist: %CSCOPEDIRS% $(HEADERS) $(SOURCES) $(LISP)
> +     list='$(SOURCES) $(HEADERS) $(LISP)'; \

Should $(TAGS_FILES) be used here, too?  (I don't know.)

> +     for i in $$list; do \
> +       echo $(abs_srcdir)/$$i >> $(top_builddir)/cscope.files; \
> +     done

Generated files shouldn't have an $(abs_srcdir) stuck in front of them,
as they live in the build tree; for them you can use $(abs_builddir),
or, even better, just $(subdir), as there is no need for absolute paths
in this case.

That we need to use absolute paths to the source tree is a bit
suboptimal, as it will prevent the thing from working fine if the
absolute directory has whitespace in the name.  $(abs_srcdir) uses
should be double-quoted.

BTW, I'd put the >> redirection after the "done", so that it's done only
once, that's a wee bit more efficient.

Do I read correctly that
  make cscope
  make cscope

will cause all files to be listed at least twice in cscope.files?
That should be fixed, the toplevel directory should truncate the file.

> @@ -141,4 +164,5 @@ GTAGS:
>  .PHONY distclean-am: distclean-tags
> 
>  distclean-tags:
> -     -rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags
> +     -rm -f TAGS ID GTAGS GRTAGS GSYMS GPATH tags cscope.out cscope.in.out \
> +     cscope.po.out cscope.files

Cheers,
Ralf