diff mbox

[v3,1/1] package/gdb: host-texinfo needed for full gdb

Message ID 1441740443-7280-1-git-send-email-brendanheading@gmail.com
State Superseded, archived
Headers show

Commit Message

Brendan Heading Sept. 8, 2015, 7:27 p.m. UTC
Fixes :
http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/

.. and many others.

when building the full gdb (ie not just gdbserver) on the host or target
the "make install" phase seems to expect makeinfo (from texinfo) to be
present.

Added a dependency so that the host always depends on host-texinfo, and
the target depends on it if the full GDB is being built. No need to set
ac_cv_prog_MAKEINFO anymore as it doesn't seem to achieve the desired
effect.

Signed-off-by: Brendan Heading <brendanheading@gmail.com>
---
v1->v2: fixed for target, removed duplication in gdb.mk. Used better
subject suggested by Vicente.

v2->v3: fix typos, fixed subject to reflect that the issue applies to
gdb in general (host and target).
---
 package/gdb/gdb.mk | 11 +++--------
 1 file changed, 3 insertions(+), 8 deletions(-)

Comments

Arnout Vandecappelle Sept. 8, 2015, 9:50 p.m. UTC | #1
On 08-09-15 21:27, Brendan Heading wrote:
> Fixes :
> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/

 So, we now have three versions to fix this:

Romain patches gdb to avoid recursing in the doc directory [1][2]
Vicente does the same but with a sed instead of patch [3].
Brendan adds host-texinfo to dependencies (this patch) [4].

 Which one do we choose?

 host-texinfo takes a bit of time to build because it also depends on
host-ncurses, but it's not a huge problem.

 The best solution is adding an upstreamable --disable-doc, but that will take a
