[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Noam Camus]

When the toolchain is custom we can denote that by setting in the triplet the vendor part.
This is done through configution, where the default is buildroot.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 package/Makefile.in                     |    4 +++-
 toolchain/toolchain-buildroot/Config.in |    4 ++++
 2 files changed, 7 insertions(+), 1 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in index eea7043..38d8416 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -20,8 +20,10 @@ endif
 MAKE1:=$(HOSTMAKE) -j1
 MAKE:=$(HOSTMAKE) $(if $(PARALLEL_JOBS),-j$(PARALLEL_JOBS))
 
+VENDOR:=$(call qstrip,$(BR2_TOOLCHAIN_BUILDROOT_VENDOR))
+
 # Compute GNU_TARGET_NAME
-GNU_TARGET_NAME=$(ARCH)-buildroot-$(TARGET_OS)-$(LIBC)$(ABI)
+GNU_TARGET_NAME=$(ARCH)-$(VENDOR)-$(TARGET_OS)-$(LIBC)$(ABI)
 
 # Blackfin FLAT needs uclinux
 ifeq ($(BR2_bfin)$(BR2_BINFMT_FLAT),yy)
diff --git a/toolchain/toolchain-buildroot/Config.in b/toolchain/toolchain-buildroot/Config.in
index cd88889..d1760dd 100644
--- a/toolchain/toolchain-buildroot/Config.in
+++ b/toolchain/toolchain-buildroot/Config.in
@@ -69,6 +69,10 @@ config BR2_TOOLCHAIN_BUILDROOT_LIBC
 	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_EGLIBC
 	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_GLIBC
 
+config BR2_TOOLCHAIN_BUILDROOT_VENDOR
+	string "custom buildroot toolchain vendor name"
+	default "buildroot"
+
 source "package/uclibc/Config.in"
 
 source "package/binutils/Config.in.host"
--
1.7.1

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Thomas Petazzoni]

Dear Noam Camus,

On Sun, 23 Feb 2014 08:28:21 +0000, Noam Camus wrote:
> When the toolchain is custom we can denote that by setting in the triplet the vendor part.
> This is done through configution, where the default is buildroot.
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>

Is there a compelling reason to allow the configuration of the vendor
part of the tuple? To me, it makes sense that all Buildroot toolchain
have the "buildroot" vendor, so that they can be identified easily.

Also, if by mistake the vendor part of the tuple is defined by the user
as "unknown", they he might get compilation issues when building with
target architecture == host architecture.

See 11017f081fc5b034e680d89eaea729c19f450e01 ('pkg-infra: make sure
cross compiling is enabled when host == target').

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Noam Camus]

Hi Thomas,

Here (EZchip) we change toolchain elements such binutils/gdb kernel (headers) and gcc.
We are using override feature of buildroot to create our custom buildroot toolchain.
One of the GCC changes we carry is that we assume dedicated vendor (ezchip) to control several GCC defaults and enable extra configurations within GCC.

About "unknown" I guess that it can be blocked by checking the value and in some cases cause intentional make error.

I hope this makes sense to you.

Noam

-----Original Message-----
From: Thomas Petazzoni [mailto:thomas.petazzoni at free-electrons.com] 
Sent: Sunday, February 23, 2014 11:47 AM
To: Noam Camus
Cc: buildroot at busybox.net
Subject: Re: [Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

Dear Noam Camus,

On Sun, 23 Feb 2014 08:28:21 +0000, Noam Camus wrote:
> When the toolchain is custom we can denote that by setting in the triplet the vendor part.
> This is done through configution, where the default is buildroot.
> 
> Signed-off-by: Noam Camus <noamc@ezchip.com>

Is there a compelling reason to allow the configuration of the vendor part of the tuple? To me, it makes sense that all Buildroot toolchain have the "buildroot" vendor, so that they can be identified easily.

Also, if by mistake the vendor part of the tuple is defined by the user as "unknown", they he might get compilation issues when building with target architecture == host architecture.

See 11017f081fc5b034e680d89eaea729c19f450e01 ('pkg-infra: make sure cross compiling is enabled when host == target').

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering http://free-electrons.com

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Thomas Petazzoni]

Dear Noam Camus,

(Please do not top post. It would also be nice if you used a mail
client that wrapped lines at a reasonable length.)

On Sun, 23 Feb 2014 11:45:46 +0000, Noam Camus wrote:

> Here (EZchip) we change toolchain elements such binutils/gdb kernel (headers) and gcc.
> We are using override feature of buildroot to create our custom buildroot toolchain.
> One of the GCC changes we carry is that we assume dedicated vendor (ezchip) to control several GCC defaults and enable extra configurations within GCC.

