[PATCH] sim: better handle builds of primary targets lacking sims

[PATCH] sim: better handle builds of primary targets lacking sims

[Jose E. Marchesi]

When building with a primary target that doesn't feature a simulator,
one would expect for nothing to be done in sim/.  However, a
$(top_builddir)/sim/testsuite directory is created, with a Makefile
containing a rule like:

check-DEJAGNU: site.exp
	echo "Dejagnu-checking in `pwd` directory ..."
	rootme=`pwd`; export rootme; echo rootme = $$rootme; \
	srcdir=`cd ${srcdir}; pwd`; export srcdir ; echo srcdir = $$srcdir; \
	EXPECT=${EXPECT} ; export EXPECT ; echo EXPECT = $$EXPECT; \
	if [ -f $$rootme/../../expect/expect ]; then \
	  TCL_LIBRARY=`cd $$srcdir/../../tcl/library && pwd`; \
	  export TCL_LIBRARY; \
	fi; \
        echo TCL_LIBRARY = $$TCL_LIBRARY; \
	runtest=$(RUNTEST); echo runtest = $$runtest; \
	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
	  $$runtest $(RUNTESTFLAGS); \
	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
	fi

Consequently, when `make check' recurses into sim/testsuite, the above
rule is executed.  Until now, the desired effect (of doing nothing)
was achieved because `runtest --version' fails due to a malformed
site.exp being generated in objdir: it is malformed because the
primary target doesn't configure a $sim_arch. i.e. this was doing the
right thing just by chance.

However, the git version of dejagnu seems to have changed in a way
runtest doesn't try to load site.exp when it gets --version.  The net
effect is that the rule above tries to actually run the tests, failing
miserably.

This little patch makes sim/configure to not recurse into
sim/testsuite if the primary target didn't configure a simulator.

Tested with:
- A simulator target (bpf-unkonwn-none).
- A simulator-less target (x86_64-linux-gnu).
- A simulator-less target and --build-targets=all.

sim/ChangeLog:

2020-09-03  Jose E. Marchesi  <jose.marchesi@oracle.com>

	* configure.ac: Do not configure sim/testsuite nor sim/igen if the
	primary target doesn't have a simulator.
	* configure: Regenerate.
---
 sim/ChangeLog    | 6 ++++++
 sim/configure    | 8 +++++---
 sim/configure.ac | 8 +++++---
 3 files changed, 16 insertions(+), 6 deletions(-)

diff --git a/sim/ChangeLog b/sim/ChangeLog
index 5316e24826..5f5f5b8c75 100644
--- a/sim/ChangeLog
+++ b/sim/ChangeLog
@@ -1,3 +1,9 @@
+2020-09-03  Jose E. Marchesi  <jose.marchesi@oracle.com>
+
+	* configure.ac: Do not configure sim/testsuite nor sim/igen if the
+	primary target doesn't have a simulator.
+	* configure: Regenerate.
+
 2020-08-04  Jose E. Marchesi  <jose.marchesi@oracle.com>
 	    David Faust <david.faust@oracle.com>
 
diff --git a/sim/configure b/sim/configure
index 37a86f435c..34f1a15a4c 100755
--- a/sim/configure
+++ b/sim/configure
@@ -3914,11 +3914,13 @@ subdirs="$subdirs aarch64"
 esac
 
 
-   subdirs="$subdirs testsuite"
+   if test x"${sim_arch}" != x; then
+      subdirs="$subdirs testsuite"
 
-   if test "$sim_igen" = yes; then
-      subdirs="$subdirs igen"
+      if test "$sim_igen" = yes; then
+         subdirs="$subdirs igen"
 
+      fi
    fi
 fi
 
diff --git a/sim/configure.ac b/sim/configure.ac
index 896791e97e..d92ca25d39 100644
--- a/sim/configure.ac
+++ b/sim/configure.ac
@@ -43,9 +43,11 @@ m4_define([SIM_ARCH], [
 ])
 if test "${enable_sim}" != no; then
    sinclude(configure.tgt)