bit of work still (esp. because we can't easily autoreconfigure).

 So for me Brendan's patch is fine. What do the others think?


 Regards,
 Arnout


[1] http://patchwork.ozlabs.org/patch/514907/
[2] http://patchwork.ozlabs.org/patch/514908/
[3] http://patchwork.ozlabs.org/patch/515401/
[4] http://patchwork.ozlabs.org/patch/515527/

> 
> .. and many others.
> 
> when building the full gdb (ie not just gdbserver) on the host or target
> the "make install" phase seems to expect makeinfo (from texinfo) to be
> present.
> 
> Added a dependency so that the host always depends on host-texinfo, and
> the target depends on it if the full GDB is being built. No need to set
> ac_cv_prog_MAKEINFO anymore as it doesn't seem to achieve the desired
> effect.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
[snip]
Brendan Heading Sept. 8, 2015, 10:02 p.m. UTC | #2
> Romain patches gdb to avoid recursing in the doc directory [1][2]
> Vicente does the same but with a sed instead of patch [3].
> Brendan adds host-texinfo to dependencies (this patch) [4].
>
>  Which one do we choose?
>
>  host-texinfo takes a bit of time to build because it also depends on
> host-ncurses, but it's not a huge problem.

I noted that, but in any scenario where host-texinfo is needed,
ncurses was already a dependency anyway.

On my W520 (i7-2760QM, SSD, Fedora 22, btrfs) with ccache it's taking
11 seconds to rebuild host-texinfo, most of that being the time taken
by the configure script. But I appreciate there are folks working on
slower hardware.

>  The best solution is adding an upstreamable --disable-doc, but that will take a
> bit of work still (esp. because we can't easily autoreconfigure).

I looked very briefly at the idea of trying to make it not build docs,
but was scared off by the huge tree with lots of Makefiles. I guessed
it would be a day or so's work, and quicker just to add the dep.

regards

Brendan
Romain Naour Sept. 8, 2015, 10:09 p.m. UTC | #3
Hi Arnout, All,

Le 08/09/2015 23:50, Arnout Vandecappelle a écrit :
> On 08-09-15 21:27, Brendan Heading wrote:
>> Fixes :
>> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/
> 
>  So, we now have three versions to fix this:
> 
> Romain patches gdb to avoid recursing in the doc directory [1][2]
> Vicente does the same but with a sed instead of patch [3].
> Brendan adds host-texinfo to dependencies (this patch) [4].
> 
>  Which one do we choose?
> 
>  host-texinfo takes a bit of time to build because it also depends on
> host-ncurses, but it's not a huge problem.
> 
>  The best solution is adding an upstreamable --disable-doc, but that will take a
> bit of work still (esp. because we can't easily autoreconfigure).

I sent a patch to gdb mailing list to add --disable-docs option but it's not
tested since I can't regenerate the configure script locally.

See https://sourceware.org/ml/gdb-patches/2015-09/msg00073.html

I'm waiting for a review.

> 
>  So for me Brendan's patch is fine. What do the others think?

I would prefer to avoid host-texinfo dependency if possible.

Best regards,
Romain

> 
> 
>  Regards,
>  Arnout
> 
> 
> [1] http://patchwork.ozlabs.org/patch/514907/
> [2] http://patchwork.ozlabs.org/patch/514908/
> [3] http://patchwork.ozlabs.org/patch/515401/
> [4] http://patchwork.ozlabs.org/patch/515527/
> 
>>
>> .. and many others.
>>
>> when building the full gdb (ie not just gdbserver) on the host or target
>> the "make install" phase seems to expect makeinfo (from texinfo) to be
>> present.
>>
>> Added a dependency so that the host always depends on host-texinfo, and
>> the target depends on it if the full GDB is being built. No need to set
>> ac_cv_prog_MAKEINFO anymore as it doesn't seem to achieve the desired
>> effect.
>>
>> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
> [snip]
> 
>
Arnout Vandecappelle Sept. 8, 2015, 10:29 p.m. UTC | #4
On 09-09-15 00:09, Romain Naour wrote:
> Hi Arnout, All,
> 
> Le 08/09/2015 23:50, Arnout Vandecappelle a écrit :
>> On 08-09-15 21:27, Brendan Heading wrote:
>>> Fixes :
>>> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/
>>
>>  So, we now have three versions to fix this:
>>
>> Romain patches gdb to avoid recursing in the doc directory [1][2]
>> Vicente does the same but with a sed instead of patch [3].
>> Brendan adds host-texinfo to dependencies (this patch) [4].
>>
>>  Which one do we choose?
>>
>>  host-texinfo takes a bit of time to build because it also depends on
>> host-ncurses, but it's not a huge problem.
>>
>>  The best solution is adding an upstreamable --disable-doc, but that will take a
>> bit of work still (esp. because we can't easily autoreconfigure).
> 
> I sent a patch to gdb mailing list to add --disable-docs option but it's not
> tested since I can't regenerate the configure script locally.
> 
> See https://sourceware.org/ml/gdb-patches/2015-09/msg00073.html
> 
> I'm waiting for a review.
> 
>>
>>  So for me Brendan's patch is fine. What do the others think?
> 
> I would prefer to avoid host-texinfo dependency if possible.

 I agree, but perhaps we can commit Brendan's patch until the --disable-docs
option is there.

 Regards,
 Arnout


[snip[
Vicente Olivert Riera Sept. 9, 2015, 9:04 a.m. UTC | #5
Dear all,

On 09/08/2015 11:29 PM, Arnout Vandecappelle wrote:
> On 09-09-15 00:09, Romain Naour wrote:
>> Hi Arnout, All,
>>
>> Le 08/09/2015 23:50, Arnout Vandecappelle a écrit :
>>> On 08-09-15 21:27, Brendan Heading wrote:
>>>> Fixes :
>>>> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/
>>>
>>>  So, we now have three versions to fix this:
>>>
>>> Romain patches gdb to avoid recursing in the doc directory [1][2]
>>> Vicente does the same but with a sed instead of patch [3].
>>> Brendan adds host-texinfo to dependencies (this patch) [4].
>>>
>>>  Which one do we choose?
>>>
>>>  host-texinfo takes a bit of time to build because it also depends on
>>> host-ncurses, but it's not a huge problem.
>>>
>>>  The best solution is adding an upstreamable --disable-doc, but that will take a
>>> bit of work still (esp. because we can't easily autoreconfigure).
>>
>> I sent a patch to gdb mailing list to add --disable-docs option but it's not
>> tested since I can't regenerate the configure script locally.
>>
>> See https://sourceware.org/ml/gdb-patches/2015-09/msg00073.html
>>
>> I'm waiting for a review.
>>
>>>
>>>  So for me Brendan's patch is fine. What do the others think?
>>
>> I would prefer to avoid host-texinfo dependency if possible.
> 
>  I agree, but perhaps we can commit Brendan's patch until the --disable-docs
> option is there.

Brendan's patch while we wait for --disable-docs sounds as the best
option for me as well.

Regards,

Vincent.

>  Regards,
>  Arnout
> 
> 
> [snip[
> 
>
Vicente Olivert Riera Sept. 9, 2015, 9:18 a.m. UTC | #6
Dear Brendan Heading,

On 09/08/2015 08:27 PM, Brendan Heading wrote:
> Fixes :
> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/
> 
> .. and many others.
> 
> when building the full gdb (ie not just gdbserver) on the host or target
> the "make install" phase seems to expect makeinfo (from texinfo) to be
> present.
> 
> Added a dependency so that the host always depends on host-texinfo, and
> the target depends on it if the full GDB is being built. No need to set
> ac_cv_prog_MAKEINFO anymore as it doesn't seem to achieve the desired
> effect.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>
Reviewed-by: Vicente Olivert Riera <Vincent.Riera@imgtec.com>

Regards,

Vincent.

> ---
> v1->v2: fixed for target, removed duplication in gdb.mk. Used better
> subject suggested by Vicente.
> 
> v2->v3: fix typos, fixed subject to reflect that the issue applies to
> gdb in general (host and target).
> ---
>  package/gdb/gdb.mk | 11 +++--------
>  1 file changed, 3 insertions(+), 8 deletions(-)
> 
> diff --git a/package/gdb/gdb.mk b/package/gdb/gdb.mk
> index 338de20..3114fd4 100644
> --- a/package/gdb/gdb.mk
> +++ b/package/gdb/gdb.mk
> @@ -33,7 +33,8 @@ ifeq ($(BR2_PACKAGE_GDB_DEBUGGER),)
>  GDB_SUBDIR = gdb/gdbserver
>  HOST_GDB_SUBDIR = .
>  else
> -GDB_DEPENDENCIES = ncurses \
> +# need makeinfo if building the full debugger
> +GDB_DEPENDENCIES = ncurses host-texinfo \
>  	$(if $(BR2_PACKAGE_LIBICONV),libiconv)
>  endif
>  
> @@ -165,14 +166,8 @@ else
>  HOST_GDB_CONF_OPTS += --without-python
>  endif
>  
> -ifeq ($(GDB_FROM_GIT),y)
> -GDB_DEPENDENCIES += host-texinfo
> +# since we always build the full GDB on the host, texinfo/makeinfo is required
>  HOST_GDB_DEPENDENCIES += host-texinfo
> -else
> -# don't generate documentation
> -GDB_CONF_ENV += ac_cv_prog_MAKEINFO=missing
> -HOST_GDB_CONF_ENV += ac_cv_prog_MAKEINFO=missing
> -endif
>  
>  # legacy $arch-linux-gdb symlink
>  define HOST_GDB_ADD_SYMLINK
>
Thomas Petazzoni Sept. 9, 2015, 9:51 a.m. UTC | #7
Hello,

On Tue, 8 Sep 2015 23:50:39 +0200, Arnout Vandecappelle wrote:

> Romain patches gdb to avoid recursing in the doc directory [1][2]
> Vicente does the same but with a sed instead of patch [3].
> Brendan adds host-texinfo to dependencies (this patch) [4].
> 
>  Which one do we choose?
> 
>  host-texinfo takes a bit of time to build because it also depends on
> host-ncurses, but it's not a huge problem.
> 
>  The best solution is adding an upstreamable --disable-doc, but that will take a
> bit of work still (esp. because we can't easily autoreconfigure).
> 
>  So for me Brendan's patch is fine. What do the others think?

On my side, I think I'd prefer for now a $(SED) to remove the doc
directory from the SUBDIRS, in Makefile.in (avoids autoreconf, avoids
per-gdb version patches). And then, Romain has contributed to gdb a
patch to disable the documentation, but as you said, it will take a
while before this patch 1/ gets accepted and 2/ is used in all gdb
versions packaged in Buildroot.

I think I would actually prefer to also do the same trick for all
packages that need host-texinfo, and remove the host-texinfo package.
The host-texinfo package (as is documented in texinfo.mk) was
originally added only because Crosstool-NG needed it as a dependency,
back when we had a Crosstool-NG backend. Now that this backend is done,
I'm not sure it make a lot of sense to build a tool to build
documentation, which nobody will ever use/read.

So I would favor Vicente's patch in fact. Yes, it is not nice to $(SED)
stuff, but it is a whole lot better than building host-texinfo,
host-ncurses, and some totally unused documentation.

Best regards,

Thomas
Thomas Petazzoni Sept. 11, 2015, 5:34 p.m. UTC | #8
Brendan,

On Tue,  8 Sep 2015 20:27:23 +0100, Brendan Heading wrote:
> Fixes :
> http://autobuild.buildroot.net/results/dd5/dd50ed99abb2c8495def826866b184030953f90e/
> 
> .. and many others.
> 
> when building the full gdb (ie not just gdbserver) on the host or target
> the "make install" phase seems to expect makeinfo (from texinfo) to be
> present.
> 
> Added a dependency so that the host always depends on host-texinfo, and
> the target depends on it if the full GDB is being built. No need to set
> ac_cv_prog_MAKEINFO anymore as it doesn't seem to achieve the desired
> effect.
> 
> Signed-off-by: Brendan Heading <brendanheading@gmail.com>

Following the discussion on the mailing list, I've merged Vicente's
proposal, and marked this patch as Rejected.

Thanks!

Thomas
diff mbox

Patch

diff --git a/package/gdb/gdb.mk b/package/gdb/gdb.mk
index 338de20..3114fd4 100644
--- a/package/gdb/gdb.mk
+++ b/package/gdb/gdb.mk
@@ -33,7 +33,8 @@  ifeq ($(BR2_PACKAGE_GDB_DEBUGGER),)
 GDB_SUBDIR = gdb/gdbserver
 HOST_GDB_SUBDIR = .
 else
-GDB_DEPENDENCIES = ncurses \
+# need makeinfo if building the full debugger
+GDB_DEPENDENCIES = ncurses host-texinfo \
 	$(if $(BR2_PACKAGE_LIBICONV),libiconv)
 endif
 
@@ -165,14 +166,8 @@  else
 HOST_GDB_CONF_OPTS += --without-python
 endif
 
-ifeq ($(GDB_FROM_GIT),y)
-GDB_DEPENDENCIES += host-texinfo
+# since we always build the full GDB on the host, texinfo/makeinfo is required
 HOST_GDB_DEPENDENCIES += host-texinfo
-else
-# don't generate documentation
-GDB_CONF_ENV += ac_cv_prog_MAKEINFO=missing
-HOST_GDB_CONF_ENV += ac_cv_prog_MAKEINFO=missing
-endif
 
 # legacy $arch-linux-gdb symlink
 define HOST_GDB_ADD_SYMLINK