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

Simon Marchi
2018-01-11T22:27:52Z

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