[for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Michal Orzel]

At the moment, in order to use a different defconfig target than default,
one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
Switch to weak assignment, so that it can be also obtained from
environment similar to other KCONFIG/KBUILD variables.

This change will activate the use of KBUILD_DEFCONFIG variable in CI
build jobs that so far had no effect.

Note, that we will deviate from Linux in this regard.

Signed-off-by: Michal Orzel <michal.orzel@amd.com>
---
In case of reluctance to this approach, we can always do:
 make -C xen defconfig KBUILD_DEFCONFIG=${KBUILD_DEFCONFIG}
in automation/scripts/build.

Also, Linux defers setting the variable to arch specific Makefile, so it is not
sth mandated by the top level Makefile or documentation.
---
 xen/Makefile | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/xen/Makefile b/xen/Makefile
index fd0e63d29efb..40feefff4ab5 100644
--- a/xen/Makefile
+++ b/xen/Makefile
@@ -256,7 +256,8 @@ export YACC = $(if $(BISON),$(BISON),bison)
 export LEX = $(if $(FLEX),$(FLEX),flex)
 
 # Default file for 'make defconfig'.
-export KBUILD_DEFCONFIG := $(ARCH)_defconfig
+# May be overruled on the command line or set in the environment.
+export KBUILD_DEFCONFIG ?= $(ARCH)_defconfig
 
 # Copy CFLAGS generated by "Config.mk" so they can be reused later without
 # reparsing Config.mk by e.g. arch/x86/boot/.
-- 
2.25.1

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Jan Beulich]

On 25.10.2023 10:28, Michal Orzel wrote:
> At the moment, in order to use a different defconfig target than default,
> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
> Switch to weak assignment, so that it can be also obtained from
> environment similar to other KCONFIG/KBUILD variables.
> 
> This change will activate the use of KBUILD_DEFCONFIG variable in CI
> build jobs that so far had no effect.

I'm certainly okay with the change, but the above sentence looks misleading
to me: Yes, the envvar was ignored so far, but isn't it the case that the
envvar as specified in CI matches what Makefile set it to (taking into
account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
hence the specifications in build.yaml could be dropped (until such time
where truly an override was intended)?

Jan

> Note, that we will deviate from Linux in this regard.
> 
> Signed-off-by: Michal Orzel <michal.orzel@amd.com>
> ---
> In case of reluctance to this approach, we can always do:
>  make -C xen defconfig KBUILD_DEFCONFIG=${KBUILD_DEFCONFIG}
> in automation/scripts/build.
> 
> Also, Linux defers setting the variable to arch specific Makefile, so it is not
> sth mandated by the top level Makefile or documentation.
> ---
>  xen/Makefile | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/xen/Makefile b/xen/Makefile
> index fd0e63d29efb..40feefff4ab5 100644
> --- a/xen/Makefile
> +++ b/xen/Makefile
> @@ -256,7 +256,8 @@ export YACC = $(if $(BISON),$(BISON),bison)
>  export LEX = $(if $(FLEX),$(FLEX),flex)
>  
>  # Default file for 'make defconfig'.
> -export KBUILD_DEFCONFIG := $(ARCH)_defconfig
> +# May be overruled on the command line or set in the environment.
> +export KBUILD_DEFCONFIG ?= $(ARCH)_defconfig
>  
>  # Copy CFLAGS generated by "Config.mk" so they can be reused later without
>  # reparsing Config.mk by e.g. arch/x86/boot/.

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Michal Orzel]

Hi,

On 25/10/2023 11:10, Jan Beulich wrote:
> 
> 
> On 25.10.2023 10:28, Michal Orzel wrote:
>> At the moment, in order to use a different defconfig target than default,
>> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
>> Switch to weak assignment, so that it can be also obtained from
>> environment similar to other KCONFIG/KBUILD variables.
>>
>> This change will activate the use of KBUILD_DEFCONFIG variable in CI
>> build jobs that so far had no effect.
> 
> I'm certainly okay with the change, but the above sentence looks misleading
> to me: Yes, the envvar was ignored so far, but isn't it the case that the
> envvar as specified in CI matches what Makefile set it to (taking into
> account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
> hence the specifications in build.yaml could be dropped (until such time
> where truly an override was intended)?
Well, today riscv64_defconfig matches tiny64_defconfig but it can change. Otherwise, why
would we need to have 2 identical files? Looking at the latest full build series from Oleksi,
only the tiny64_defconfig file gets updated which would be the clear indication that what is
specified in the CI matches the author's expectation.

Also, I never mentioned that this change fixes something. I just wrote that it gives a meaning
to a variable that so far had no effect.

~Michal

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Jan Beulich]

