diff mbox series

tools: Override makefile ARCH variable if defined, but empty

Message ID 20241106193208.290067-1-bjorn@kernel.org (mailing list archive)
State Not Applicable
Headers show
Series tools: Override makefile ARCH variable if defined, but empty | expand

Checks

Context Check Description
netdev/tree_selection success Not a local patch

Commit Message

Björn Töpel Nov. 6, 2024, 7:32 p.m. UTC
From: Björn Töpel <bjorn@rivosinc.com>

There are a number of tools (bpftool, selftests), that require a
"bootstrap" build. Here, a bootstrap build is a build host variant of
a target. E.g., assume that you're performing a bpftool cross-build on
x86 to riscv, a bootstrap build would then be an x86 variant of
bpftool. The typical way to perform the host build variant, is to pass
"ARCH=" in a sub-make. However, if a variable has been set with a
command argument, then ordinary assignments in the makefile are
ignored.

This side-effect results in that ARCH, and variables depending on ARCH
are not set.

Workaround by overriding ARCH to the host arch, if ARCH is empty.

Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
---
 tools/scripts/Makefile.arch | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)


base-commit: 7758b206117dab9894f0bcb8333f8e4731c5065a

Comments

Jean-Philippe Brucker Nov. 7, 2024, 9:13 a.m. UTC | #1
On Wed, Nov 06, 2024 at 08:32:06PM +0100, Björn Töpel wrote:
> From: Björn Töpel <bjorn@rivosinc.com>
> 
> There are a number of tools (bpftool, selftests), that require a
> "bootstrap" build. Here, a bootstrap build is a build host variant of
> a target. E.g., assume that you're performing a bpftool cross-build on
> x86 to riscv, a bootstrap build would then be an x86 variant of
> bpftool. The typical way to perform the host build variant, is to pass
> "ARCH=" in a sub-make. However, if a variable has been set with a
> command argument, then ordinary assignments in the makefile are
> ignored.
> 
> This side-effect results in that ARCH, and variables depending on ARCH
> are not set.
> 
> Workaround by overriding ARCH to the host arch, if ARCH is empty.
> 
> Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>

Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>

> ---
>  tools/scripts/Makefile.arch | 4 ++--
>  1 file changed, 2 insertions(+), 2 deletions(-)
> 
> diff --git a/tools/scripts/Makefile.arch b/tools/scripts/Makefile.arch
> index f6a50f06dfc4..eabfe9f411d9 100644
> --- a/tools/scripts/Makefile.arch
> +++ b/tools/scripts/Makefile.arch
> @@ -7,8 +7,8 @@ HOSTARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
>                                    -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
>                                    -e s/riscv.*/riscv/ -e s/loongarch.*/loongarch/)
>  
> -ifndef ARCH
> -ARCH := $(HOSTARCH)
> +ifeq ($(strip $(ARCH)),)
> +override ARCH := $(HOSTARCH)
>  endif
>  
>  SRCARCH := $(ARCH)
> 
> base-commit: 7758b206117dab9894f0bcb8333f8e4731c5065a
> -- 
> 2.45.2
>
Alexandre Ghiti Nov. 7, 2024, 10:09 a.m. UTC | #2
Hi Björn,

On Thu, Nov 7, 2024 at 10:12 AM Jean-Philippe Brucker
<jean-philippe@linaro.org> wrote:
>
> On Wed, Nov 06, 2024 at 08:32:06PM +0100, Björn Töpel wrote:
> > From: Björn Töpel <bjorn@rivosinc.com>
> >
> > There are a number of tools (bpftool, selftests), that require a
> > "bootstrap" build. Here, a bootstrap build is a build host variant of
> > a target. E.g., assume that you're performing a bpftool cross-build on
> > x86 to riscv, a bootstrap build would then be an x86 variant of
> > bpftool. The typical way to perform the host build variant, is to pass
> > "ARCH=" in a sub-make. However, if a variable has been set with a
> > command argument, then ordinary assignments in the makefile are
> > ignored.
> >
> > This side-effect results in that ARCH, and variables depending on ARCH
> > are not set.
> >
> > Workaround by overriding ARCH to the host arch, if ARCH is empty.
> >
> > Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
> > Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
>
> Reviewed-by: Jean-Philippe Brucker <jean-philippe@linaro.org>
>
> > ---
> >  tools/scripts/Makefile.arch | 4 ++--
> >  1 file changed, 2 insertions(+), 2 deletions(-)
> >
> > diff --git a/tools/scripts/Makefile.arch b/tools/scripts/Makefile.arch
> > index f6a50f06dfc4..eabfe9f411d9 100644
> > --- a/tools/scripts/Makefile.arch
> > +++ b/tools/scripts/Makefile.arch
> > @@ -7,8 +7,8 @@ HOSTARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
> >                                    -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
> >                                    -e s/riscv.*/riscv/ -e s/loongarch.*/loongarch/)
> >
> > -ifndef ARCH
> > -ARCH := $(HOSTARCH)
> > +ifeq ($(strip $(ARCH)),)
> > +override ARCH := $(HOSTARCH)
> >  endif
> >
> >  SRCARCH := $(ARCH)
> >
> > base-commit: 7758b206117dab9894f0bcb8333f8e4731c5065a
> > --
> > 2.45.2
> >

You can add:

Tested-by: Alexandre Ghiti <alexghiti@rivosinc.com>

Thanks,

Alex
Björn Töpel Nov. 20, 2024, 1:25 p.m. UTC | #3
Björn Töpel <bjorn@kernel.org> writes:

> From: Björn Töpel <bjorn@rivosinc.com>
>
> There are a number of tools (bpftool, selftests), that require a
> "bootstrap" build. Here, a bootstrap build is a build host variant of
> a target. E.g., assume that you're performing a bpftool cross-build on
> x86 to riscv, a bootstrap build would then be an x86 variant of
> bpftool. The typical way to perform the host build variant, is to pass
> "ARCH=" in a sub-make. However, if a variable has been set with a
> command argument, then ordinary assignments in the makefile are
> ignored.
>
> This side-effect results in that ARCH, and variables depending on ARCH
> are not set.
>
> Workaround by overriding ARCH to the host arch, if ARCH is empty.
>
> Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>

Arnaldo/Palmer/Quentin:

A bit unsure what tree this patch should go. It's very important for the
RISC-V builds, so maybe via Palmer's RISC-V tree?

Opinions? Just want to make sure it doesn't fall between any chairs!
:-)


Björn
Namhyung Kim Nov. 21, 2024, 6:04 a.m. UTC | #4
On Wed, Nov 20, 2024 at 02:25:22PM +0100, Björn Töpel wrote:
> Björn Töpel <bjorn@kernel.org> writes:
> 
> > From: Björn Töpel <bjorn@rivosinc.com>
> >
> > There are a number of tools (bpftool, selftests), that require a
> > "bootstrap" build. Here, a bootstrap build is a build host variant of
> > a target. E.g., assume that you're performing a bpftool cross-build on
> > x86 to riscv, a bootstrap build would then be an x86 variant of
> > bpftool. The typical way to perform the host build variant, is to pass
> > "ARCH=" in a sub-make. However, if a variable has been set with a
> > command argument, then ordinary assignments in the makefile are
> > ignored.
> >
> > This side-effect results in that ARCH, and variables depending on ARCH
> > are not set.
> >
> > Workaround by overriding ARCH to the host arch, if ARCH is empty.
> >
> > Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
> > Signed-off-by: Björn Töpel <bjorn@rivosinc.com>

Reviewed-by: Namhyung Kim <namhyung@kernel.org>

> 
> Arnaldo/Palmer/Quentin:
> 
> A bit unsure what tree this patch should go. It's very important for the
> RISC-V builds, so maybe via Palmer's RISC-V tree?

I think it'd be best to route this through the bpf tree as it seems the
main target is bpftool.  But given the size and the scope of the change,
it should be fine with perf-tools or RISC-V tree.

Thanks,
Namhyung

> 
> Opinions? Just want to make sure it doesn't fall between any chairs!
> :-)
> 
> 
> Björn
Quentin Monnet Nov. 21, 2024, 11:29 a.m. UTC | #5
2024-11-20 22:04 UTC-0800 ~ Namhyung Kim <namhyung@kernel.org>
> On Wed, Nov 20, 2024 at 02:25:22PM +0100, Björn Töpel wrote:
>> Björn Töpel <bjorn@kernel.org> writes:
>>
>>> From: Björn Töpel <bjorn@rivosinc.com>
>>>
>>> There are a number of tools (bpftool, selftests), that require a
>>> "bootstrap" build. Here, a bootstrap build is a build host variant of
>>> a target. E.g., assume that you're performing a bpftool cross-build on
>>> x86 to riscv, a bootstrap build would then be an x86 variant of
>>> bpftool. The typical way to perform the host build variant, is to pass
>>> "ARCH=" in a sub-make. However, if a variable has been set with a
>>> command argument, then ordinary assignments in the makefile are
>>> ignored.
>>>
>>> This side-effect results in that ARCH, and variables depending on ARCH
>>> are not set.
>>>
>>> Workaround by overriding ARCH to the host arch, if ARCH is empty.
>>>
>>> Fixes: 8859b0da5aac ("tools/bpftool: Fix cross-build")
>>> Signed-off-by: Björn Töpel <bjorn@rivosinc.com>
> 
> Reviewed-by: Namhyung Kim <namhyung@kernel.org>


Acked-by: Quentin Monnet <qmo@kernel.org>


>> Arnaldo/Palmer/Quentin:
>>
>> A bit unsure what tree this patch should go. It's very important for the
>> RISC-V builds, so maybe via Palmer's RISC-V tree?
> 
> I think it'd be best to route this through the bpf tree as it seems the
> main target is bpftool.  But given the size and the scope of the change,
> it should be fine with perf-tools or RISC-V tree.


The bpf tree would make sense to me as well (but I don't merge patches
myself; let me Cc BPF maintainers).

Quentin
diff mbox series

Patch

diff --git a/tools/scripts/Makefile.arch b/tools/scripts/Makefile.arch
index f6a50f06dfc4..eabfe9f411d9 100644
--- a/tools/scripts/Makefile.arch
+++ b/tools/scripts/Makefile.arch
@@ -7,8 +7,8 @@  HOSTARCH := $(shell uname -m | sed -e s/i.86/x86/ -e s/x86_64/x86/ \
                                   -e s/sh[234].*/sh/ -e s/aarch64.*/arm64/ \
                                   -e s/riscv.*/riscv/ -e s/loongarch.*/loongarch/)
 
-ifndef ARCH
-ARCH := $(HOSTARCH)
+ifeq ($(strip $(ARCH)),)
+override ARCH := $(HOSTARCH)
 endif
 
 SRCARCH := $(ARCH)