initramfs: Fix spurious rebuilds

PATCH In review Florian Fainelli 2 participants 3 messages Jan 4, 2017 Kbuild

Share & tools

Florian Fainelli

author

2017-01-04 01:38 UTC #0 lore raw

PATCH 1/1 initramfs: Fix spurious rebuilds

Commit 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile") makes suffix_y be a quote variable, which can be illustrated looking at the build output:

GEN usr/initramfs_data.cpio".gz"

Make sure that we do strip off double quotes from CONFIG_INITRAMFS_COMPRESSION, since the Makefile tracks targets with unquoted suffixes.

Fixes 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile")

Signed-off-by Florian Fainelli <[email protected]>

---
This is against v4.10-rc2, thanks!

 usr/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

usr/Makefile +1 -1

index 17a513268325..a9ae8b493e2b 100644
@@ -5,7 +5,7 @@
 klibcdirs:;
 PHONY += klibcdirs
 
-suffix_y = $(CONFIG_INITRAMFS_COMPRESSION)
+suffix_y = $(subst ",,$(CONFIG_INITRAMFS_COMPRESSION))
 AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/initramfs_data.cpio$(suffix_y)"
 
 # Generate builtin.o based on initramfs_data.o

Signature · 1 line

2.9.3

klondike

2017-01-04 13:39 UTC #1 lore raw

Quoted 23 lines · Florian Fainelli

Commit 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile") makes suffix_y be a quote variable, which can be illustrated looking at the build output:
GEN usr/initramfs_data.cpio".gz"
Make sure that we do strip off double quotes from CONFIG_INITRAMFS_COMPRESSION, since the Makefile tracks targets with unquoted suffixes.
Fixes 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile")
Signed-off-by Florian Fainelli <[email protected]>
---
This is against v4.10-rc2, thanks!
 usr/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
usr/Makefile +0 -0
index 17a513268325..a9ae8b493e2b 100644

usr/Makefile line 11

Quoted 8 lines

usr/Makefile +1 -1

@@ -5,7 +5,7 @@
 klibcdirs:;
 PHONY += klibcdirs
 
-suffix_y = $(CONFIG_INITRAMFS_COMPRESSION)
+suffix_y = $(subst ",,$(CONFIG_INITRAMFS_COMPRESSION))
 AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/initramfs_data.cpio$(suffix_y)"
 
 # Generate builtin.o based on initramfs_data.o

Hello,

Thanks for the fix, when I tested the patch I checked that the correct file was created and embedded but didn't check for spurious rebuilds.

I'm CCing Nicholas Piggin as he sent another (larger version) of the patch which also addresses a few other hacks in the Makefile, see "[PATCH] kbuild: Fix dependency checking for initramfs build". I haven't had time to check that second patch.

I have run the tests I ran for my own patches and they all passed, so feel free to add Reviewed-by and Tested-by if you want :)

Sincerely,
Francisco Blas Izquierdo Riera (klondike)

Florian Fainelli

author

2017-01-04 14:40 UTC #2 lore raw

Quoted 35 lines · klondike

Quoted 23 lines · Florian Fainelli
Commit 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile") makes suffix_y be a quote variable, which can be illustrated looking at the build output:
GEN usr/initramfs_data.cpio".gz"
Make sure that we do strip off double quotes from CONFIG_INITRAMFS_COMPRESSION, since the Makefile tracks targets with unquoted suffixes.
Fixes 35e669e1a254 ("initramfs: select builtin initram compression algorithm on KConfig instead of Makefile")
Signed-off-by Florian Fainelli <[email protected]>
---
This is against v4.10-rc2, thanks!
 usr/Makefile | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
usr/Makefile +0 -0
index 17a513268325..a9ae8b493e2b 100644
usr/Makefile line 11
Quoted 8 lines
usr/Makefile +1 -1
@@ -5,7 +5,7 @@
 klibcdirs:;
 PHONY += klibcdirs
 
-suffix_y = $(CONFIG_INITRAMFS_COMPRESSION)
+suffix_y = $(subst ",,$(CONFIG_INITRAMFS_COMPRESSION))
 AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE="usr/initramfs_data.cpio$(suffix_y)"
 
 # Generate builtin.o based on initramfs_data.o
Hello,

Quoted 7 lines

Thanks for the fix, when I tested the patch I checked that the correct file was created and embedded but didn't check for spurious rebuilds.
I'm CCing Nicholas Piggin as he sent another (larger version) of the patch which also addresses a few other hacks in the Makefile, see "[PATCH] kbuild: Fix dependency checking for initramfs build". I haven't had time to check that second patch.

Nick's patch [1] is a more elegant way to solve this, although it introduces more changes, so may be less suitable for a -rc fix? There are also some potential things that may or may not be relevant:

+AFLAGS_initramfs_data.o += -DINITRAMFS_IMAGE=usr/$(datafile_y)

INITRAMFS_IMAGE now gets defined without quotes, which may not be a problem, but does not seem necessary.

-targets := initramfs_data.cpio.gz initramfs_data.cpio.bz2 \
- initramfs_data.cpio.lzma initramfs_data.cpio.xz \
- initramfs_data.cpio.lzo initramfs_data.cpio.lz4 \
- initramfs_data.cpio
+targets := $(datafile_y)

Humm that could actually cause some additional rebuild if you kept your initramfs image unchanged, but you are switching between compression algorithms selection, but that may fall under the premature optimization case.

[1]: https://www.spinics.net/lists/linux-kbuild/msg14078.html

Signature · 1 line

Florian