Re: [gentoo-dev] [PATCH] kernel-2.eclass: Make xmakeopts an array for spaces in toolchain vars

[Joshua Kinard]

On 1/24/2023 18:40, James Le Cuirot wrote:
> On Mon, 2023-01-23 at 11:20 -0500, Joshua Kinard wrote:
>> On 1/21/2023 06:03, James Le Cuirot wrote:
>>> Variables like CC can have spaces for additional arguments. This is
>>> particularly useful for reliably setting the sysroot.
>>>
>>> Signed-off-by: James Le Cuirot <chewi@g.o>
>>> ---
>>>    eclass/kernel-2.eclass | 21 +++++++++++++++------
>>>    1 file changed, 15 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/eclass/kernel-2.eclass b/eclass/kernel-2.eclass
>>> index 873d4a204669..f7fcf15743f0 100644
>>> --- a/eclass/kernel-2.eclass
>>> +++ b/eclass/kernel-2.eclass
>>> @@ -1,4 +1,4 @@
>>> -# Copyright 1999-2022 Gentoo Authors
>>> +# Copyright 1999-2023 Gentoo Authors
>>>    # Distributed under the terms of the GNU General Public License v2
>>>
>>>    # @ECLASS: kernel-2.eclass
>>> @@ -756,13 +756,22 @@ env_setup_xmakeopts() {
>>>
>>>    	# When cross-compiling, we need to set the ARCH/CROSS_COMPILE
>>>    	# variables properly or bad things happen !
>>> -	xmakeopts="ARCH=${KARCH}"
>>> +	xmakeopts=( ARCH="${KARCH}" )
>>>    	if [[ ${CTARGET} != ${CHOST} ]] && ! cross_pre_c_headers; then
>>> -		xmakeopts="${xmakeopts} CROSS_COMPILE=${CTARGET}-"
>>> +		xmakeopts+=( CROSS_COMPILE="${CTARGET}-" )
>>>    	elif type -p ${CHOST}-ar >/dev/null; then
>>> -		xmakeopts="${xmakeopts} CROSS_COMPILE=${CHOST}-"
>>> +		xmakeopts+=( CROSS_COMPILE="${CHOST}-" )
>>>    	fi
>>> -	xmakeopts="${xmakeopts} HOSTCC=$(tc-getBUILD_CC) CC=$(tc-getCC) LD=$(tc-getLD) AR=$(tc-getAR) NM=$(tc-getNM) OBJCOPY=$(tc-getOBJCOPY) READELF=$(tc-getREADELF) STRIP=$(tc-getSTRIP)"
>>> +	xmakeopts+=(
>>> +		HOSTCC="$(tc-getBUILD_CC)"
>>> +		CC="$(tc-getCC)"
>>> +		LD="$(tc-getLD)"
>>> +		AR="$(tc-getAR)"
>>> +		NM="$(tc-getNM)"
>>> +		OBJCOPY="$(tc-getOBJCOPY)"
>>> +		READELF="$(tc-getREADELF)"
>>> +		STRIP="$(tc-getSTRIP)"
>>> +	)
>>>    	export xmakeopts
>>>    }
>>>
>>> @@ -850,7 +859,7 @@ install_headers() {
>>>    	local ddir=$(kernel_header_destdir)
>>>
>>>    	env_setup_xmakeopts
>>> -	emake headers_install INSTALL_HDR_PATH="${ED}"${ddir}/.. ${xmakeopts}
>>> +	emake headers_install INSTALL_HDR_PATH="${ED}"${ddir}/.. "${xmakeopts[@]}"
>>>
>>>    	# let other packages install some of these headers
>>>    	rm -rf "${ED}"${ddir}/scsi || die #glibc/uclibc/etc...
>>
>> Can we perhaps use this opportunity to make "xmakeopts" more clear via a better name, as well as uppercase it
>> to indicate that it is an exported variable?  E.g., something like "CROSS_MAKEOPTS" is more clear to the
>> reader than "xmakeopts", IMHO.
>>
>> I realize such a change may be a tad invasive to the eclass and possibly touch some ebuilds, so that may need
>> to be a separate patch that this proposed change would then be based off of.
>
> I hadn't noticed some older linux-headers ebuilds use this variable, so thanks
> for bringing that to my attention. Arguably they shouldn't, as it appears to
> be an internal variable, even if it is exported, but it's been dropped in
> later versions anyway. I've now made further changes. Please see the two
> additional patches.

The changes look good to me, thanks!

Signed-off-by: Joshua Kinard <kumba@g.o>


-- 
Joshua Kinard
Gentoo/MIPS
kumba@g.o
rsa6144/5C63F4E3F5C6C943 2015-04-27
177C 1972 1FB8 F254 BAD0 3E72 5C63 F4E3 F5C6 C943

"The past tempts us, the present confuses us, the future frightens us.  And our lives slip away, moment by 
moment, lost in that vast, terrible in-between."

         --Emperor Turhan, Centauri Republic