Re: [PATCH] Move gdbserver to top level

Pedro Alves
2020-01-25T16:50:23Z

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