re-expand CXX_FOR_TARGET after libstdc++-v3 is configured

I got a report from RTH and Aldy that libitm (in the
transactional-memory branch) would fail to build at first, but once you
retried, it would succeed.  What's new about libitm is that it expects
CXX to be a functional C++ compiler, with usual C++ headers in place.
Other libraries that require a C++ compiler don't seem to depend on the
C++ headers.

It turns out the problem is that CXX_FOR_TARGET is shell-evaluated and
passed down to sub-makes before libstdc++-v3 is configured, but it can
only get the right -I flags after libstdc++-v3 is configured.  Thus the
first build fails, but the subsequent one overrides the incorrect CXX
settings passed to libitm and it works out in the end.

This patch changes our handling of CXX_FOR_TARGET in two ways:

- before libstdc++-v3 is configured, we add an invalid flag
  (-funconfigured-libstdc++-v3) to CXX_FOR_TARGET, to make sure it isn't
  used (libstdc++-v3 and libjava use RAW_CXX_FOR_TARGET)

- we do *not* pass CXX_FOR_TARGET to a sub-make if its shell expansion
  contains -funconfigured-, so that the Makefile definition prevails and
  gets expanded at every use within the sub-make.  I considered a
  cheaper make test rather than a $(shell) expansion, but it doesn't
  have the desired effect: only the shell knows how to evaluate the
  backticks in CXX_FOR_TARGET, so we end up not ever passing down an
  expansion, because the macro definition contains the -funconfigured-*
  word within backticks.  I guess it would be possible to make it a make
  test, testing for the backticks too, but it's not as robust, and
  probably not worth it.


Here's the patch.  I'll install it soon if there aren't any objections.
Tested on x86_64-linux-gnu.

Hi Alexandre,

* Alexandre Oliva wrote on Sat, Nov 13, 2010 at 08:19:35PM CET:
> I got a report from RTH and Aldy that libitm (in the
> transactional-memory branch) would fail to build at first, but once you
> retried, it would succeed.  What's new about libitm is that it expects
> CXX to be a functional C++ compiler, with usual C++ headers in place.
> Other libraries that require a C++ compiler don't seem to depend on the
> C++ headers.
> 
> It turns out the problem is that CXX_FOR_TARGET is shell-evaluated and
> passed down to sub-makes before libstdc++-v3 is configured, but it can
> only get the right -I flags after libstdc++-v3 is configured.  Thus the
> first build fails, but the subsequent one overrides the incorrect CXX
> settings passed to libitm and it works out in the end.
> 
> This patch changes our handling of CXX_FOR_TARGET in two ways:
> 
> - before libstdc++-v3 is configured, we add an invalid flag
>   (-funconfigured-libstdc++-v3) to CXX_FOR_TARGET, to make sure it isn't
>   used (libstdc++-v3 and libjava use RAW_CXX_FOR_TARGET)

I'm not really sure I understand the problem completely, but to me it
sounds like
- there is a dependency problem at the toplevel (missing dependency of
  configure-target-libitm on all-target-libstdc++-v3 for example), and
- variables that, at the toplevel, evaluate differently upon a make
  rerun will cause config.cache conflicts in the lower levels.

I'm not claiming that your patch is wrong, but for proper operation,
AFAICS there should be a static stateless map from configure-target-* to
the value of variables passed to that configure script.

> - we do *not* pass CXX_FOR_TARGET to a sub-make if its shell expansion
>   contains -funconfigured-, so that the Makefile definition prevails and
>   gets expanded at every use within the sub-make.  I considered a
>   cheaper make test rather than a $(shell) expansion, but it doesn't
>   have the desired effect: only the shell knows how to evaluate the
>   backticks in CXX_FOR_TARGET, so we end up not ever passing down an
>   expansion, because the macro definition contains the -funconfigured-*
>   word within backticks.  I guess it would be possible to make it a make
>   test, testing for the backticks too, but it's not as robust, and
>   probably not worth it.