On 25.10.2023 11:21, Michal Orzel wrote:
> On 25/10/2023 11:10, Jan Beulich wrote:
>> On 25.10.2023 10:28, Michal Orzel wrote:
>>> At the moment, in order to use a different defconfig target than default,
>>> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
>>> Switch to weak assignment, so that it can be also obtained from
>>> environment similar to other KCONFIG/KBUILD variables.
>>>
>>> This change will activate the use of KBUILD_DEFCONFIG variable in CI
>>> build jobs that so far had no effect.
>>
>> I'm certainly okay with the change, but the above sentence looks misleading
>> to me: Yes, the envvar was ignored so far, but isn't it the case that the
>> envvar as specified in CI matches what Makefile set it to (taking into
>> account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
>> hence the specifications in build.yaml could be dropped (until such time
>> where truly an override was intended)?
> Well, today riscv64_defconfig matches tiny64_defconfig but it can change. Otherwise, why
> would we need to have 2 identical files? Looking at the latest full build series from Oleksi,
> only the tiny64_defconfig file gets updated which would be the clear indication that what is
> specified in the CI matches the author's expectation.
> 
> Also, I never mentioned that this change fixes something. I just wrote that it gives a meaning
> to a variable that so far had no effect.

Well, sure, but if you e.g. said "... that so far would have had no effect
if they didn't match the default anyway", things would have been unambiguous.

Jan

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Michal Orzel]

On 25/10/2023 11:26, Jan Beulich wrote:
> 
> 
> On 25.10.2023 11:21, Michal Orzel wrote:
>> On 25/10/2023 11:10, Jan Beulich wrote:
>>> On 25.10.2023 10:28, Michal Orzel wrote:
>>>> At the moment, in order to use a different defconfig target than default,
>>>> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
>>>> Switch to weak assignment, so that it can be also obtained from
>>>> environment similar to other KCONFIG/KBUILD variables.
>>>>
>>>> This change will activate the use of KBUILD_DEFCONFIG variable in CI
>>>> build jobs that so far had no effect.
>>>
>>> I'm certainly okay with the change, but the above sentence looks misleading
>>> to me: Yes, the envvar was ignored so far, but isn't it the case that the
>>> envvar as specified in CI matches what Makefile set it to (taking into
>>> account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
>>> hence the specifications in build.yaml could be dropped (until such time
>>> where truly an override was intended)?
>> Well, today riscv64_defconfig matches tiny64_defconfig but it can change. Otherwise, why
>> would we need to have 2 identical files? Looking at the latest full build series from Oleksi,
>> only the tiny64_defconfig file gets updated which would be the clear indication that what is
>> specified in the CI matches the author's expectation.
>>
>> Also, I never mentioned that this change fixes something. I just wrote that it gives a meaning
>> to a variable that so far had no effect.
> 
> Well, sure, but if you e.g. said "... that so far would have had no effect
> if they didn't match the default anyway", things would have been unambiguous.
Ok, I can see you did not provide any tag in which case I will wait for other's feedback.
Then, I can either respin the patch adding sentence you suggested or leave it to Stefano
to do when committing to his for-4.19 branch.

~Michal

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Jan Beulich]

On 25.10.2023 11:35, Michal Orzel wrote:
> 
> 
> On 25/10/2023 11:26, Jan Beulich wrote:
>>
>>
>> On 25.10.2023 11:21, Michal Orzel wrote:
>>> On 25/10/2023 11:10, Jan Beulich wrote:
>>>> On 25.10.2023 10:28, Michal Orzel wrote:
>>>>> At the moment, in order to use a different defconfig target than default,
>>>>> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
>>>>> Switch to weak assignment, so that it can be also obtained from
>>>>> environment similar to other KCONFIG/KBUILD variables.
>>>>>
>>>>> This change will activate the use of KBUILD_DEFCONFIG variable in CI
>>>>> build jobs that so far had no effect.
>>>>
>>>> I'm certainly okay with the change, but the above sentence looks misleading
>>>> to me: Yes, the envvar was ignored so far, but isn't it the case that the
>>>> envvar as specified in CI matches what Makefile set it to (taking into
>>>> account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
>>>> hence the specifications in build.yaml could be dropped (until such time
>>>> where truly an override was intended)?
>>> Well, today riscv64_defconfig matches tiny64_defconfig but it can change. Otherwise, why
>>> would we need to have 2 identical files? Looking at the latest full build series from Oleksi,
>>> only the tiny64_defconfig file gets updated which would be the clear indication that what is
>>> specified in the CI matches the author's expectation.
>>>
>>> Also, I never mentioned that this change fixes something. I just wrote that it gives a meaning
>>> to a variable that so far had no effect.
>>
>> Well, sure, but if you e.g. said "... that so far would have had no effect
>> if they didn't match the default anyway", things would have been unambiguous.
> Ok, I can see you did not provide any tag in which case I will wait for other's feedback.
> Then, I can either respin the patch adding sentence you suggested or leave it to Stefano
> to do when committing to his for-4.19 branch.

The reason I didn't offer A-b (yet) is that with the given description plus
the claim on Matrix by someone that things don't work because of this
override not working, it wasn't really clear to me whether that claim was
wrong, or whether my view of the situation is. In the latter case I could
hardly ack the patch, as that would then mean I'd ack something I don't
understand. Provided there really has not been any breakage so far because
of this, feel free to add
Acked-by: Jan Beulich <jbeulich@suse.com>
preferably with the slightly adjusted description.

Jan

Re: [for-4.19][PATCH] build: Allow setting KBUILD_DEFCONFIG in the environment

[Stefano Stabellini]

On Wed, 25 Oct 2023, Jan Beulich wrote:
> On 25.10.2023 11:35, Michal Orzel wrote:
> > 
> > 
> > On 25/10/2023 11:26, Jan Beulich wrote:
> >>
> >>
> >> On 25.10.2023 11:21, Michal Orzel wrote:
> >>> On 25/10/2023 11:10, Jan Beulich wrote:
> >>>> On 25.10.2023 10:28, Michal Orzel wrote:
> >>>>> At the moment, in order to use a different defconfig target than default,
> >>>>> one needs to specify KBUILD_DEFCONFIG=<target> on the command line.
> >>>>> Switch to weak assignment, so that it can be also obtained from
> >>>>> environment similar to other KCONFIG/KBUILD variables.
> >>>>>
> >>>>> This change will activate the use of KBUILD_DEFCONFIG variable in CI
> >>>>> build jobs that so far had no effect.
> >>>>
> >>>> I'm certainly okay with the change, but the above sentence looks misleading
> >>>> to me: Yes, the envvar was ignored so far, but isn't it the case that the
> >>>> envvar as specified in CI matches what Makefile set it to (taking into
> >>>> account that for RISC-V riscv64_defconfig aliases tiny64_defconfig), and
> >>>> hence the specifications in build.yaml could be dropped (until such time
> >>>> where truly an override was intended)?
> >>> Well, today riscv64_defconfig matches tiny64_defconfig but it can change. Otherwise, why
> >>> would we need to have 2 identical files? Looking at the latest full build series from Oleksi,
> >>> only the tiny64_defconfig file gets updated which would be the clear indication that what is
> >>> specified in the CI matches the author's expectation.
> >>>
> >>> Also, I never mentioned that this change fixes something. I just wrote that it gives a meaning
> >>> to a variable that so far had no effect.
> >>
> >> Well, sure, but if you e.g. said "... that so far would have had no effect
> >> if they didn't match the default anyway", things would have been unambiguous.
> > Ok, I can see you did not provide any tag in which case I will wait for other's feedback.
> > Then, I can either respin the patch adding sentence you suggested or leave it to Stefano
> > to do when committing to his for-4.19 branch.
> 
> The reason I didn't offer A-b (yet) is that with the given description plus
> the claim on Matrix by someone that things don't work because of this
> override not working, it wasn't really clear to me whether that claim was
> wrong, or whether my view of the situation is. In the latter case I could
> hardly ack the patch, as that would then mean I'd ack something I don't
> understand. Provided there really has not been any breakage so far because
> of this, feel free to add
> Acked-by: Jan Beulich <jbeulich@suse.com>
> preferably with the slightly adjusted description.

Added to for-4.19 with the adjusted commit message