Re: [PATCH] Leverage sysroot for VxWorks

[Olivier Hainque <hainque@adacore.com>]

Hi Rasmus,

> On 20 Dec 2021, at 12:40, Rasmus Villemoes <rasmus.villemoes@prevas.dk> wrote:
> 
> On 10/12/2021 19.24, Olivier Hainque wrote:
> 
>> For the toolchains we build, this is achieved with a few
>> configure options like:
>> 
>>   --with-sysroot
>>   --with-build-sysroot=${WIND_BASE}/target
> 
> So forward-porting our private patches up until
> 
> 7bf710b5116 - libstdc++: Add support for '?' in linker script globs
> 
> (i.e. the commit before this one) went without problems, with no changes
> required in our build scripts. Then when rebasing to this one, now known as
> 
> f3f923e5139 - Leverage sysroot for VxWorks
> 
> the build broke as expected because I'd been using somewhat different
> values of --with(-build)-sysroot. However, changing those configure
> options as indicated above again produced a working toolchain, so this
> is all good.

Great !

>>  --with-specs=%{!sysroot=*:--sysroot=%:getenv(WIND_BASE /target)}
> 
> However, I'm a little confused about the purpose of this one.

> First,
> shouldn't this be '%{!-sysroot=*....}', i.e. with a leading dash, since
> the option is --sysroot ? But whether I use one or the other, it seems
> that the resulting compiler ignores a --sysroot argument; if I
> explicitly unexport WIND_BASE and manually add a --sysroot argument that
> should have the same effect as above, gcc fails with WIND_BASE not defined:

> The specs syntax doesn't explicitly list the negative form of %{S*:X},
> and I can't find any in-tree examples (though it's rather hard to grep
> for), so I don't even know if this is supposed to work.

I think it is, and it is used for example in:

  config/i386/i386.h:  {"cpu", "%{!mtune=*:%{!mcpu=*:%{!march=*:-mtune=%(VALUE)}}}" },  \

> It's not a big deal, we can just continue to define and export
> WIND_BASE, but it's probably best for long-term maintenance if our build
> scripts and configure options are aligned with what you do on your end,
> so I would just like to understand this fully.

Sure, appreciated.

The purpose was indeed to also prevent producing the default option
if one is there already, and you are right that the form above doesn't
work as expected on this account.

I had made a couple of basic tests and thought it did, but must have
made a mistake. Might have been skewed by an implicit conviction.

The reason for which it doesn't work is the do_save = false in
driver_handle_option for --sysroot:

   case OPT__sysroot_:
      target_system_root = arg;
      target_system_root_changed = 1;
      do_save = false;
      break;


Changing that has the intended effect on our use case,
with an additional dash indeed.

I'll test further and propose the change if it doesn't raise
unexpected problems. We could also even consider using DRIVER_SELF_SPECS
then.

Thanks for your feedback,

Olivier