[libffi,1/4] Use a template to pass $CC and $CXX to DejaGNU

Message ID alpine.LFD.2.21.2004031343310.461@redsun52.ssa.fujisawa.hgst.com
State New
Headers show
Series
  • [libffi,1/4] Use a template to pass $CC and $CXX to DejaGNU
Related show

Commit Message

David Edelsohn via Gcc-patches April 3, 2020, 10:55 p.m.
Use an Autoconf template rather an inline piece of scriptery to set 
DejaGNU's $CC_FOR_TARGET and $CXX_FOR_TARGET variables from $CC and $CXX 
respectively, making it easier to maintain and making it take advantage 
of Automake's dependency and rule generation.  Relocate the generated 
`local.exp' file to within testsuite/ so as to make its regeneration 
rule to actually work, i.e. (in testsuite/Makefile.in):

EXTRA_DEJAGNU_SITE_CONFIG = local.exp
site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)
	[...]
local.exp: $(top_builddir)/config.status $(srcdir)/local.exp.in
	cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@

It wouldn't work if the regeneration rule was placed in the top-level 
Makefile.in, which is what Automake would faithfully do if `local.exp' 
stayed in the top level directory.
---
Hi,

 I think having individual AC_CONFIG_FILES macro invocations for each 
output file or group of files would make this change (and code itself) 
more readable, however it hasn't been done before and I decided not to 
change the style on this occasion.  It may make sense as a follow-up 
clean-up.

  Maciej
---
 Makefile.am            |    3 ---
 configure.ac           |    7 +------
 testsuite/Makefile.am  |    2 +-
 testsuite/local.exp.in |    2 ++
 4 files changed, 4 insertions(+), 10 deletions(-)

libffi-test-cc-for-target-template.diff

Comments

David Edelsohn via Gcc-patches April 6, 2020, 5:58 p.m. | #1
On Fri, 2020-04-03 at 23:55 +0100, Maciej W. Rozycki via Gcc-patches wrote:
> Use an Autoconf template rather an inline piece of scriptery to set 

> DejaGNU's $CC_FOR_TARGET and $CXX_FOR_TARGET variables from $CC and $CXX 

> respectively, making it easier to maintain and making it take advantage 

> of Automake's dependency and rule generation.  Relocate the generated 

> `local.exp' file to within testsuite/ so as to make its regeneration 

> rule to actually work, i.e. (in testsuite/Makefile.in):

> 

> EXTRA_DEJAGNU_SITE_CONFIG = local.exp

> site.exp: Makefile $(EXTRA_DEJAGNU_SITE_CONFIG)

> 	[...]

> local.exp: $(top_builddir)/config.status $(srcdir)/local.exp.in

> 	cd $(top_builddir) && $(SHELL) ./config.status $(subdir)/$@

> 

> It wouldn't work if the regeneration rule was placed in the top-level 

> Makefile.in, which is what Automake would faithfully do if `local.exp' 

> stayed in the top level directory.

> ---

> Hi,

> 

>  I think having individual AC_CONFIG_FILES macro invocations for each 

> output file or group of files would make this change (and code itself) 

> more readable, however it hasn't been done before and I decided not to 

> change the style on this occasion.  It may make sense as a follow-up 

> clean-up.

> 

>   Maciej

> ---

>  Makefile.am            |    3 ---

>  configure.ac           |    7 +------

>  testsuite/Makefile.am  |    2 +-

>  testsuite/local.exp.in |    2 ++

>  4 files changed, 4 insertions(+), 10 deletions(-)

So from the cover letter I got the impression this series was supposed to be
pulling down some bits from upstream libffi into gcc's copy.  But AFAICT that's
not the case.

I don't see anything inherently wrong here.  It was just a bit confusing.  It's
actually follow-on patches where you're cherry picking from the upstream libffi
repository.

OK for the trunk.
jeff
>

Patch

Index: libffi/Makefile.am
===================================================================
--- libffi.orig/Makefile.am
+++ libffi/Makefile.am
@@ -23,9 +23,6 @@  EXTRA_DIST = LICENSE ChangeLog.old					\
 	libtool-ldflags libtool-version configure.host README.md        \
 	libffi.map.in LICENSE-BUILDTOOLS msvc_build make_sunver.pl	
 
-# local.exp is generated by configure
-DISTCLEANFILES = local.exp
-
 # Subdir rules rely on $(FLAGS_TO_PASS)
 FLAGS_TO_PASS = $(AM_MAKEFLAGS)
 
Index: libffi/configure.ac
===================================================================
--- libffi.orig/configure.ac
+++ libffi/configure.ac
@@ -56,11 +56,6 @@  if test "x$GCC" = "xyes"; then
   CFLAGS="$CFLAGS -fexceptions"
 fi
 
-cat > local.exp <<EOF
-set CC_FOR_TARGET "$CC"
-set CXX_FOR_TARGET "$CXX"
-EOF
-
 AM_MAINTAINER_MODE
 
 AC_CHECK_HEADERS(sys/mman.h)
@@ -401,7 +396,7 @@  test -d src || mkdir src
 test -d src/$TARGETDIR || mkdir src/$TARGETDIR
 ], [TARGETDIR="$TARGETDIR"])
 
-AC_CONFIG_FILES(include/Makefile include/ffi.h Makefile testsuite/Makefile man/Makefile doc/Makefile libffi.pc)
+AC_CONFIG_FILES(include/Makefile include/ffi.h Makefile testsuite/Makefile testsuite/local.exp man/Makefile doc/Makefile libffi.pc)
 
 AC_OUTPUT
 
Index: libffi/testsuite/Makefile.am
===================================================================
--- libffi.orig/testsuite/Makefile.am
+++ libffi/testsuite/Makefile.am
@@ -2,7 +2,7 @@ 
 
 AUTOMAKE_OPTIONS = foreign dejagnu
 
-EXTRA_DEJAGNU_SITE_CONFIG=../local.exp
+EXTRA_DEJAGNU_SITE_CONFIG = local.exp
 
 CLEANFILES = *.exe core* *.log *.sum
 
Index: libffi/testsuite/local.exp.in
===================================================================
--- /dev/null
+++ libffi/testsuite/local.exp.in
@@ -0,0 +1,2 @@ 
+set CC_FOR_TARGET "@CC@"
+set CXX_FOR_TARGET "@CXX@"