[PATCH] edk2-build.sh: Use CROSS_COMPILE if it's set externally

[PATCH] edk2-build.sh: Use CROSS_COMPILE if it's set externally

[Alan Ott]

---
 edk2-build.sh | 8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/edk2-build.sh b/edk2-build.sh
index 60da4df..fb0cb84 100755
--- a/edk2-build.sh
+++ b/edk2-build.sh
@@ -73,11 +73,13 @@ function do_build
 	    import_openssl
 	fi
 
-	set_cross_compile
-	CROSS_COMPILE="$TEMP_CROSS_COMPILE"
+	if [ -z $CROSS_COMPILE ]; then
+		set_cross_compile
+		CROSS_COMPILE="$TEMP_CROSS_COMPILE"
+	fi
 
 	echo "Building $PLATFORM_NAME - $PLATFORM_ARCH"
-	echo "CROSS_COMPILE=\"$TEMP_CROSS_COMPILE\""
+	echo "CROSS_COMPILE=\"$CROSS_COMPILE\""
 	echo "$board"_BUILDFLAGS="'$PLATFORM_BUILDFLAGS'"
 
 	if [ "$TARGETS" == "" ]; then
-- 
2.9.3

Re: [PATCH] edk2-build.sh: Use CROSS_COMPILE if it's set externally

[Leif Lindholm]

(Adding linaro-uefi, since this is not official tooling.)

On Fri, Aug 18, 2017 at 07:29:59PM -0400, Alan Ott wrote:
> ---
>  edk2-build.sh | 8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/edk2-build.sh b/edk2-build.sh
> index 60da4df..fb0cb84 100755
> --- a/edk2-build.sh
> +++ b/edk2-build.sh
> @@ -73,11 +73,13 @@ function do_build
>  	    import_openssl
>  	fi
>  
> -	set_cross_compile
> -	CROSS_COMPILE="$TEMP_CROSS_COMPILE"
> +	if [ -z $CROSS_COMPILE ]; then
> +		set_cross_compile
> +		CROSS_COMPILE="$TEMP_CROSS_COMPILE"
> +	fi
>  
>  	echo "Building $PLATFORM_NAME - $PLATFORM_ARCH"
> -	echo "CROSS_COMPILE=\"$TEMP_CROSS_COMPILE\""
> +	echo "CROSS_COMPILE=\"$CROSS_COMPILE\""
>  	echo "$board"_BUILDFLAGS="'$PLATFORM_BUILDFLAGS'"
>  
>  	if [ "$TARGETS" == "" ]; then

Sorry, can't accept this one.

I use this script to build multiple platforms across multiple
architectures in one go, and this change breaks that.

But also, do you really need it? Cross compilers accessible on the
PATH should be automatically detected.

If you do need it, because you're using some non-standard toolchain
(such as aarch64-none-eabi), I would like to see something like what
exists (in semi-broken form) in uefi-build.sh: CROSS_COMPILE_32 vs
CROSS_COMPILE_64. However, in order to be useful, that would need to
be extended to do a per-architecture override:
CROSS_COMPILE_AARCH64
CROSS_COMPILE_ARM
CROSS_COMPILE_IA32
CROSS_COMPILE_X64

Regards,

Leif

Re: [PATCH] edk2-build.sh: Use CROSS_COMPILE if it's set externally

[Alan Ott]

On 08/19/2017 06:37 AM, Leif Lindholm wrote:
> (Adding linaro-uefi, since this is not official tooling.)

Ok, no problem.

> On Fri, Aug 18, 2017 at 07:29:59PM -0400, Alan Ott wrote:
>> ---
>>   edk2-build.sh | 8 +++++---
>>   1 file changed, 5 insertions(+), 3 deletions(-)
>>
>> diff --git a/edk2-build.sh b/edk2-build.sh
>> index 60da4df..fb0cb84 100755
>> --- a/edk2-build.sh
>> +++ b/edk2-build.sh
>> @@ -73,11 +73,13 @@ function do_build
>>   	    import_openssl
>>   	fi
>>   
>> -	set_cross_compile
>> -	CROSS_COMPILE="$TEMP_CROSS_COMPILE"
>> +	if [ -z $CROSS_COMPILE ]; then
>> +		set_cross_compile
>> +		CROSS_COMPILE="$TEMP_CROSS_COMPILE"
>> +	fi
>>   
>>   	echo "Building $PLATFORM_NAME - $PLATFORM_ARCH"
>> -	echo "CROSS_COMPILE=\"$TEMP_CROSS_COMPILE\""
>> +	echo "CROSS_COMPILE=\"$CROSS_COMPILE\""
>>   	echo "$board"_BUILDFLAGS="'$PLATFORM_BUILDFLAGS'"
>>   
>>   	if [ "$TARGETS" == "" ]; then
> Sorry, can't accept this one.
>
> I use this script to build multiple platforms across multiple
> architectures in one go, and this change breaks that.

