diff mbox series

[v2] package/pkg-kconfig: fix reconfigure for kconfig packages

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

Commit Message

Angelo Compagnucci Nov. 26, 2019, 12:24 p.m. UTC 
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

    infra/pkg-kconfig: do not rely on package's .config as a timestamp

that introduced the .stamp_dotconfig file.

For this reason, to trigger a kconfig package reconfigure is now
necessary to remove the .stamp_dotconfig file.

Signed-off-by: Angelo Compagnucci <angelo@amarulasolutions.com>
---
 package/pkg-kconfig.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni Nov. 26, 2019, 1:18 p.m. UTC | #1 
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
Angelo Compagnucci Nov. 26, 2019, 1:22 p.m. UTC | #2 
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
Thomas Petazzoni Nov. 26, 2019, 1:29 p.m. UTC | #3 
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
Angelo Compagnucci Nov. 26, 2019, 1:55 p.m. UTC | #4 
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
Thomas Petazzoni Nov. 26, 2019, 1:58 p.m. UTC | #5 
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
Angelo Compagnucci Nov. 26, 2019, 1:59 p.m. UTC | #6 
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 mbox series

Patch

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