[PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Yao Qi]

The recent committed CTF patch introduces a new configure option
'--with-babeltrace'.  This patch is to print the configure choice
in 'gdb --configuration'.  Is it OK?

gdb:

2013-04-12  Yao Qi  <yao@codesourcery.com>

	* top.c (print_gdb_configuration): Print configure-time
	parameter on using libbabeltrace or not.
---
 gdb/top.c |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/gdb/top.c b/gdb/top.c
index 6a402ba..480b67e 100644
--- a/gdb/top.c
+++ b/gdb/top.c
@@ -1242,6 +1242,15 @@ This GDB was configured as follows:\n\
              --without-zlib\n\
 "));
 #endif
+#if HAVE_LIBBABELTRACE
+    fprintf_filtered (stream, _("\
+             --with-babeltrace\n\
+"));
+#else
+    fprintf_filtered (stream, _("\
+             --without-babeltrace\n\
+"));
+#endif
 }
 \f
 
-- 
1.7.7.6

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> From: Yao Qi <yao@codesourcery.com>
> Date: Fri, 12 Apr 2013 18:11:15 +0800
> 
> The recent committed CTF patch introduces a new configure option
> '--with-babeltrace'.  This patch is to print the configure choice
> in 'gdb --configuration'.  Is it OK?

Looks OK to me.

Thanks.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Fri, Apr 12, 2013 at 3:11 AM, Yao Qi <yao@codesourcery.com> wrote:
> The recent committed CTF patch introduces a new configure option
> '--with-babeltrace'.  This patch is to print the configure choice
> in 'gdb --configuration'.  Is it OK?
>
> gdb:
>
> 2013-04-12  Yao Qi  <yao@codesourcery.com>
>
>         * top.c (print_gdb_configuration): Print configure-time
>         parameter on using libbabeltrace or not.
> ---
>  gdb/top.c |    9 +++++++++
>  1 files changed, 9 insertions(+), 0 deletions(-)
>
> diff --git a/gdb/top.c b/gdb/top.c
> index 6a402ba..480b67e 100644
> --- a/gdb/top.c
> +++ b/gdb/top.c
> @@ -1242,6 +1242,15 @@ This GDB was configured as follows:\n\
>               --without-zlib\n\
>  "));
>  #endif
> +#if HAVE_LIBBABELTRACE
> +    fprintf_filtered (stream, _("\
> +             --with-babeltrace\n\
> +"));
> +#else
> +    fprintf_filtered (stream, _("\
> +             --without-babeltrace\n\
> +"));
> +#endif
>  }

Do we care about printing --with-libbabeltrace-prefix?
I don't have a strong preference, but if we're going to go down this
path I think we should.
[An alternative would be to punt on details at this level and instead
record the complete string passed to configure and print that.]

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Mon, 15 Apr 2013 11:50:51 -0700
> From: Doug Evans <dje@google.com>
> Cc: gdb-patches <gdb-patches@sourceware.org>
> 
> Do we care about printing --with-libbabeltrace-prefix?
> I don't have a strong preference, but if we're going to go down this
> path I think we should.

Any configure-time option that is known to GDB at run time should be
included in the configuration description.

> [An alternative would be to punt on details at this level and instead
> record the complete string passed to configure and print that.]

That has its limitations, in that it only shows options explicitly
mentioned on the configure command line, omitting the options that
were automatically chosen by configure as result of probing the
system.  E.g., --with-expat.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Mon, Apr 15, 2013 at 10:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> [An alternative would be to punt on details at this level and instead
>> record the complete string passed to configure and print that.]
>
> That has its limitations, in that it only shows options explicitly
> mentioned on the configure command line, omitting the options that
> were automatically chosen by configure as result of probing the
> system.  E.g., --with-expat.

The intent is that it would be a supplement, not a replacement.
[Apologies if that wasn't clear ...]

Note that the current code doesn't print *where* libexpat,etc. were found.
Do we want to take the step of adding that to config.in so it's
available to gdb to print?
[that's an honest question, not a rhetorical one]
Which gets back to my earlier question:
Do we care about printing --with-libbabeltrace-prefix?

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Yao Qi]

On 04/17/2013 04:36 AM, Doug Evans wrote:
> Note that the current code doesn't print*where*  libexpat,etc. were found.
> Do we want to take the step of adding that to config.in so it's
> available to gdb to print?

When we cross compile GDB (build native arm-linux GDB on i686-pc-linux, 
for example), the location of libexpat or libbabeltrace on configuration 
time (on host) would be different with the location on run-time (on 
target).  I am afraid it is not useful to print where libexpat or 
libbabeltrace were found in the configure and build step.

-- 
Yao (齐尧)

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Tue, Apr 16, 2013 at 5:32 PM, Yao Qi <yao@codesourcery.com> wrote:
> On 04/17/2013 04:36 AM, Doug Evans wrote:
>>
>> Note that the current code doesn't print*where*  libexpat,etc. were found.
>>
>> Do we want to take the step of adding that to config.in so it's
>> available to gdb to print?
>
>
> When we cross compile GDB (build native arm-linux GDB on i686-pc-linux, for
> example), the location of libexpat or libbabeltrace on configuration time
> (on host) would be different with the location on run-time (on target).  I
> am afraid it is not useful to print where libexpat or libbabeltrace were
> found in the configure and build step.

nit: For a canadian cross, s/(on host)/(on build)/ and s/(on target)/(on host)/.

It's certainly less useful, but since we're going down this path I
wanted to at least raise the question.
GCC prints this info.

Also, I wonder if we shouild print prefix and/or exec_prefix.
We print "(relocatable)" for relocatable directories but I'm left
guessing what the root of the relocatable tree is.
[We also don't explain what "relocatable" means AFAICS.  I know, but I
can imagine a user wondering.]

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Yao Qi]

