Re: [PATCH 2/3] automation: Change build script to use arch defconfig

[Jan Beulich <jbeulich@xxxxxxxx>]

On 26.09.2023 20:35, Shawn Anastasio wrote:
> On 9/26/23 12:43 PM, Shawn Anastasio wrote:
>> On 9/26/23 2:19 AM, Jan Beulich wrote:
>>> On 26.09.2023 01:12, Stefano Stabellini wrote:
>>>> On Mon, 25 Sep 2023, Shawn Anastasio wrote:
>>>>> Change automation build script to call the make defconfig target before
>>>>> setting CONFIG_DEBUG and extra options. This fixes issues on Power where
>>>>> the build fails without using the ppc64_defconfig.
>>>>>
>>>>> Signed-off-by: Shawn Anastasio <sanastasio@xxxxxxxxxxxxxxxxxxxxx>
>>>>> Reported-by: Jan Beulich <jbeulich@xxxxxxxx>
>>>
>>> Nit: Tags in chronological order please (also affects patch 1).
>>>
>> Will fix.
>>
>>>> What is the problem specifically? Is the issue that CONFIG_DEBUG enabled
>>>> before make olddefconfig causes certain DEBUG options also to default to
>>>> yes, and these additional options don't work well on Power?
>>>
>>> No, the issue is that "make olddefconfig" takes the existing .config
>>> without even considering the arch's default configuration that was
>>> specified (KBUILD_DEFCONFIG).
>>>
>>>>> --- a/automation/scripts/build
>>>>> +++ b/automation/scripts/build
>>>>> @@ -22,7 +22,12 @@ if [[ "${RANDCONFIG}" == "y" ]]; then
>>>>>      # RANDCONFIG implies HYPERVISOR_ONLY
>>>>>      HYPERVISOR_ONLY="y"
>>>>>  else
>>>>> -    echo "CONFIG_DEBUG=${debug}" > xen/.config
>>>>> +    # Start off with arch's defconfig
>>>>> +    make -C xen defconfig
>>>>> +
>>>>> +    # Drop existing CONFIG_DEBUG and replace with value of ${debug}
>>>>> +    sed -i 's/^CONFIG_DEBUG=[yn]//g' xen/.config
>>>>> +    echo "CONFIG_DEBUG=${debug}" >> xen/.config
>>>>>
>>>>>      if [[ -n "${EXTRA_XEN_CONFIG}" ]]; then
>>>>>          echo "${EXTRA_XEN_CONFIG}" >> xen/.config
>>>
>>> It never really became clear to me whether kconfig honors the first,
>>> last, or any random setting in a .config that it takes as input, when
>>> a certain option appears there more than once. The change you make
>>> implies it's consistently "last" - can you confirm that's the actual
>>> behavior of kconfig?
>>>
>>
>> I actually tried to avoid this issue alltogether with the sed command I
>> added before the echo to drop any existing CONFIG_DEBUG= line.
>>
> 
> Just realized that options in $EXTRA_XEN_CONFIG would also be subject to
> this which is likely what you meant to point out -- my apologies.
> 
> I've tested locally and Kconfig does indeed seem to honor the last
> setting. It throws some warnings about the overridden symbol but the
> build continues with the latest value, e.g:
> 
> .config:94:warning: override: reassigning to symbol DEBUG
> .config:95:warning: override: reassigning to symbol DEBUG

I might guess such warnings are okay to appear, but this needs confirming
by one of the people more familiar with how the CI works and what output
is or is not okay to appear. I can see upsides and downsides to such
warnings ...

Jan