<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 + +: