Patchwork [ada,build] host/target configuration

login
register
mail settings
Submitter Eric Botcazou
Date May 30, 2013, 7:21 a.m.
Message ID <105381699.29RaUYo0Gd@polaris>
Download mbox | patch
Permalink /patch/247462/
State New
Headers show

Comments

Eric Botcazou - May 30, 2013, 7:21 a.m.
> However, it seems that the first androideabi snippet was dead code.
> Can you delete it in a follow-up?

No, it's not dead code, just broken at the moment, now fixed by:


2013-05-30  Eric Botcazou  <ebotcazou@adacore.com>

	* gcc-interface/Makefile.in (arm% androideabi): Robustify.
Paolo Bonzini - May 30, 2013, 7:27 a.m.
Il 30/05/2013 09:21, Eric Botcazou ha scritto:
>> However, it seems that the first androideabi snippet was dead code.
>> Can you delete it in a follow-up?
> 
> No, it's not dead code, just broken at the moment, now fixed by:
> 
> 
> 2013-05-30  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* gcc-interface/Makefile.in (arm% androideabi): Robustify.
> 
> 

I don't think this fixes it.  The problem is that the second eabi
conditional overrides the first (the one for Android).

Paolo
Eric Botcazou - May 30, 2013, 7:33 a.m.
> I don't think this fixes it.  The problem is that the second eabi
> conditional overrides the first (the one for Android).

Then let's fix the second eabi or swap them, but the first one must stay.
Paolo Bonzini - May 30, 2013, 7:36 a.m.
Il 30/05/2013 09:33, Eric Botcazou ha scritto:
>> I don't think this fixes it.  The problem is that the second eabi
>> > conditional overrides the first (the one for Android).
> Then let's fix the second eabi or swap them, but the first one must stay.

Yes, got it.  Swapping them looks like the right thing to do.

Paolo
Thomas Schwinge - May 31, 2013, 7:10 a.m.
Hi!

On Thu, 30 May 2013 09:21:12 +0200, Eric Botcazou <ebotcazou@adacore.com> wrote:
> > However, it seems that the first androideabi snippet was dead code.
> > Can you delete it in a follow-up?
> 
> No, it's not dead code, just broken at the moment, now fixed by:
> 
> 
> 2013-05-30  Eric Botcazou  <ebotcazou@adacore.com>
> 
> 	* gcc-interface/Makefile.in (arm% androideabi): Robustify.

> -ifeq ($(strip $(filter-out arm% linux-androideabi,$(arch) $(osys)-$(word 4,$(targ)))),)
> +ifeq ($(strip $(filter-out arm% androideabi,$(arch) $(osys))),)

I don't understand this change.  This used to match configurations
arm*-[vendor]-linux-androideabi; now it only matches
arm*-[vendor]-androideabi, which isn't in use (for a Android system is
always based on the Linux kernel, in my understanding).


Grüße,
 Thomas
Eric Botcazou - May 31, 2013, 8:03 a.m.
> I don't understand this change.  This used to match configurations
> arm*-[vendor]-linux-androideabi; now it only matches
> arm*-[vendor]-androideabi, which isn't in use (for a Android system is
> always based on the Linux kernel, in my understanding).

This is meant to match arm-linux-androideabi, which didn't match anymore after 
the initial problematic targ change but needs to.
Paolo Bonzini - May 31, 2013, 10:46 a.m.
Il 31/05/2013 10:03, Eric Botcazou ha scritto:
>> I don't understand this change.  This used to match configurations
>> arm*-[vendor]-linux-androideabi; now it only matches
>> arm*-[vendor]-androideabi, which isn't in use (for a Android system is
>> always based on the Linux kernel, in my understanding).
> 
> This is meant to match arm-linux-androideabi, which didn't match anymore after 
> the initial problematic targ change but needs to.
> 

Do you mean arm-linux-androideabi, or arm-none-linux-androideabi?

Paolo
Eric Botcazou - May 31, 2013, 11:14 a.m.
> Do you mean arm-linux-androideabi, or arm-none-linux-androideabi?

Thr former, but I guess that we want to support the latter as well.
Thomas Schwinge - May 31, 2013, 11:29 a.m.
Hi!

On Fri, 31 May 2013 13:14:39 +0200, Eric Botcazou <ebotcazou@adacore.com> wrote:
> > Do you mean arm-linux-androideabi, or arm-none-linux-androideabi?
> 
> Thr former, but I guess that we want to support the latter as well.