I see now that it's in do_build() which is called for each platform 
built, so yes, my patch won't work. Sorry :(

> But also, do you really need it? Cross compilers accessible on the
> PATH should be automatically detected.

Yes, I can do this.[1]

> If you do need it, because you're using some non-standard toolchain
> (such as aarch64-none-eabi), I would like to see something like what
> exists (in semi-broken form) in uefi-build.sh: CROSS_COMPILE_32 vs
> CROSS_COMPILE_64. However, in order to be useful, that would need to
> be extended to do a per-architecture override:
> CROSS_COMPILE_AARCH64
> CROSS_COMPILE_ARM
> CROSS_COMPILE_IA32
> CROSS_COMPILE_X64

Yes, I was incorrectly thinking it was intended to be similar to 
uefi-build.sh and didn't look closely enough.

Sorry for the extra noise,

Alan.

[1] For what it's worth I've gotten into the habit of specifying the 
full path in CROSS_COMPILE when cross compiling things which build this 
way (kernels, u-boots, etc). It makes typos fail early and operates 
independently of my actual PATH, preventing a mistyped or missing path 
entry from causing it to use a compiler down the list in the system or 
user path.

Re: [PATCH] edk2-build.sh: Use CROSS_COMPILE if it's set externally

[Leif Lindholm]

On Sat, Aug 19, 2017 at 09:49:25AM -0400, Alan Ott wrote:
> On 08/19/2017 06:37 AM, Leif Lindholm wrote:
> > (Adding linaro-uefi, since this is not official tooling.)
> 
> Ok, no problem.
> 
> > On Fri, Aug 18, 2017 at 07:29:59PM -0400, Alan Ott wrote:
> > > ---
> > >   edk2-build.sh | 8 +++++---
> > >   1 file changed, 5 insertions(+), 3 deletions(-)
> > > 
> > > diff --git a/edk2-build.sh b/edk2-build.sh
> > > index 60da4df..fb0cb84 100755
> > > --- a/edk2-build.sh
> > > +++ b/edk2-build.sh
> > > @@ -73,11 +73,13 @@ function do_build
> > >   	    import_openssl
> > >   	fi
> > > -	set_cross_compile
> > > -	CROSS_COMPILE="$TEMP_CROSS_COMPILE"
> > > +	if [ -z $CROSS_COMPILE ]; then
> > > +		set_cross_compile
> > > +		CROSS_COMPILE="$TEMP_CROSS_COMPILE"
> > > +	fi
> > >   	echo "Building $PLATFORM_NAME - $PLATFORM_ARCH"
> > > -	echo "CROSS_COMPILE=\"$TEMP_CROSS_COMPILE\""
> > > +	echo "CROSS_COMPILE=\"$CROSS_COMPILE\""
> > >   	echo "$board"_BUILDFLAGS="'$PLATFORM_BUILDFLAGS'"
> > >   	if [ "$TARGETS" == "" ]; then
> > Sorry, can't accept this one.
> > 
> > I use this script to build multiple platforms across multiple
> > architectures in one go, and this change breaks that.
> 
> I see now that it's in do_build() which is called for each platform built,
> so yes, my patch won't work. Sorry :(
> 
> > But also, do you really need it? Cross compilers accessible on the
> > PATH should be automatically detected.
> 
> Yes, I can do this.[1]
> 
> > If you do need it, because you're using some non-standard toolchain
> > (such as aarch64-none-eabi), I would like to see something like what
> > exists (in semi-broken form) in uefi-build.sh: CROSS_COMPILE_32 vs
> > CROSS_COMPILE_64. However, in order to be useful, that would need to
> > be extended to do a per-architecture override:
> > CROSS_COMPILE_AARCH64
> > CROSS_COMPILE_ARM
> > CROSS_COMPILE_IA32
> > CROSS_COMPILE_X64
> 
> Yes, I was incorrectly thinking it was intended to be similar to
> uefi-build.sh and didn't look closely enough.
> 
> Sorry for the extra noise,
> 
> Alan.
> 
> [1] For what it's worth I've gotten into the habit of specifying the full
> path in CROSS_COMPILE when cross compiling things which build this way
> (kernels, u-boots, etc). It makes typos fail early and operates
> independently of my actual PATH, preventing a mistyped or missing path entry
> from causing it to use a compiler down the list in the system or user path.

Sure, it's a valid thing to do. Especially if testing across different
versions of toolchains. Just, I tend to do that by flipping a symlink.
If someone was to implement what I suggest above, I would be happy to
take it.

And sorry for mistyping the address to my own mailing list :|

/
    Leif