From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: Received: (qmail 120111 invoked by alias); 12 Jan 2018 04:18:00 -0000 Mailing-List: contact gdb-patches-help@sourceware.org; run by ezmlm Precedence: bulk List-Id: List-Subscribe: List-Archive: List-Post: List-Help: , Sender: gdb-patches-owner@sourceware.org Received: (qmail 119494 invoked by uid 89); 12 Jan 2018 04:17:59 -0000 Authentication-Results: sourceware.org; auth=none X-Virus-Found: No X-Spam-SWARE-Status: No, score=-25.9 required=5.0 tests=BAYES_00,GIT_PATCH_0,GIT_PATCH_1,GIT_PATCH_2,GIT_PATCH_3,KAM_LAZY_DOMAIN_SECURITY,SPF_HELO_PASS,T_RP_MATCHES_RCVD autolearn=ham version=3.3.2 spammy= X-HELO: mx1.redhat.com Received: from mx1.redhat.com (HELO mx1.redhat.com) (209.132.183.28) by sourceware.org (qpsmtpd/0.93/v0.84-503-g423c35a) with ESMTP; Fri, 12 Jan 2018 04:17:58 +0000 Received: from smtp.corp.redhat.com (int-mx01.intmail.prod.int.phx2.redhat.com [10.5.11.11]) (using TLSv1.2 with cipher AECDH-AES256-SHA (256/256 bits)) (No client certificate requested) by mx1.redhat.com (Postfix) with ESMTPS id BA834859FA; Fri, 12 Jan 2018 04:17:56 +0000 (UTC) Received: from localhost (unused-10-15-17-193.yyz.redhat.com [10.15.17.193]) by smtp.corp.redhat.com (Postfix) with ESMTPS id 51742605DE; Fri, 12 Jan 2018 04:17:56 +0000 (UTC) From: Sergio Durigan Junior To: Simon Marchi Cc: GDB Patches , Tom Tromey , Eli Zaretskii Subject: Re: [PATCH] Install and generate docs for gdb-add-index References: <20180111213530.23774-1-sergiodj@redhat.com> <43156279-dce3-2b0f-11d2-fc12061a2d55@ericsson.com> Date: Fri, 12 Jan 2018 04:18:00 -0000 In-Reply-To: <43156279-dce3-2b0f-11d2-fc12061a2d55@ericsson.com> (Simon Marchi's message of "Thu, 11 Jan 2018 17:27:31 -0500") Message-ID: <876087d824.fsf@redhat.com> User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.3 (gnu/linux) MIME-Version: 1.0 Content-Type: text/plain X-IsSubscribed: yes X-SW-Source: 2018-01/txt/msg00235.txt.bz2 On Thursday, January 11 2018, Simon Marchi wrote: > 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 >> 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. Thanks for the review, Simon. >> 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? Ah, you're right. Strange, I don't know why it is indented this way originally. Anyway, fixed it. >> @$(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. That's a good point. I moved the text up (after the first paragraph, as suggested) and modified it a bit: ... @node Index Files @section Index Files Speed Up @value{GDBN} @cindex index files @cindex @samp{.gdb_index} section When @value{GDBN} finds a symbol file, it scans the symbols in the file in order to construct an internal symbol table. This lets most @value{GDBN} operations work quickly---at the cost of a delay early on. For large programs, this delay can be quite lengthy, so @value{GDBN} provides a way to build an index, which speeds up startup. For convenience, @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 It is also possible to do the work manually. Here is what @command{gdb-add-index} does behind the curtains. ... I will submit the full patch as a v2. Thanks, -- Sergio GPG key ID: 237A 54B1 0287 28BF 00EF 31F4 D0EB 7628 65FC 5E36 Please send encrypted e-mail if possible http://sergiodj.net/