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

[Hans-Peter Nilsson <hp@bitrange.com>]

On Tue, 19 Jul 2022, Christophe Lyon wrote:
>
>
> 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?

I presume s/required by sim/required by readline/ but I'd agree
libreadline should either fail or fallback to a trivial version
with a warning.

Other than that rather obvious statement, I have no input.

brgds, H-P