Re: [PATCH] Install and generate docs for gdb-add-index

[Simon Marchi <simon.marchi@ericsson.com>]

On 2018-01-11 04:35 PM, Sergio Durigan Junior wrote:
> The "gdb-add-index" script has been resurrected on:
> 
>   commit caf26be91a584ef141ac5d3cb31007731af8b8e3
>   Author: Samuel Bronson <naesten@gmail.com>
>   Date:   Fri Nov 15 16:09:33 2013 -0500
> 
>       Resurrect gdb-add-index as a contrib script
> 
> However, for some reason (I couldn't find it in the archives), only
> the script has been checked-in; the Makefile parts responsible for
> installing it in the system were left out.  This commit fixes that, by
> also resurrecting the Makefile and documentation bits.
> 
> This commit is part of our effort to upstream the local Fedora GDB
> changes.  With this commit, we'll only carry a very small
> Fedora-specific modification to the script.

Woo thanks!

I have some comments, but I'm just being picky.

> diff --git a/gdb/Makefile.in b/gdb/Makefile.in
> index 17b71c6e7c..d63220db4e 100644
> --- a/gdb/Makefile.in
> +++ b/gdb/Makefile.in
> @@ -1770,6 +1770,15 @@ install-only: $(CONFIG_INSTALL)
>  		  $(INSTALL_SCRIPT) gcore \
>  			  $(DESTDIR)$(bindir)/$$transformed_name; \
>  	fi
> +	transformed_name=`t='$(program_transform_name)'; \
> +			  echo gdb-add-index | sed -e "$$t"` ; \
> +		if test "x$$transformed_name" = x; then \
> +		  transformed_name=gdb-add-index ; \
> +		else \
> +		  true ; \
> +		fi ; \
> +		$(INSTALL_PROGRAM) $(srcdir)/contrib/gdb-add-index.sh \
> +			$(DESTDIR)$(bindir)/$$transformed_name$(EXEEXT)

I know this is just copy pasted from above, but doesn't the indentation look odd?
Shouldn't the if/else/fi and the INSTALL_PROGRAM lines be indented with one tab
less?

>  	@$(MAKE) DO=install "DODIRS=$(SUBDIRS)" $(FLAGS_TO_PASS) subdir_do
>  
>  install-strip:
> diff --git a/gdb/doc/Makefile.in b/gdb/doc/Makefile.in
> index eafadee899..0323bc4ecb 100644
> --- a/gdb/doc/Makefile.in
> +++ b/gdb/doc/Makefile.in
> @@ -174,7 +174,7 @@ POD2MAN5 = pod2man --center="GNU Development Tools" \
>  		   --release="gdb-`sed q version.subst`" --section=5
>  
>  # List of man pages generated from gdb.texi
> -MAN1S = gdb.1 gdbserver.1 gcore.1
> +MAN1S = gdb.1 gdbserver.1 gcore.1 gdb-add-index.1
>  MAN5S = gdbinit.5
>  MANS = $(MAN1S) $(MAN5S)
>  
> @@ -633,6 +633,13 @@ gcore.1: $(GDB_DOC_FILES)
>  		mv -f $@.T$$$$ $@) || (rm -f $@.T$$$$ && exit 1)
>  	rm -f gcore.pod
>  
> +gdb-add-index.1: $(GDB_DOC_FILES)
> +	touch $@
> +	-$(TEXI2POD) $(MANCONF) -Dgdb-add-index < $(srcdir)/gdb.texinfo > gdb-add-index.pod
> +	-($(POD2MAN1) gdb-add-index.pod | sed -e '/^.if n .na/d' > $@.T$$$$ && \
> +		mv -f $@.T$$$$ $@) || (rm -f $@.T$$$$ && exit 1)
> +	rm -f gdb-add-index.pod
> +
>  gdbinit.5: $(GDB_DOC_FILES)
>  	touch $@
>  	-$(TEXI2POD) $(MANCONF) -Dgdbinit < $(srcdir)/gdb.texinfo > gdbinit.pod
> diff --git a/gdb/doc/gdb.texinfo b/gdb/doc/gdb.texinfo
> index dba2fa766a..62fcdd524e 100644
> --- a/gdb/doc/gdb.texinfo
> +++ b/gdb/doc/gdb.texinfo
> @@ -19775,6 +19775,14 @@ There are currently some limitation on indices.  They only work when
>  for DWARF debugging information, not stabs.  And, they do not
>  currently work for programs using Ada.
>  
> +@value{GDBN} comes with a program, @command{gdb-add-index}, which can
> +be used to add the index to a symbol file.  It takes the symbol file
> +as its only argument:
> +
> +@smallexample
> +$ gdb-add-index symfile
> +@end smallexample
> +

Would it be good to put the part about gdb-add-index earlier in the page/section,
for example just after the first paragraph?  The doc starts by telling you the
manual method with the scary objcopy lines and all.  I think it would be nice if
it started by telling: here's the easy and recommended way, and then here are all
the gory details about how it's done internally, if you need it.  I could imagine
users starting to read the section, and stopping midway because they think it's
too complicated/not worth it, and never get to the gdb-add-index part.

Simon