Re: [PATCH] Move gdbserver to top level

[Pedro Alves <palves@redhat.com>]

On 1/24/20 10:54 PM, Tom Tromey wrote:
>>>>>> "Pedro" == Pedro Alves <palves@redhat.com> writes:
> 
> Pedro> This should really be done by sourcing a file in gdbserver/
> Pedro> If we do that, then the info about whether a system supports
> Pedro> gdbserver is contained under gdbserver/, and does not
> Pedro> require syncing with gcc whenever is changes.
> 
> Good idea, I've made this change.  Here's an updated patch.
> 
> As before, I've removed the generated files.
> 
> If you want to test it, it is in submit/move-gdbserver-to-top on my
> github.

Thanks!  I poked at it a bit.  I think this looks good, though I
have a comment on the dependencies below.

I think this should have a NEWS entry, given that it affects
people building standalone gdbserver today.  Do you have
documentation changes queued?

>  2020-01-19  Simon Marchi  <simon.marchi@polymtl.ca>
>  
>  	* remote-sim.c (gdbsim_target::wait): Return
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 805f2e3ac43..5b8a4efbdf6 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -41,7 +41,7 @@ config.guess; config.sub; readline/support/config.{sub,guess}
>  depcomp; mkinstalldirs
>          Send bug reports and patches to bug-automake@gnu.org.
>  
> -gdb/; gdbsupport/; gnulib/; readline/; sim/; GDB's part of include/
> +gdb/; gdbserver/; gdbsupport/; gnulib/; readline/; sim/; GDB's part of include/
>  	GDB: http://www.gnu.org/software/gdb/
>  	Patches to gdb-patches@sourceware.org.
>  	See also gdb/MAINTAINERS and sim/MAINTAINERS.
> diff --git a/Makefile.def b/Makefile.def
> index 253eb45ef14..de35083d81e 100644
> --- a/Makefile.def
> +++ b/Makefile.def
> @@ -114,6 +114,7 @@ host_modules= { module= zlib; no_install=true; no_check=true;
>  	        extra_configure_flags='@extra_host_zlib_configure_flags@';};
>  host_modules= { module= gnulib; };
>  host_modules= { module= gdbsupport; };
> +host_modules= { module= gdbserver; };
>  host_modules= { module= gdb; };
>  host_modules= { module= expect; };
>  host_modules= { module= guile; };
> @@ -409,6 +410,12 @@ dependencies = { module=all-gdb; on=all-libdecnumber; };
>  dependencies = { module=all-gdb; on=all-libtermcap; };
>  dependencies = { module=all-gdb; on=all-libctf; };
>  
> +// Host modules specific to gdbserver.
> +dependencies = { module=configure-gdbserver; on=all-intl; };

This surprised me, as I don't think gdbserver actually depends on
intl currently?  Might still be a good idea to actually enable it,
but I was surprised from an "as pure as possible" perspective.

> +// Depend on all-libiconv so that configure checks for iconv
> +// functions will work.
> +dependencies = { module=configure-gdbserver; on=all-libiconv; };
> +

Likewise curious.  Can you clarify where the configure checks
come from?

Locally I removed the intl and iconv dependencies and the
resulting gdbserver (w/ make all-gdbserver) was identical byte
for byte for me.

Thanks,
Pedro Alves