[U-Boot] Buildman Kconfig issue with consecutive builds

[Simon Glass]

Hi Schrempf,

On Thu, 7 Nov 2019 at 08:15, Schrempf Frieder
<frieder.schrempf at kontron.de> wrote:
>
> On 07.11.19 15:02, Bin Meng wrote:
> > Hi Frieder,
> >
> > On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder
> > <frieder.schrempf at kontron.de> wrote:
> >>
> >> Hi Bin,
> >>
> >> On 07.11.19 13:41, Bin Meng wrote:
> >>> Hi Schrempf,
> >>>
> >>> On Thu, Nov 7, 2019 at 12:17 AM Schrempf Frieder
> >>> <frieder.schrempf at kontron.de> wrote:
> >>>>
> >>>> Hi,
> >>>>
> >>>> I'm having some trouble using buildman to test the impact of some
> >>>> Kconfig cleanup patches ([1]).
> >>>>
> >>>> The patches introduce a new CONFIG_SPL_* option and I try to find out
> >>>> which defconfigs need to be fixed, by comparing build sizes.
> >>>>
> >>>> Now when I added a patch to fix a defconfig I noticed that buildman
> >>>> wouldn't report the expected size changes and upon looking more closely
> >>>> I found that the added Kconfig options are still missing in u-boot-spl.cfg.
> >>>>
> >>>> The strange thing is, that when I try to build only the last commit then
> >>>> the Kconfig options are there, which is why I suspect a bug in buildman
> >>>> not handling Kconfig changes correctly with consecutive builds.
> >>>>
> >>>> Can anyone have a look what is wrong or how I can debug this issue?
> >>>>
> >>>> The issue can be reproduced with the branch at [1], running:
> >>>>
> >>>> buildman -b spi_flash_kconfig_cleanup_3 --step 0 xilinx_zynqmp_virt
> >>>>
> >>>
> >>> Could you please add "-C" to the buildman command line and have a try?
> >>
> >> Indeed forcing the reconfig between the build steps with '-C' fixes the
> >> issue.
> >>
> >> Is it a known problem, that buildman doesn't handle Kconfig changes
> >> correctly without '-C' in some cases?
> >
> > AFAIK, this is an intended design of calling buildman w/o '-C' to save
> > some build time.
>
> Ok, if that's the case I will try to come up with a patch that adds a
> note to the README. This has cost me a few hours because I was thinking
> buildman does the right thing and Kconfig options are messed up somewhere.

An incremental build means that it does not run 'make xxx_defconfig'
on every commit. Doing it this way saves *a lot* of time for large
builds and the main purpose of buildman is to validate that U-Boot
builds.

However it might be possible to have it both ways...the code fragment
below compares the Kconfig files and configs/ directory against the
data of the 'u-boot' output file, and could trigger a full rebuild if
newer.

If you have time (sounds like you do!), you could incorporate that
into buildman.

files = ['%s/u-boot' % outdir]
  if os.path.exists(files[0]):
    if options.incremental:
      cmd = ['find', 'configs/', '-cnewer', files[0]]
      result = cros_build_lib.RunCommand(cmd, capture_output=True, **kwargs)
      if result.output:
        logging.warning('config/ dir has changed - dropping -i')
        options.incremental = False

    if options.incremental:
      cmd = ['find', '.', '-name', 'Kconfig', '-and', '-cnewer', files[0]]
      result = cros_build_lib.RunCommand(cmd, capture_output=True, **kwargs)
      if result.output:
        logging.warning('Kconfig file(s) changed - dropping -i')
        options.incremental = False


The current logic is in RunJob() and do_config is the thing that
causes a reconfig.

Regards,
Simon