-   AC_CONFIG_SUBDIRS(testsuite)
-   if test "$sim_igen" = yes; then
-      AC_CONFIG_SUBDIRS(igen)
+   if test x"${sim_arch}" != x; then
+      AC_CONFIG_SUBDIRS(testsuite)
+      if test "$sim_igen" = yes; then
+         AC_CONFIG_SUBDIRS(igen)
+      fi
    fi
 fi
 
-- 
2.25.0.2.g232378479e

Re: [PATCH] sim: better handle builds of primary targets lacking sims

[Jose E. Marchesi]

> - A simulator-less target and --build-targets=all.

Thats obviously --enable-targets=all :)

Re: [PATCH] sim: better handle builds of primary targets lacking sims

[Andrew Burgess]

* Jose E. Marchesi via Gdb-patches <gdb-patches@sourceware.org> [2020-09-03 15:37:43 +0200]:

> When building with a primary target that doesn't feature a simulator,
> one would expect for nothing to be done in sim/.  However, a
> $(top_builddir)/sim/testsuite directory is created, with a Makefile
> containing a rule like:
> 
> check-DEJAGNU: site.exp
> 	echo "Dejagnu-checking in `pwd` directory ..."
> 	rootme=`pwd`; export rootme; echo rootme = $$rootme; \
> 	srcdir=`cd ${srcdir}; pwd`; export srcdir ; echo srcdir = $$srcdir; \
> 	EXPECT=${EXPECT} ; export EXPECT ; echo EXPECT = $$EXPECT; \
> 	if [ -f $$rootme/../../expect/expect ]; then \
> 	  TCL_LIBRARY=`cd $$srcdir/../../tcl/library && pwd`; \
> 	  export TCL_LIBRARY; \
> 	fi; \
>         echo TCL_LIBRARY = $$TCL_LIBRARY; \
> 	runtest=$(RUNTEST); echo runtest = $$runtest; \
> 	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
> 	  $$runtest $(RUNTESTFLAGS); \
> 	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
> 	fi
> 
> Consequently, when `make check' recurses into sim/testsuite, the above
> rule is executed.  Until now, the desired effect (of doing nothing)
> was achieved because `runtest --version' fails due to a malformed
> site.exp being generated in objdir: it is malformed because the
> primary target doesn't configure a $sim_arch. i.e. this was doing the
> right thing just by chance.
> 
> However, the git version of dejagnu seems to have changed in a way
> runtest doesn't try to load site.exp when it gets --version.  The net
> effect is that the rule above tries to actually run the tests, failing
> miserably.
> 
> This little patch makes sim/configure to not recurse into
> sim/testsuite if the primary target didn't configure a simulator.
> 
> Tested with:
> - A simulator target (bpf-unkonwn-none).
> - A simulator-less target (x86_64-linux-gnu).
> - A simulator-less target and --build-targets=all.
> 
> sim/ChangeLog:
> 
> 2020-09-03  Jose E. Marchesi  <jose.marchesi@oracle.com>
> 
> 	* configure.ac: Do not configure sim/testsuite nor sim/igen if the
> 	primary target doesn't have a simulator.
> 	* configure: Regenerate.

I make no claim at being a configure expect, but this looks reasonable
to me, so I think go ahead and apply.

Thanks,
Andrew


