| Message ID | beb83dd70b15ee6bd896fb26532f5debf955cd48.1695681330.git.sanastasio@raptorengineering.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | Fix Power CI build | expand |
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@raptorengineering.com> > Reported-by: Jan Beulich <jbeulich@suse.com> 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? If that is the case, wouldn't it be better to remove the -debug jobs until they work well on Power? Or make them allow_failure: true ? > --- > automation/scripts/build | 7 ++++++- > 1 file changed, 6 insertions(+), 1 deletion(-) > > diff --git a/automation/scripts/build b/automation/scripts/build > index b4edcf010e..19dd9e8270 100755 > --- 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 > -- > 2.30.2 >
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@raptorengineering.com> >> Reported-by: Jan Beulich <jbeulich@suse.com> Nit: Tags in chronological order please (also affects patch 1). > 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? Jan
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@raptorengineering.com> >>> Reported-by: Jan Beulich <jbeulich@suse.com> > > 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. > Jan Thanks, Shawn
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@raptorengineering.com> >>>> Reported-by: Jan Beulich <jbeulich@suse.com> >> >> 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 Thanks, Shawn
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@raptorengineering.com> >>>>> Reported-by: Jan Beulich <jbeulich@suse.com> >>> >>> 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
diff --git a/automation/scripts/build b/automation/scripts/build index b4edcf010e..19dd9e8270 100755 --- 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
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@raptorengineering.com> Reported-by: Jan Beulich <jbeulich@suse.com> --- automation/scripts/build | 7 ++++++- 1 file changed, 6 insertions(+), 1 deletion(-) -- 2.30.2