On 04/12/2013 08:33 PM, Eli Zaretskii wrote:
>> From: Yao Qi <yao@codesourcery.com>
>> Date: Fri, 12 Apr 2013 18:11:15 +0800
>>
>> The recent committed CTF patch introduces a new configure option
>> '--with-babeltrace'.  This patch is to print the configure choice
>> in 'gdb --configuration'.  Is it OK?
>
> Looks OK to me.
>

Patch is committed.  Thanks for the review.

-- 
Yao (齐尧)

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Yao Qi]

On 04/17/2013 08:55 AM, Doug Evans wrote:
> nit: For a canadian cross, s/(on host)/(on build)/ and s/(on target)/(on host)/.
>
Oh, you are right.

> It's certainly less useful, but since we're going down this path I
> wanted to at least raise the question.
> GCC prints this info.
>

Yes, 'gcc -###' prints some directories on the build machine.

> Also, I wonder if we shouild print prefix and/or exec_prefix.
> We print "(relocatable)" for relocatable directories but I'm left
> guessing what the root of the relocatable tree is.
> [We also don't explain what "relocatable" means AFAICS.  I know, but I
> can imagine a user wondering.]

The "relocatable" is unclear to me unless I checked configure.ac and 
previous discussions.
-- 
Yao (齐尧)

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Tue, 16 Apr 2013 13:36:54 -0700
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> On Mon, Apr 15, 2013 at 10:54 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> [An alternative would be to punt on details at this level and instead
> >> record the complete string passed to configure and print that.]
> >
> > That has its limitations, in that it only shows options explicitly
> > mentioned on the configure command line, omitting the options that
> > were automatically chosen by configure as result of probing the
> > system.  E.g., --with-expat.
> 
> The intent is that it would be a supplement, not a replacement.
> [Apologies if that wasn't clear ...]

Sorry for my misunderstanding.  In that case, it's probably a good
idea, if there are important things that we cannot catch otherwise.

> Note that the current code doesn't print *where* libexpat,etc. were found.
> Do we want to take the step of adding that to config.in so it's
> available to gdb to print?
> [that's an honest question, not a rhetorical one]
> Which gets back to my earlier question:
> Do we care about printing --with-libbabeltrace-prefix?

I'm not sure that place is of interest to anyone but the person who
configured GDB.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Wed, 17 Apr 2013 10:54:10 +0800
> From: Yao Qi <yao@codesourcery.com>
> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches <gdb-patches@sourceware.org>
> 
> The "relocatable" is unclear to me unless I checked configure.ac and 
> previous discussions.

I will add something to explain that, assuming I will find a way to
say that concisely.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Wed, Apr 17, 2013 at 10:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Wed, 17 Apr 2013 10:54:10 +0800
>> From: Yao Qi <yao@codesourcery.com>
>> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches <gdb-patches@sourceware.org>
>>
>> The "relocatable" is unclear to me unless I checked configure.ac and
>> previous discussions.
>
> I will add something to explain that, assuming I will find a way to
> say that concisely.

[for completeness sake]
I'd be ok with just adding something to the manual.
It would be good to document the fact that a gdb install tree can be
moved and gdb will still find its relocatable ancillary files.
This goes along with the ability to do "make install DESTDIR=/foo"
which is a nice capability.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Wed, 17 Apr 2013 10:25:23 -0700
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> On Wed, Apr 17, 2013 at 10:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> >> Date: Wed, 17 Apr 2013 10:54:10 +0800
> >> From: Yao Qi <yao@codesourcery.com>
> >> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches <gdb-patches@sourceware.org>
> >>
> >> The "relocatable" is unclear to me unless I checked configure.ac and
> >> previous discussions.
> >
> > I will add something to explain that, assuming I will find a way to
> > say that concisely.
> 
> [for completeness sake]
> I'd be ok with just adding something to the manual.
> It would be good to document the fact that a gdb install tree can be
> moved and gdb will still find its relocatable ancillary files.
> This goes along with the ability to do "make install DESTDIR=/foo"
> which is a nice capability.

Is the below OK?

Index: gdb/top.c
===================================================================
RCS file: /cvs/src/src/gdb/top.c,v
retrieving revision 1.236
diff -u -p -r1.236 top.c
--- gdb/top.c	30 May 2013 17:27:44 -0000	1.236
+++ gdb/top.c	22 Jun 2013 11:30:47 -0000
@@ -1252,6 +1252,9 @@ This GDB was configured as follows:\n\
              --without-babeltrace\n\
 "));
 #endif
+    fprintf_filtered (stream, _("\n\
+ (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
+"));
 }
 \f
 

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Sergio Durigan Junior]

On Saturday, June 22 2013, Eli Zaretskii wrote:

> Is the below OK?
>
> Index: gdb/top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 top.c
> --- gdb/top.c	30 May 2013 17:27:44 -0000	1.236
> +++ gdb/top.c	22 Jun 2013 11:30:47 -0000
> @@ -1252,6 +1252,9 @@ This GDB was configured as follows:\n\
>               --without-babeltrace\n\
>  "));
>  #endif
> +    fprintf_filtered (stream, _("\n\
> + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
                         ^^^

"means the directory" ?

-- 
Sergio

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> From: Sergio Durigan Junior <sergiodj@redhat.com>
> Cc: Doug Evans <dje@google.com>, yao@codesourcery.com,
>         gdb-patches@sourceware.org
> Date: Sat, 22 Jun 2013 17:02:58 -0300
> 
> > +    fprintf_filtered (stream, _("\n\
> > + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
>                          ^^^
> 
> "means the directory" ?

That will overflow the 80-column line.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Sergio Durigan Junior]

On Saturday, June 22 2013, Eli Zaretskii wrote:

>> From: Sergio Durigan Junior <sergiodj@redhat.com>
>> Cc: Doug Evans <dje@google.com>, yao@codesourcery.com,
>>         gdb-patches@sourceware.org
>> Date: Sat, 22 Jun 2013 17:02:58 -0300
>> 
>> > +    fprintf_filtered (stream, _("\n\
>> > + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
>>                          ^^^
>> 
>> "means the directory" ?
>
> That will overflow the 80-column line.

Isn't it possible to break the string?

Anyway, I am out of context and I was just pointing out that the
sentence seemed strange/incomplete to me.

-- 
Sergio

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Sat, Jun 22, 2013 at 4:31 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Wed, 17 Apr 2013 10:25:23 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
>>
>> On Wed, Apr 17, 2013 at 10:01 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> Date: Wed, 17 Apr 2013 10:54:10 +0800
>> >> From: Yao Qi <yao@codesourcery.com>
>> >> CC: Eli Zaretskii <eliz@gnu.org>, gdb-patches <gdb-patches@sourceware.org>
>> >>
>> >> The "relocatable" is unclear to me unless I checked configure.ac and
>> >> previous discussions.
>> >
>> > I will add something to explain that, assuming I will find a way to
>> > say that concisely.
>>
>> [for completeness sake]
>> I'd be ok with just adding something to the manual.
>> It would be good to document the fact that a gdb install tree can be
>> moved and gdb will still find its relocatable ancillary files.
>> This goes along with the ability to do "make install DESTDIR=/foo"
>> which is a nice capability.
>
> Is the below OK?
>
> Index: gdb/top.c
> ===================================================================
> RCS file: /cvs/src/src/gdb/top.c,v
> retrieving revision 1.236
> diff -u -p -r1.236 top.c
> --- gdb/top.c   30 May 2013 17:27:44 -0000      1.236
> +++ gdb/top.c   22 Jun 2013 11:30:47 -0000
> @@ -1252,6 +1252,9 @@ This GDB was configured as follows:\n\
>               --without-babeltrace\n\
>  "));
>  #endif
> +    fprintf_filtered (stream, _("\n\
> + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
> +"));
>  }

If you want to print something here, then I think you need to take
into account whether "relocatable" is printed at all.
There are lots of places where it may be printed, and each is
conditioned by a macro like FOO_RELOCATABLE.
Taking into account all those macros (and having to maintain the
result over time) seems problematic.

How about just a sentence or two in the manual near the description of
--configuration (or some such).

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Mon, 24 Jun 2013 11:35:31 -0700
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> > diff -u -p -r1.236 top.c
> > --- gdb/top.c   30 May 2013 17:27:44 -0000      1.236
> > +++ gdb/top.c   22 Jun 2013 11:30:47 -0000
> > @@ -1252,6 +1252,9 @@ This GDB was configured as follows:\n\
> >               --without-babeltrace\n\
> >  "));
> >  #endif
> > +    fprintf_filtered (stream, _("\n\
> > + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
> > +"));
> >  }
> 
> If you want to print something here, then I think you need to take
> into account whether "relocatable" is printed at all.

I don't think it's a real danger.  One must invest a non-trivial
effort to have GDB configured without a single relocatable directory.

The requirement was to explain this succinctly.  If the above is not
deemed good enough, I'd rather not say anything at all.

> How about just a sentence or two in the manual near the description of
> --configuration (or some such).

I tried that, but found no good place to say that.  Just saying
something where --configuration is described will get this lost in the
humongous manual, and users will not necessarily know where to look.

Again, if the above is not good enough, let's forget about the whole
thing.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Mon, Jun 24, 2013 at 11:44 AM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 24 Jun 2013 11:35:31 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
>>
>> > diff -u -p -r1.236 top.c
>> > --- gdb/top.c   30 May 2013 17:27:44 -0000      1.236
>> > +++ gdb/top.c   22 Jun 2013 11:30:47 -0000
>> > @@ -1252,6 +1252,9 @@ This GDB was configured as follows:\n\
>> >               --without-babeltrace\n\
>> >  "));
>> >  #endif
>> > +    fprintf_filtered (stream, _("\n\
>> > + (\"Relocatable\" means directory can be moved with the GDB installation tree.)\n\
>> > +"));
>> >  }
>>
>> If you want to print something here, then I think you need to take
>> into account whether "relocatable" is printed at all.
>
> I don't think it's a real danger.  One must invest a non-trivial
> effort to have GDB configured without a single relocatable directory.

Ok, How about:

+    /* We assume "relocatable" will be printed at least once, thus we always
+       print this text.  It's a reasonably safe assumption for now.  */
+    fprintf_filtered (stream, _("\n\
+ (\"Relocatable\" means the directory can be moved with the GDB installation"
"tree and GDB will still find it.)\n\"
+"));

[N.B. I didn't verify 80-column-ness, leaving that to you if you like
the wording.]

>> How about just a sentence or two in the manual near the description of
>> --configuration (or some such).
>
> I tried that, but found no good place to say that.  Just saying
> something where --configuration is described will get this lost in the
> humongous manual, and users will not necessarily know where to look.

"relocatable" doesn't appear that many times in the manual.
"(relocatable)" doesn't currently appear at all.
If a user can't find it I'm not worried about claims it's the manual's fault.

Having this text is not important to me though, so feel free to drop it.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Mon, 24 Jun 2013 12:03:40 -0700
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> Ok, How about:
> 
> +    /* We assume "relocatable" will be printed at least once, thus we always
> +       print this text.  It's a reasonably safe assumption for now.  */
> +    fprintf_filtered (stream, _("\n\
> + (\"Relocatable\" means the directory can be moved with the GDB installation"
> "tree and GDB will still find it.)\n\"
> +"));
> 
> [N.B. I didn't verify 80-column-ness, leaving that to you if you like
> the wording.]

This clearly exceeds 80 columns.  We can split it into 2 lines, if
people like this wording enough to sacrifice another line.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Doug Evans]

On Mon, Jun 24, 2013 at 12:22 PM, Eli Zaretskii <eliz@gnu.org> wrote:
>> Date: Mon, 24 Jun 2013 12:03:40 -0700
>> From: Doug Evans <dje@google.com>
>> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
>>
>> Ok, How about:
>>
>> +    /* We assume "relocatable" will be printed at least once, thus we always
>> +       print this text.  It's a reasonably safe assumption for now.  */
>> +    fprintf_filtered (stream, _("\n\
>> + (\"Relocatable\" means the directory can be moved with the GDB installation"
>> "tree and GDB will still find it.)\n\"
>> +"));
>>
>> [N.B. I didn't verify 80-column-ness, leaving that to you if you like
>> the wording.]
>
> This clearly exceeds 80 columns.  We can split it into 2 lines, if
> people like this wording enough to sacrifice another line.

Heh, that's what I intended.  s,installation,installation\n\,
Sorry for the confusion.

Re: [PATCH] print '--with{,out}-babeltrace' in 'gdb --configuration'

[Eli Zaretskii]

> Date: Mon, 24 Jun 2013 12:03:40 -0700
> From: Doug Evans <dje@google.com>
> Cc: Yao Qi <yao@codesourcery.com>, gdb-patches <gdb-patches@sourceware.org>
> 
> Ok, How about:
> 
> +    /* We assume "relocatable" will be printed at least once, thus we always
> +       print this text.  It's a reasonably safe assumption for now.  */
> +    fprintf_filtered (stream, _("\n\
> + (\"Relocatable\" means the directory can be moved with the GDB installation"
> "tree and GDB will still find it.)\n\"
> +"));

Committed with necessary newline additions.

Thanks.