Message ID | 1574771063-29218-1-git-send-email-angelo@amarulasolutions.com |
---|---|
State | Superseded |
Headers | show |
Series | [v2] package/pkg-kconfig: fix reconfigure for kconfig packages | expand |
On Tue, 26 Nov 2019 13:24:23 +0100 Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > From: Angelo Compagnucci <angelo.compagnucci@gmail.com> > > Commit 4b81badbcc0b25678ac6627548160702731cf393 > > Currently, calling foo-reconfigure for a kconfig-based package will not > re-trigger the configuration (kconfig-wise) step for the package. > > already solved this problem, but it was lately regressed by > > Commit 05fea6e4a60a38a797d9bacbf318a2cd7dbd435f Are you sure it was regressed ? When I read the code at commit 4b81badbcc0b25678ac6627548160702731cf393, I don't see even back then how linux-reconfigure would trigger a complete reconfiguration. Have you actually verified that it was working as expected back at commit 4b81badbcc0b25678ac6627548160702731cf393 ? Indeed, at commit 4b81badbcc0b25678ac6627548160702731cf393, we were already only removing the stamp_kconfig_fixup_done stamp file, so my understanding is rather that commit 4b81badbcc0b25678ac6627548160702731cf393 doesn't implement what the commit log says. Thomas
On Tue, Nov 26, 2019 at 2:18 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Tue, 26 Nov 2019 13:24:23 +0100 > Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > > From: Angelo Compagnucci <angelo.compagnucci@gmail.com> > > > > Commit 4b81badbcc0b25678ac6627548160702731cf393 > > > > Currently, calling foo-reconfigure for a kconfig-based package will not > > re-trigger the configuration (kconfig-wise) step for the package. > > > > already solved this problem, but it was lately regressed by > > > > Commit 05fea6e4a60a38a797d9bacbf318a2cd7dbd435f > > Are you sure it was regressed ? When I read the code at commit > 4b81badbcc0b25678ac6627548160702731cf393, I don't see even back then > how linux-reconfigure would trigger a complete reconfiguration. Have > you actually verified that it was working as expected back at commit > 4b81badbcc0b25678ac6627548160702731cf393 ? Indeed, at commit > 4b81badbcc0b25678ac6627548160702731cf393, we were already only removing > the stamp_kconfig_fixup_done stamp file, so my understanding is rather > that commit 4b81badbcc0b25678ac6627548160702731cf393 doesn't implement > what the commit log says. Not tested that commit really, I guessed based on what the commit messages said. I'll rework the commit message . Thanks! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Tue, 26 Nov 2019 14:22:26 +0100 Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > Are you sure it was regressed ? When I read the code at commit > > 4b81badbcc0b25678ac6627548160702731cf393, I don't see even back then > > how linux-reconfigure would trigger a complete reconfiguration. Have > > you actually verified that it was working as expected back at commit > > 4b81badbcc0b25678ac6627548160702731cf393 ? Indeed, at commit > > 4b81badbcc0b25678ac6627548160702731cf393, we were already only removing > > the stamp_kconfig_fixup_done stamp file, so my understanding is rather > > that commit 4b81badbcc0b25678ac6627548160702731cf393 doesn't implement > > what the commit log says. > > Not tested that commit really, I guessed based on what the commit messages said. > > I'll rework the commit message . Instead of just reworking the commit message, I think it would instead make sense to "git checkout 4b81badbcc0b25678ac6627548160702731cf393" and test what was the behavior back then. Thomas
On Tue, Nov 26, 2019 at 2:29 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Tue, 26 Nov 2019 14:22:26 +0100 > Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > > > Are you sure it was regressed ? When I read the code at commit > > > 4b81badbcc0b25678ac6627548160702731cf393, I don't see even back then > > > how linux-reconfigure would trigger a complete reconfiguration. Have > > > you actually verified that it was working as expected back at commit > > > 4b81badbcc0b25678ac6627548160702731cf393 ? Indeed, at commit > > > 4b81badbcc0b25678ac6627548160702731cf393, we were already only removing > > > the stamp_kconfig_fixup_done stamp file, so my understanding is rather > > > that commit 4b81badbcc0b25678ac6627548160702731cf393 doesn't implement > > > what the commit log says. > > > > Not tested that commit really, I guessed based on what the commit messages said. > > > > I'll rework the commit message . > > Instead of just reworking the commit message, I think it would instead > make sense to "git checkout 4b81badbcc0b25678ac6627548160702731cf393" > and test what was the behavior back then. Checked out at 4b81badbcc0b25678ac6627548160702731cf393 $ make linux-reconfigure umask 0022 && make -C /home/angelo/DEV/BUILDROOT/buildroot O=/home/angelo/DEV/BUILDROOT/br_qemu_arm/. linux-reconfigure rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_staging_installed rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_target_installed rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_images_installed rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_host_installed rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_built rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_kconfig_fixup_done rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_configured /bin/sed -i -e "/\\<CONFIG_KERNEL_GZIP\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo 'CONFIG_KERNEL_GZIP=y' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config /bin/sed -i -e "/\\<CONFIG_KERNEL_XZ\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo '# CONFIG_KERNEL_XZ is not set' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config /bin/sed -i -e "/\\<CONFIG_CPU_LITTLE_ENDIAN\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo 'CONFIG_CPU_LITTLE_ENDIAN=y' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config /bin/sed -i -e "/\\<CONFIG_AEABI\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo 'CONFIG_AEABI=y' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config # As the kernel gets compiled before root filesystems are # built, we create a fake cpio file. It'll be # replaced later by the real cpio archive, and the kernel will be # rebuilt using the linux-rebuild-with-initramfs target. /bin/sed -i -e "/\\<CONFIG_DEVTMPFS\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo 'CONFIG_DEVTMPFS=y' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config /bin/sed -i -e "/\\<CONFIG_DEVTMPFS_MOUNT\\>/d" /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config echo 'CONFIG_DEVTMPFS_MOUNT=y' >> /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config scripts/kconfig/conf --olddefconfig Kconfig # # configuration written to .config # >>> linux 4.7 Configuring >>> linux 4.7 Building > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
On Tue, 26 Nov 2019 14:55:12 +0100 Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > Instead of just reworking the commit message, I think it would instead > > make sense to "git checkout 4b81badbcc0b25678ac6627548160702731cf393" > > and test what was the behavior back then. > > Checked out at 4b81badbcc0b25678ac6627548160702731cf393 > > $ make linux-reconfigure > umask 0022 && make -C /home/angelo/DEV/BUILDROOT/buildroot > O=/home/angelo/DEV/BUILDROOT/br_qemu_arm/. linux-reconfigure > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_staging_installed > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_target_installed > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_images_installed > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_host_installed > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_built > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_kconfig_fixup_done > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_configured > /bin/sed -i -e "/\\<CONFIG_KERNEL_GZIP\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo 'CONFIG_KERNEL_GZIP=y' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > /bin/sed -i -e "/\\<CONFIG_KERNEL_XZ\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo '# CONFIG_KERNEL_XZ is not set' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > /bin/sed -i -e "/\\<CONFIG_CPU_LITTLE_ENDIAN\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo 'CONFIG_CPU_LITTLE_ENDIAN=y' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > /bin/sed -i -e "/\\<CONFIG_AEABI\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo 'CONFIG_AEABI=y' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > # As the kernel gets compiled before root filesystems are > # built, we create a fake cpio file. It'll be > # replaced later by the real cpio archive, and the kernel will be > # rebuilt using the linux-rebuild-with-initramfs target. > /bin/sed -i -e "/\\<CONFIG_DEVTMPFS\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo 'CONFIG_DEVTMPFS=y' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > /bin/sed -i -e "/\\<CONFIG_DEVTMPFS_MOUNT\\>/d" > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > echo 'CONFIG_DEVTMPFS_MOUNT=y' >> > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > scripts/kconfig/conf --olddefconfig Kconfig So indeed, it only redoes the fixups, and does not regenerate the .config from scratch. So there was no regression introduced later, it's just that from the start, the -reconfigure behavior did not match what the commit log said. Thomas
On Tue, Nov 26, 2019 at 2:58 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Tue, 26 Nov 2019 14:55:12 +0100 > Angelo Compagnucci <angelo@amarulasolutions.com> wrote: > > > > Instead of just reworking the commit message, I think it would instead > > > make sense to "git checkout 4b81badbcc0b25678ac6627548160702731cf393" > > > and test what was the behavior back then. > > > > Checked out at 4b81badbcc0b25678ac6627548160702731cf393 > > > > $ make linux-reconfigure > > umask 0022 && make -C /home/angelo/DEV/BUILDROOT/buildroot > > O=/home/angelo/DEV/BUILDROOT/br_qemu_arm/. linux-reconfigure > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_staging_installed > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_target_installed > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_images_installed > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_host_installed > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_built > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_kconfig_fixup_done > > rm -f /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.stamp_configured > > /bin/sed -i -e "/\\<CONFIG_KERNEL_GZIP\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo 'CONFIG_KERNEL_GZIP=y' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > /bin/sed -i -e "/\\<CONFIG_KERNEL_XZ\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo '# CONFIG_KERNEL_XZ is not set' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > /bin/sed -i -e "/\\<CONFIG_CPU_LITTLE_ENDIAN\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo 'CONFIG_CPU_LITTLE_ENDIAN=y' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > /bin/sed -i -e "/\\<CONFIG_AEABI\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo 'CONFIG_AEABI=y' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > # As the kernel gets compiled before root filesystems are > > # built, we create a fake cpio file. It'll be > > # replaced later by the real cpio archive, and the kernel will be > > # rebuilt using the linux-rebuild-with-initramfs target. > > /bin/sed -i -e "/\\<CONFIG_DEVTMPFS\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo 'CONFIG_DEVTMPFS=y' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > /bin/sed -i -e "/\\<CONFIG_DEVTMPFS_MOUNT\\>/d" > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > echo 'CONFIG_DEVTMPFS_MOUNT=y' >> > > /home/angelo/DEV/BUILDROOT/br_qemu_arm/build/linux-4.7/.config > > scripts/kconfig/conf --olddefconfig Kconfig > > So indeed, it only redoes the fixups, and does not regenerate the > .config from scratch. So there was no regression introduced later, it's > just that from the start, the -reconfigure behavior did not match what > the commit log said. Indeed, so I supposed v3 og the patch has a more suitable commit message. Thanks again! > > Thomas > -- > Thomas Petazzoni, CTO, Bootlin > Embedded Linux and Kernel engineering > https://bootlin.com
diff --git a/package/pkg-kconfig.mk b/package/pkg-kconfig.mk index 86d7c14..f1931b8 100644 --- a/package/pkg-kconfig.mk +++ b/package/pkg-kconfig.mk @@ -175,7 +175,7 @@ $$($(2)_TARGET_CONFIGURE): $$($(2)_DIR)/.stamp_kconfig_fixup_done $(1)-clean-for-reconfigure: $(1)-clean-kconfig-for-reconfigure $(1)-clean-kconfig-for-reconfigure: - rm -f $$($(2)_DIR)/.stamp_kconfig_fixup_done + rm -f $$($(2)_DIR)/$$($(2)_KCONFIG_STAMP_DOTCONFIG) # Only enable the foo-*config targets when the package is actually enabled. # Note: the variable $(2)_KCONFIG_VAR is not related to the kconfig