Re: [PATCH] sim: unify target->subdir handling for default tests

[Doug Evans <dje at google dot com>]

On Wed, Apr 21, 2010 at 2:12 PM, Mike Frysinger <vapier@gentoo.org> wrote:
> The testsuite subdir has a note about unifying the target->subdir logic,
> so do just that. ?The end goal here is to have `make check` work out of
> the box without having to delve into dejagnu internals.
>
> The target-specific logic is split out of the top level configure.ac file
> and into a dedicated configure.tgt similar to other subprojects (gdb and
> ld and etc...) with the difference that this file has to be included at
> the m4 level instead of the shell level. ?This is necessary only because
> autoconf requires AC_CONFIG_SUBDIRS be given a string literal and not a
> variable value.
>
> Then the toplevel and the testsuite configure files pull this in, the sim
> subdir gets expanded into testsuite/site.exp, and the default sim run code
> uses this info to set the sim path to the local compiled run file if it
> hasn't already been specified.
>
> Signed-off-by: Mike Frysinger <vapier@gentoo.org>
>
> sim/:
> 2010-04-21 ?Mike Frysinger ?<vapier@gentoo.org>
>
> ? ? ? ?* configure.ac: Target logic moved out to and included from ...
> ? ? ? ?* configure.tgt: ... this new file.
> ? ? ? ?* configure: Regenerated.
>
> sim/testsuite/:
> 2010-04-21 ?Mike Frysinger ?<vapier@gentoo.org>
>
> ? ? ? ?* Makefile.in (arch): Set to @sim_arch@.
> ? ? ? ?* configure.ac: Delete arch logic and include ../configure.tgt.
> ? ? ? ?* configure: Regenerated.
> ? ? ? ?* lib/sim-defs.exp (sim_run): Default sim to ../arch/run.
> ---
> ?sim/configure.ac ? ? ? ? ? ? ? | ?120 +++-------------------------------------
> ?sim/configure.tgt ? ? ? ? ? ? ?| ?114 ++++++++++++++++++++++++++++++++++++++
> ?sim/testsuite/Makefile.in ? ? ?| ? ?2 +-
> ?sim/testsuite/configure.ac ? ? | ? ?8 +--
> ?sim/testsuite/lib/sim-defs.exp | ? ?9 +++
> ?5 files changed, 133 insertions(+), 120 deletions(-)
> ?create mode 100644 sim/configure.tgt
>

This is ok with me with a few qualifications.
I'd leave it for a few days to see if anyone else wants to comment.

I'd add a comment to configure.tgt saying that it's intended to be m4-included.
[It's kinda clear since a few lines in there's an m4 command :-), but
an explicit comment would help, I think.]

How have you tested it?
[I'd test it on at least two targets, if not more.]