Re: Fallout (mostly runtime library-related) from GCC version number change (take two)

[Ian Lance Taylor <ian at airs dot com>]

Zack Weinberg <zack@codesourcery.com> writes:

> *Please* read and comment.

OK.

> ===================================================================
> Index: fixincludes/Makefile.in
> --- fixincludes/Makefile.in	23 Feb 2005 22:30:01 -0000	1.8
> +++ fixincludes/Makefile.in	17 Mar 2005 22:55:18 -0000
> @@ -49,10 +49,10 @@ target = @target@
>  target_noncanonical:=@target_noncanonical@
>  
>  # The version of GCC in this tree
> -gcc_version=@gcc_version@
> +gcc_version = $(shell cat $(srcdir)/../gcc/BASE-VER)

As I may have seen you mention somewhere, this should use :=, not =.

>  # Directory in which the compiler finds libraries etc.
> -libsubdir = $(libdir)/gcc/$(target_noncanonical)/$(version)
> +libsubdir = $(libdir)/gcc/$(target_noncanonical)/$(gcc_version)

Interesting.  This looks like a change in behaviour, but the old
behaviour does look wrong.

> +mkheaders: mkheaders.almost $(srcdir)/../gcc/BASE-VER
> +	sed -e 's/@gcc_version@/$(shell cat $(srcdir)/../gcc/BASE-VER)/' \
> +		< $< > $@T
> +	mv -f $@T $@

Why not use $(gcc_version) here, rather than invoking $(shell) again?

> ===================================================================
> Index: gcc/Makefile.in
> --- gcc/Makefile.in	16 Mar 2005 06:03:18 -0000	1.1454
> +++ gcc/Makefile.in	17 Mar 2005 22:55:22 -0000
> @@ -3424,6 +3424,13 @@ install-include-dir: installdirs
>  	mkdir $(DESTDIR)$(libsubdir)/include
>  	-chmod a+rx $(DESTDIR)$(libsubdir)/include
>  
> +# Create or recreate the install-tools include file directory.
> +itoolsdir = $(libexecsubdir)/install-tools
> +itoolsdatadir = $(libsubdir)/install-tools
> +install-itoolsdirs: installdirs
> +	$(mkinstalldirs) $(DESTDIR)$(itoolsdatadir)/include
> +	$(mkinstalldirs) $(DESTDIR)$(itoolsdir)
> +
>  # Install the include directory using tar.
>  install-headers-tar: stmp-int-hdrs $(STMP_FIXPROTO) install-include-dir
>  # We use `pwd`/include instead of just include to problems with CDPATH
> @@ -3446,10 +3453,8 @@ install-headers-cpio: stmp-int-hdrs $(ST
>  install-headers-cp: stmp-int-hdrs $(STMP_FIXPROTO) install-include-dir
>  	cp -p -r include $(DESTDIR)$(libsubdir)
>  
> -itoolsdir = $(libexecsubdir)/install-tools
> -itoolsdatadir = $(libsubdir)/install-tools
>  # Install supporting files for fixincludes to be run later.
> -install-mkheaders: stmp-int-hdrs $(STMP_FIXPROTO) install-include-dir \
> +install-mkheaders: stmp-int-hdrs $(STMP_FIXPROTO) install-itoolsdirs \
>    macro_list xlimits.h
>  	for file in $(USER_H); do \
>  	  realfile=`echo $$file | sed -e 's|.*/\([^/]*\)$$|\1|'`; \

Looks like you lost the dependency on install-include-dir here.  Was
that intentional?

> ===================================================================
> Index: libffi/configure.ac
> --- libffi/configure.ac	2 Dec 2004 11:04:21 -0000	1.9
> +++ libffi/configure.ac	17 Mar 2005 22:56:13 -0000
> @@ -226,10 +226,10 @@ esac
>  AC_SUBST(toolexecdir)
>  AC_SUBST(toolexeclibdir)
>  
> -TL_AC_GCC_VERSION([$srcdir/..])
> -
>  #Figure out where generated headers like ffitarget.h get installed.
> -tool_include_dir='$(libdir)/gcc/$(target_alias)/'${gcc_version}/include
> +gcc_version='$(shell cat $(srcdir)/../gcc/BASE-VER)'
> +tool_include_dir='$(libdir)/gcc/$(target_alias)/$(gcc_version)/include'
> +AC_SUBST(gcc_version)
>  AC_SUBST(tool_include_dir)

This is going to lead to multiple evaluation of gcc_version.  automake
is going to initialize the variables using =, so they will be
rexpanded every time.  I would be particularly worried about the usage
in subdirectories of libffi; I doubt srcdir will be correct.  It would
seem safer to use top_srcdir instead.

Note that there is now no reason to substitute tool_include_dir.  It
can be determined entirely in terms of make variables, and you might
as well copy the definition into Makefile.  Then that value could be
set using :=, avoiding the extra evaluations.

> Index: libgfortran/configure.ac
> --- libgfortran/configure.ac	12 Dec 2004 08:59:02 -0000	1.20
> +++ libgfortran/configure.ac	17 Mar 2005 22:56:14 -0000
> @@ -68,7 +68,10 @@ else
>    LIBGFOR_IS_NATIVE=true
>  fi
>  
> -TL_AC_GCC_VERSION([$srcdir/..])
> +# Define this here so Automake will put it into all subdirectory
> +# Makefiles.
> +gcc_version='$(shell cat $(srcdir)/../gcc/BASE-VER)'
> +AC_SUBST(gcc_version)
>  
>  # Calculate toolexeclibdir
>  # Also toolexecdir, though it's only used in toolexeclibdir
> @@ -78,7 +81,7 @@ case ${version_specific_libs} in
>      # and header files if --enable-version-specific-runtime-libs option
>      # is selected.
>      toolexecdir='$(libdir)/gcc/$(target_alias)'
> -    toolexeclibdir='$(toolexecdir)/'${gcc_version}'$(MULTISUBDIR)'
> +    toolexeclibdir='$(toolexecdir)/$(gcc_version)$(MULTISUBDIR)'
>      ;;
>    no)
>      if test -n "$with_cross_host" &&

I suppose you could get around the multiple evaluation of gcc_version
here by adding something like
    my_gcc_version := $(gcc_version)
in Makefile.am, and then using my_gcc_version instead of gcc_version
in the setting to toolexeclibdir.  Here again top_srcdir might be
safer in the setting of gcc_version, although since there don't seem
to be any subdirectories it may not matter.

I have to go now, so I won't continue any further for now.  Hopes this
helps.

Ian