Hum, ok. Is the vendor part of the tuple meant to be used this way?

> About "unknown" I guess that it can be blocked by checking the value and in some cases cause intentional make error.
> 
> I hope this makes sense to you.

Right. I guess we can make this configurable. However, your patch has
some issues:

 * It only creates the option BR2_TOOLCHAIN_BUILDROOT_VENDOR in the
   internal toolchain backend. But GNU_TARGET_NAME is also used with
   the external toolchain backend, which means in the external
   toolchain case we would end up with a GNU_TARGET_NAME having an
   empty vendor part of the tuple.

 * The new Config.in option lacks an help text, and the help text
   should explain in detail how this should be used, and in which
   situations. Also the title of the help text should be just
   "custom toolchain vendor name".

Thanks!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Noam Camus]

>Right. I guess we can make this configurable. However, your patch has some issues:

> * It only creates the option BR2_TOOLCHAIN_BUILDROOT_VENDOR in the
>   internal toolchain backend. But GNU_TARGET_NAME is also used with
>   the external toolchain backend, which means in the external
>   toolchain case we would end up with a GNU_TARGET_NAME having an
>   empty vendor part of the tuple.

> * The new Config.in option lacks an help text, and the help text
>   should explain in detail how this should be used, and in which
>   situations. Also the title of the help text should be just
>   "custom toolchain vendor name".


Below is the revised patch.
Is it OK?
Is it the right way to bring up the corrections?
====================================================

When the toolchain is custom we can denote that by setting in the triplet the vendor part.
This is done through configution, where the default is buildroot.

Signed-off-by: Noam Camus <noamc@ezchip.com>
---
 package/Makefile.in                     |   12 +++++++++++-
 toolchain/toolchain-buildroot/Config.in |   12 ++++++++++++
 2 files changed, 23 insertions(+), 1 deletions(-)

diff --git a/package/Makefile.in b/package/Makefile.in index eea7043..bdd6889 100644
--- a/package/Makefile.in
+++ b/package/Makefile.in
@@ -20,8 +20,18 @@ endif
 MAKE1:=$(HOSTMAKE) -j1
 MAKE:=$(HOSTMAKE) $(if $(PARALLEL_JOBS),-j$(PARALLEL_JOBS))
 
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT_VENDOR),)
+VENDOR:=buildroot
+else
+ifeq ($(BR2_TOOLCHAIN_BUILDROOT_VENDOR),unknown)
+$(error You may not provide 'unknown' as vendor to toolchain tuple) 
+else VENDOR:=$(call qstrip,$(BR2_TOOLCHAIN_BUILDROOT_VENDOR))
+endif
+endif
+
 # Compute GNU_TARGET_NAME
-GNU_TARGET_NAME=$(ARCH)-buildroot-$(TARGET_OS)-$(LIBC)$(ABI)
+GNU_TARGET_NAME=$(ARCH)-$(VENDOR)-$(TARGET_OS)-$(LIBC)$(ABI)
 
 # Blackfin FLAT needs uclinux
 ifeq ($(BR2_bfin)$(BR2_BINFMT_FLAT),yy)
diff --git a/toolchain/toolchain-buildroot/Config.in b/toolchain/toolchain-buildroot/Config.in
index cd88889..216f308 100644
--- a/toolchain/toolchain-buildroot/Config.in
+++ b/toolchain/toolchain-buildroot/Config.in
@@ -69,6 +69,18 @@ config BR2_TOOLCHAIN_BUILDROOT_LIBC
 	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_EGLIBC
 	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_GLIBC
 
+config BR2_TOOLCHAIN_BUILDROOT_VENDOR
+	string "custom toolchain vendor name"
+	default "buildroot"
+	help
+	  This option accepts name for vendor field in toolchain tuple.
+	  The toolchain tuple full format is:
+	  cpu-vendor-kernel-os
+	  You may use it in case of custom toolchain.
+	  e.g. if your gcc depend on this field to make decisions.
+	  NOTE: "unknown" is not allowed since it may be confused
+	  with host toolchain.
+
 source "package/uclibc/Config.in"
 
 source "package/binutils/Config.in.host"
--
1.7.1

Thanks
Noam

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Yann E. MORIN]

Noam, All,

On 2014-02-23 15:41 +0000, Noam Camus spake thusly:
[--SNIP--]
> Below is the revised patch.
> Is it OK?
> Is it the right way to bring up the corrections?

Please just re-send the patch as you did the first time.
Be sure to annotate it as:
    [PATCH v2] blabla

