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

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

[Hans-Peter Nilsson]

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
-- 
2.30.2

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

[Tom Tromey]

>>>>> Hans-Peter Nilsson via Binutils <binutils@sourceware.org> writes:

> Calling on "global maintainers" as per toplevel/MAINTAINERS
> for "Makefile.*".

> Ok to commit?

I think this is fine.

> (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.)

The files are canonically maintained in gcc, so normally we'd check it
in there first.  You probably need separate permission.  I think it's ok
to diverge temporarily.

Tom

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

[Luis Machado]

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?

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

[Hans-Peter Nilsson]

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").

brgds, H-P

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

[Christophe Lyon]

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

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

[Hans-Peter Nilsson]

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