In my reading, the code supported both these before your recent change,
and now supports neither, as I reported this morning.  With which
configurations exactly have you tested your change?


Grüße,
 Thomas
Eric Botcazou - May 31, 2013, 7:54 p.m.
> In my reading, the code supported both these before your recent change,
> and now supports neither, as I reported this morning.

Did you test it?

> With which configurations exactly have you tested your change?

arm-linux-androideabi
Thomas Schwinge - May 31, 2013, 8:31 p.m.
Hi!

On Fri, 31 May 2013 21:54:55 +0200, Eric Botcazou <ebotcazou@adacore.com> wrote:
> > In my reading, the code supported both these before your recent change,
> > and now supports neither, as I reported this morning.
> 
> Did you test it?

I didn't; now I have, and...

> > With which configurations exactly have you tested your change?
> 
> arm-linux-androideabi

..., you're right that this one works, but it only works by chance:

    targ:=$(subst -, ,$(subst -gnu, ,$(target_alias)))
    arch:=$(word 1,$(targ))
    ifeq ($(words $(targ)),2)
      manu:=
      osys:=$(word 2,$(targ))
    else
      manu:=$(word 2,$(targ))
      osys:=$(word 3,$(targ))
    endif
    
    default:
    	@echo "target_alias = »$(target_alias)«"
    	@echo "targ = »$(targ)«"
    	@echo "arch = »$(arch)«"
    	@echo "manu = »$(manu)«"
    	@echo "osys = »$(osys)«"
    ifeq ($(strip $(filter-out arm% androideabi,$(arch) $(osys))),)
    	@echo matched
    else
    	@echo not matched
    endif

We get:

    $ make target_alias=arm-linux-androideabi
    target_alias = »arm-linux-androideabi«
    targ = »arm linux androideabi«
    arch = »arm«
    manu = »linux«
    osys = »androideabi«
    matched

So, your case works because the manu/osys parsing wrongly detects/assigns
a manufacturer »linux« and an operating system androideabi.  Then, the
following case fails, which is expected to yield identical results, with
"complete triplets" -- which I took for granted in my reasoning about the
Makefile code:

    $ make target_alias=arm-unknown-linux-androideabi
    target_alias = »arm-unknown-linux-androideabi«
    targ = »arm unknown linux androideabi«
    arch = »arm«
    manu = »unknown«
    osys = »linux«
    not matched


My suggested change would make all these work -- however I have not yet
had the time to fully digest your other emails with the reasoning that
you need configure GCC with non-canonical target and target_alias set
differently.


Grüße,
 Thomas
Eric Botcazou - June 2, 2013, 8:44 a.m.
> So, your case works because the manu/osys parsing wrongly detects/assigns
> a manufacturer »linux« and an operating system androideabi.  Then, the
> following case fails, which is expected to yield identical results, with
> "complete triplets" -- which I took for granted in my reasoning about the
> Makefile code:
> 
>     $ make target_alias=arm-unknown-linux-androideabi
>     target_alias = »arm-unknown-linux-androideabi«
>     targ = »arm unknown linux androideabi«
>     arch = »arm«
>     manu = »unknown«
>     osys = »linux«
>     not matched
> 
> 
> My suggested change would make all these work -- however I have not yet
> had the time to fully digest your other emails with the reasoning that
> you need configure GCC with non-canonical target and target_alias set
> differently.

The whole discussion started from wrong premises since, contrary to what the 
ChangeLog says, neither Pascal nor I have nothing to do with the original, 
problematic change (see PR ada/57188 for my take on it).  We all agree that 
the mess should be fixed somehow or other and Olivier is working on it.

Patch

Index: gcc-interface/Makefile.in
===================================================================
--- gcc-interface/Makefile.in	(revision 199343)
+++ gcc-interface/Makefile.in	(working copy)
@@ -995,7 +995,7 @@  ifeq ($(strip $(filter-out mips% wrs vx%
   EXTRA_LIBGNAT_OBJS+=vx_stack_info.o
 endif
 
-ifeq ($(strip $(filter-out arm% linux-androideabi,$(arch) $(osys)-$(word 4,$(targ)))),)
+ifeq ($(strip $(filter-out arm% androideabi,$(arch) $(osys))),)
   LIBGNAT_TARGET_PAIRS = \
   a-intnam.ads<a-intnam-linux.ads \
   s-inmaop.adb<s-inmaop-posix.adb \