You can do that with:
    git send-email --annotate [...]

[--SNIP--]
> diff --git a/toolchain/toolchain-buildroot/Config.in b/toolchain/toolchain-buildroot/Config.in
> index cd88889..216f308 100644
> --- a/toolchain/toolchain-buildroot/Config.in
> +++ b/toolchain/toolchain-buildroot/Config.in
> @@ -69,6 +69,18 @@ config BR2_TOOLCHAIN_BUILDROOT_LIBC
>  	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_EGLIBC
>  	default "glibc"  if BR2_TOOLCHAIN_BUILDROOT_GLIBC
>  
> +config BR2_TOOLCHAIN_BUILDROOT_VENDOR
> +	string "custom toolchain vendor name"
> +	default "buildroot"
> +	help
> +	  This option accepts name for vendor field in toolchain tuple.
> +	  The toolchain tuple full format is:
> +	  cpu-vendor-kernel-os
> +	  You may use it in case of custom toolchain.
> +	  e.g. if your gcc depend on this field to make decisions.
> +	  NOTE: "unknown" is not allowed since it may be confused
> +	  with host toolchain.

I think Thomas said on IRC that he will have a better wording for the
help text. Wait a little bit for his suggestion, before re-sending.

Thanks!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 223 225 172 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Thomas Petazzoni]

Dear Noam Camus,

On Sun, 23 Feb 2014 15:41:43 +0000, Noam Camus wrote:
> +config BR2_TOOLCHAIN_BUILDROOT_VENDOR
> +	string "custom toolchain vendor name"
> +	default "buildroot"
> +	help
> +	  This option accepts name for vendor field in toolchain tuple.
> +	  The toolchain tuple full format is:
> +	  cpu-vendor-kernel-os
> +	  You may use it in case of custom toolchain.
> +	  e.g. if your gcc depend on this field to make decisions.
> +	  NOTE: "unknown" is not allowed since it may be confused
> +	  with host toolchain.

A better wording IMO:

	This option allows to customize the "vendor" part of the
	toolchain tuple, where the toolchain tuple has the form
	<cpu>-<vendor>-<kernel>-<os>. The default value, "buildroot",
	is fine for most cases, except in very specific situations
	where gcc might make different decisions based on the vendor
	part of the tuple. The value "unknown" is not allowed as the
	cross-compiling toolchain might then be confused with the
	native toolchain when the target and host architecture are
	identical.

	If you're not sure, just leave the default "buildroot" value.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor part

[Noam Camus]

>A better wording IMO:

>        This option allows to customize the "vendor" part of the
>         toolchain tuple, where the toolchain tuple has the form
>         <cpu>-<vendor>-<kernel>-<os>. The default value, "buildroot",
>         is fine for most cases, except in very specific situations
>         where gcc might make different decisions based on the vendor
>         part of the tuple. The value "unknown" is not allowed as the
>         cross-compiling toolchain might then be confused with the
>         native toolchain when the target and host architecture are
>         identical.

>        If you're not sure, just leave the default "buildroot" value.

Thanks 
It is indeed much better.
I will send PATCH V2
________________________________________
From: Thomas Petazzoni [thomas.petazzoni at free-electrons.com]
Sent: Monday, March 03, 2014 11:44 PM
To: Noam Camus
Cc: buildroot at busybox.net
Subject: Re: [Buildroot] [PATCH] toolchain: control GNU_TARGET_NAME vendor      part

Dear Noam Camus,

On Sun, 23 Feb 2014 15:41:43 +0000, Noam Camus wrote:
> +config BR2_TOOLCHAIN_BUILDROOT_VENDOR
> +     string "custom toolchain vendor name"
> +     default "buildroot"
> +     help
> +       This option accepts name for vendor field in toolchain tuple.
> +       The toolchain tuple full format is:
> +       cpu-vendor-kernel-os
> +       You may use it in case of custom toolchain.
> +       e.g. if your gcc depend on this field to make decisions.
> +       NOTE: "unknown" is not allowed since it may be confused
> +       with host toolchain.

A better wording IMO:

        This option allows to customize the "vendor" part of the
        toolchain tuple, where the toolchain tuple has the form
        <cpu>-<vendor>-<kernel>-<os>. The default value, "buildroot",
        is fine for most cases, except in very specific situations
        where gcc might make different decisions based on the vendor
        part of the tuple. The value "unknown" is not allowed as the
        cross-compiling toolchain might then be confused with the
        native toolchain when the target and host architecture are
        identical.

        If you're not sure, just leave the default "buildroot" value.

Thomas
--
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com