Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Schrempf, On Thu, 7 Nov 2019 at 12:14, Schrempf Frieder wrote: > Hi Simon, > > On 07.11.19 17:23, Simon Glass wrote: > > Hi Schrempf, > > > > On Thu, 7 Nov 2019 at 08:15, Schrempf Frieder > > wrote: > >> > >> On 07.11.19 15:02, Bin Meng wrote: > >>> Hi Frieder, > >>> > >>> On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder > >>> 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 > > 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. > > Ok, thanks for the explanation. > > > > > If you have time (sounds like you do!), you could incorporate that > > into buildman. > > It's kind of funny that you got the impression, that I have time ;) > Actually I do not have much time to work on U-Boot in general among all > the other things. > > And now I went deep down into the rabbit hole from "I want to get some > boards upstreamed" to "I need to port a QSPI controller driver first" to > "the driver port affects existing CONFIG options that are a total mess > and need to be fixed" to "I need to run buildman on my cleanup patches" > to "buildman could need some tweaking". > > So unless there will be a lot of rainy weekends, I probably won't start > working on optimizing buildman ;) > May you be plagued with downpours :-) - Simon ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Simon, On 07.11.19 17:23, Simon Glass wrote: > Hi Schrempf, > > On Thu, 7 Nov 2019 at 08:15, Schrempf Frieder > wrote: >> >> On 07.11.19 15:02, Bin Meng wrote: >>> Hi Frieder, >>> >>> On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder >>> 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 > 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. Ok, thanks for the explanation. > > If you have time (sounds like you do!), you could incorporate that > into buildman. It's kind of funny that you got the impression, that I have time ;) Actually I do not have much time to work on U-Boot in general among all the other things. And now I went deep down into the rabbit hole from "I want to get some boards upstreamed" to "I need to port a QSPI controller driver first" to "the driver port affects existing CONFIG options that are a total mess and need to be fixed" to "I need to run buildman on my cleanup patches" to "buildman could need some tweaking". So unless there will be a lot of rainy weekends, I probably won't start working on optimizing buildman ;) Regards, Frieder > > 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 > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Schrempf, On Thu, 7 Nov 2019 at 08:15, Schrempf Frieder wrote: > > On 07.11.19 15:02, Bin Meng wrote: > > Hi Frieder, > > > > On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder > > 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 > >>> 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 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
On 07.11.19 15:02, Bin Meng wrote: > Hi Frieder, > > On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder > 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 >>> 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. ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Frieder, On Thu, Nov 7, 2019 at 9:28 PM Schrempf Frieder 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 > > 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. Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Bin, On 07.11.19 13:41, Bin Meng wrote: > Hi Schrempf, > > On Thu, Nov 7, 2019 at 12:17 AM Schrempf Frieder > 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? Thanks, Frieder > >> for a build where the added Kconfig options are missing in the resulting >> u-boot-spl.cfg. >> >> And: >> >> buildman -b spi_flash_kconfig_cleanup_3^..spi_flash_kconfig_cleanup_3 >> xilinx_zynqmp_virt >> >> for a build of only the last commit with expected output. >> >> Thanks, >> Frieder >> >> [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup_3 > > Regards, > Bin > ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
Hi Schrempf, On Thu, Nov 7, 2019 at 12:17 AM Schrempf Frieder 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? > for a build where the added Kconfig options are missing in the resulting > u-boot-spl.cfg. > > And: > > buildman -b spi_flash_kconfig_cleanup_3^..spi_flash_kconfig_cleanup_3 > xilinx_zynqmp_virt > > for a build of only the last commit with expected output. > > Thanks, > Frieder > > [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup_3 Regards, Bin ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
Re: [U-Boot] Buildman Kconfig issue with consecutive builds
On 06.11.19 17:16, Frieder Schrempf 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 Forgot to mention that my master is set to 32d870a82203a16a6e05958e2a02287af4dd825a (which is not upstream) in this case. > > for a build where the added Kconfig options are missing in the resulting > u-boot-spl.cfg. > > And: > > buildman -b spi_flash_kconfig_cleanup_3^..spi_flash_kconfig_cleanup_3 > xilinx_zynqmp_virt > > for a build of only the last commit with expected output. > > Thanks, > Frieder > > [1]: > https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup_3 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot
[U-Boot] Buildman Kconfig issue with consecutive builds
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 for a build where the added Kconfig options are missing in the resulting u-boot-spl.cfg. And: buildman -b spi_flash_kconfig_cleanup_3^..spi_flash_kconfig_cleanup_3 xilinx_zynqmp_virt for a build of only the last commit with expected output. Thanks, Frieder [1]: https://github.com/fschrempf/u-boot/commits/spi_flash_kconfig_cleanup_3 ___ U-Boot mailing list U-Boot@lists.denx.de https://lists.denx.de/listinfo/u-boot