> ---
>  sim/ChangeLog    | 6 ++++++
>  sim/configure    | 8 +++++---
>  sim/configure.ac | 8 +++++---
>  3 files changed, 16 insertions(+), 6 deletions(-)
> 
> diff --git a/sim/ChangeLog b/sim/ChangeLog
> index 5316e24826..5f5f5b8c75 100644
> --- a/sim/ChangeLog
> +++ b/sim/ChangeLog
> @@ -1,3 +1,9 @@
> +2020-09-03  Jose E. Marchesi  <jose.marchesi@oracle.com>
> +
> +	* configure.ac: Do not configure sim/testsuite nor sim/igen if the
> +	primary target doesn't have a simulator.
> +	* configure: Regenerate.
> +
>  2020-08-04  Jose E. Marchesi  <jose.marchesi@oracle.com>
>  	    David Faust <david.faust@oracle.com>
>  
> diff --git a/sim/configure b/sim/configure
> index 37a86f435c..34f1a15a4c 100755
> --- a/sim/configure
> +++ b/sim/configure
> @@ -3914,11 +3914,13 @@ subdirs="$subdirs aarch64"
>  esac
>  
>  
> -   subdirs="$subdirs testsuite"
> +   if test x"${sim_arch}" != x; then
> +      subdirs="$subdirs testsuite"
>  
> -   if test "$sim_igen" = yes; then
> -      subdirs="$subdirs igen"
> +      if test "$sim_igen" = yes; then
> +         subdirs="$subdirs igen"
>  
> +      fi
>     fi
>  fi
>  
> diff --git a/sim/configure.ac b/sim/configure.ac
> index 896791e97e..d92ca25d39 100644
> --- a/sim/configure.ac
> +++ b/sim/configure.ac
> @@ -43,9 +43,11 @@ m4_define([SIM_ARCH], [
>  ])
>  if test "${enable_sim}" != no; then
>     sinclude(configure.tgt)
> -   AC_CONFIG_SUBDIRS(testsuite)
> -   if test "$sim_igen" = yes; then
> -      AC_CONFIG_SUBDIRS(igen)
> +   if test x"${sim_arch}" != x; then
> +      AC_CONFIG_SUBDIRS(testsuite)
> +      if test "$sim_igen" = yes; then
> +         AC_CONFIG_SUBDIRS(igen)
> +      fi
>     fi
>  fi
>  
> -- 
> 2.25.0.2.g232378479e
> 

Re: [PATCH] sim: better handle builds of primary targets lacking sims

[Jose E. Marchesi]

>> When building with a primary target that doesn't feature a simulator,
>> one would expect for nothing to be done in sim/.  However, a
>> $(top_builddir)/sim/testsuite directory is created, with a Makefile
>> containing a rule like:
>> 
>> check-DEJAGNU: site.exp
>> 	echo "Dejagnu-checking in `pwd` directory ..."
>> 	rootme=`pwd`; export rootme; echo rootme = $$rootme; \
>> 	srcdir=`cd ${srcdir}; pwd`; export srcdir ; echo srcdir = $$srcdir; \
>> 	EXPECT=${EXPECT} ; export EXPECT ; echo EXPECT = $$EXPECT; \
>> 	if [ -f $$rootme/../../expect/expect ]; then \
>> 	  TCL_LIBRARY=`cd $$srcdir/../../tcl/library && pwd`; \
>> 	  export TCL_LIBRARY; \
>> 	fi; \
>>         echo TCL_LIBRARY = $$TCL_LIBRARY; \
>> 	runtest=$(RUNTEST); echo runtest = $$runtest; \
>> 	if $(SHELL) -c "$$runtest --version" > /dev/null 2>&1; then \
>> 	  $$runtest $(RUNTESTFLAGS); \
>> 	else echo "WARNING: could not find \`runtest'" 1>&2; :;\
>> 	fi
>> 
>> Consequently, when `make check' recurses into sim/testsuite, the above
>> rule is executed.  Until now, the desired effect (of doing nothing)
>> was achieved because `runtest --version' fails due to a malformed
>> site.exp being generated in objdir: it is malformed because the
>> primary target doesn't configure a $sim_arch. i.e. this was doing the
>> right thing just by chance.
>> 
>> However, the git version of dejagnu seems to have changed in a way
>> runtest doesn't try to load site.exp when it gets --version.  The net
>> effect is that the rule above tries to actually run the tests, failing
>> miserably.
>> 
>> This little patch makes sim/configure to not recurse into
>> sim/testsuite if the primary target didn't configure a simulator.
>> 
>> Tested with:
>> - A simulator target (bpf-unkonwn-none).
>> - A simulator-less target (x86_64-linux-gnu).
>> - A simulator-less target and --build-targets=all.
>> 
>> sim/ChangeLog:
>> 
>> 2020-09-03  Jose E. Marchesi  <jose.marchesi@oracle.com>
>> 
>> 	* configure.ac: Do not configure sim/testsuite nor sim/igen if the
>> 	primary target doesn't have a simulator.
>> 	* configure: Regenerate.
>
> I make no claim at being a configure expect, but this looks reasonable
> to me, so I think go ahead and apply.

Committed, thanks.