Wasn't toplevel supposed to be buildable with non-GNU make as well
(given that binutils and some other parts of src aim to be buildable
without GNU make)?  Again, it might well be that it still is even with
this $(shell ...) expansion as long as make doesn't actually have to do
the expansion if not building GCC, but it would be good to try it out
(several GNU/Linux distros ship some BSD make under the names pmake or
freebsd-make, if you need an easy way to test).

Thanks,
Ralf

On Nov 14, 2010, Ralf Wildenhues <Ralf.Wildenhues@gmx.de> wrote:

> - there is a dependency problem at the toplevel (missing dependency of
>   configure-target-libitm on all-target-libstdc++-v3 for example), and

No, no, totally different problem.  The deps are just fine.  The problem
is that the rule for “all” calls a sub-make, passing it a CXX_FOR_TARGET
evaluated *before* libstdc++-v3 is configured, so the solution is to
delay the expansion of CXX_FOR_TARGET.

> - variables that, at the toplevel, evaluate differently upon a make
>   rerun will cause config.cache conflicts in the lower levels.

That was true before the patch, but it wasn't the issue.

> Wasn't toplevel supposed to be buildable with non-GNU make as well

Yep, except when building GCC.  That's why the $(shell) bit is limited
to an @if conditional that's only passed on to make when we're building
GCC.

On 11/13/2010 08:19 PM, Alexandre Oliva wrote:
> The problem is that the rule for “all” calls a sub-make, passing it a
> CXX_FOR_TARGET evaluated *before* libstdc++-v3 is configured, so the
> solution is to delay the expansion of CXX_FOR_TARGET.

Maybe the problem is this:

# We leave this in just in case, but it is not needed anymore.
RECURSE_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS)
MAKEOVERRIDES=

We could try removing both of these and see what happens...

Paolo

On Nov 14, 2010, Paolo Bonzini <bonzini@gnu.org> wrote:

> On 11/13/2010 08:19 PM, Alexandre Oliva wrote:
>> The problem is that the rule for “all” calls a sub-make, passing it a
>> CXX_FOR_TARGET evaluated *before* libstdc++-v3 is configured, so the
>> solution is to delay the expansion of CXX_FOR_TARGET.

> Maybe the problem is this:

> # We leave this in just in case, but it is not needed anymore.
> RECURSE_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS)
> MAKEOVERRIDES=

> We could try removing both of these and see what happens...

That won't be enough.  We explicitly pass down TARGET_FLAGS_TO_PASS
(including CXX_FOR_TARGET) when recursing for all-host all-target.  And
it's not like it would be right to refrain from overriding a number of
variables, CXX_FOR_TARGET included: we want to use different compilers
and options for different stages, rather than taking whatever overrider
the user might have passed for stage1 or so.

On 11/17/2010 07:55 AM, Alexandre Oliva wrote:
> On Nov 14, 2010, Paolo Bonzini<bonzini@gnu.org>  wrote:
>
>> On 11/13/2010 08:19 PM, Alexandre Oliva wrote:
>>> The problem is that the rule for “all” calls a sub-make, passing it a
>>> CXX_FOR_TARGET evaluated *before* libstdc++-v3 is configured, so the
>>> solution is to delay the expansion of CXX_FOR_TARGET.
>
>> Maybe the problem is this:
>
>> # We leave this in just in case, but it is not needed anymore.
>> RECURSE_FLAGS_TO_PASS = $(BASE_FLAGS_TO_PASS)
>> MAKEOVERRIDES=
>
>> We could try removing both of these and see what happens...
>
> That won't be enough.  We explicitly pass down TARGET_FLAGS_TO_PASS
> (including CXX_FOR_TARGET) when recursing for all-host all-target.  And
> it's not like it would be right to refrain from overriding a number of
> variables, CXX_FOR_TARGET included: we want to use different compilers
> and options for different stages, rather than taking whatever overrider
> the user might have passed for stage1 or so.

I see.

Paolo