Re: [PATCH] toplevel: Makefile.def: Make configure-sim depend on all-readline

[Christophe Lyon <christophe.lyon@arm.com>]

On 7/16/22 00:20, Hans-Peter Nilsson wrote:
> On Tue, 12 Jul 2022, Luis Machado via Gdb-patches wrote:
> 
>> Hi,
>>
>> On 3/8/22 19:21, Hans-Peter Nilsson via Binutils wrote:
>>> Calling on "global maintainers" as per toplevel/MAINTAINERS
>>> for "Makefile.*".
>>>
>>> Ok to commit?
>>>
>>> (If so, I'll also commit this change to the gcc repo, where
>>> this dependency is normally unused, i.e. when source trees
>>> are kept separate.)
>>>
>>> brgds, H-P
>>> ----- 8< -----
>>>
>>> Without this, a "make all-sim" without the equivalent of
>>> libreadline-dev installed on the build system, won't
>>> properly pick up the in-tree readline build, and you'll see:
>>>
>>> mkdir -p -- ./sim
>>> Configuring in ./sim
>>> configure: creating cache ./config.cache
>>> checking build system type... x86_64-pc-linux-gnu
>>> checking host system type... x86_64-pc-linux-gnu
>>> checking target system type... cris-axis-elf
>>> checking for x86_64-pc-linux-gnu-gcc... gcc
>>> checking whether the C compiler works... yes
>>> ...
>>> checking for library containing tgetent... -ltermcap
>>> checking for readline in -lreadline... no
>>> configure: error: the required "readline" library is missing
>>> make[1]: *** [Makefile:11188: configure-sim] Error 1
>>> make[1]: Leaving directory '/home/hp/sim/b'
>>>
>>> The sim dependency on readline is apparently (nominally)
>>> valid as there's a readline call in sim/erc32/sis.c.
>>>
>>> 2022-02-21  Hans-Peter Nilsson  <hp@axis.com>
>>>
>>> 	* Makefile.def (dependencies): Make configure-sim depend on
>>> 	all-readline.
>>> ---
>>>    Makefile.def | 2 +-
>>>    Makefile.in  | 2 +-
>>>    2 files changed, 2 insertions(+), 2 deletions(-)
>>>
>>> diff --git a/Makefile.def b/Makefile.def
>>> index a504192e6d72..8181a7aa97b5 100644
>>> --- a/Makefile.def
>>> +++ b/Makefile.def
>>> @@ -570,7 +570,7 @@ dependencies = { module=all-sim; on=all-intl; };
>>>    dependencies = { module=all-sim; on=all-libiberty; };
>>>    dependencies = { module=all-sim; on=all-bfd; };
>>>    dependencies = { module=all-sim; on=all-opcodes; };
>>> -dependencies = { module=all-sim; on=all-readline; };
>>> +dependencies = { module=configure-sim; on=all-readline; };
>>>      // Other host modules.
>>>    dependencies = { module=all-fastjar; on=all-zlib; };
>>> diff --git a/Makefile.in b/Makefile.in
>>> index 2b77a4706947..843e150dac63 100644
>>> --- a/Makefile.in
>>> +++ b/Makefile.in
>>> @@ -63072,7 +63072,7 @@ install-strip-sid: maybe-install-strip-tcl
>>>    install-sid: maybe-install-tk
>>>    install-strip-sid: maybe-install-strip-tk
>>>    configure-sim: maybe-all-gnulib
>>> -all-sim: maybe-all-readline
>>> +configure-sim: maybe-all-readline
>>>    all-fastjar: maybe-all-build-texinfo
>>>    all-libctf: all-libiberty
>>>    all-stage1-libctf: all-stage1-libiberty
>>
>> I still see this, even with this patch applied. It seems sim requires
>> libreadline-dev regardless.
>>
>> Based on the commit message, I'd expect that to not be the case anymore,
>> right?
> 
> That patch should have fixed occurrences of that particular
> error.  You saying "libreadline-dev" rather than "libreadline"
> makes me think your error is different, kind of like half-way
> picking up libreadline-dev from your system rather than the
> sources.  Either way, sorry, I have no clue ...unless you
> configure with "weird" options like --disable-readline (in which
> case the solution is "don't do that, then").
> 

I ran configure with --enable-targets=all, then 'make all-gdb', which 
fails to link sim in erc32:
/usr/bin/ld: ../../readline/readline/libreadline.a(display.o): in 
function `_rl_move_cursor_relative':
/tmp/mygdb/build/readline/readline/../../../binutils-gdb/readline/readline/display.c:2805: 
undefined reference to `tputs'
[...]


This is because there's none of termcap tinfo curses ncurses installed 
on the machine, which leads to TERMCAP_LIB= <nothing> in sim/Makefile

If termcap is actually required by sim, shouldn't configure fail if it 
is absent?

Thanks,

Christophe


> brgds, H-P