Re: [LEDE-DEV] [PATCH] x86: image: avoid duplicating some ops by using Image/Prepare/grub2

[Felix Fietkau]

On 2017-03-13 00:00, Rafał Miłecki wrote:
> From: Rafał Miłecki 
> 
> These commands aren't root filesystem image specific so it's OK to
> execute them just once. It saves some time when building images.
> 
> Signed-off-by: Rafał Miłecki 
> ---
>  target/linux/x86/image/Makefile | 50 
> +
>  1 file changed, 26 insertions(+), 24 deletions(-)
> 
> diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
> index f34788d67b..8a153e6392 100644
> --- a/target/linux/x86/image/Makefile
> +++ b/target/linux/x86/image/Makefile
> @@ -51,6 +51,32 @@ ifneq ($(CONFIG_TARGET_x86_xen_domu),)
>GRUB_ROOT = xen/xvda,msdos1
>  endif
>  
> +define Image/Prepare/grub2
> + $(INSTALL_DIR) $(KDIR)/root.grub/boot/grub $(KDIR)/grub2
> + $(CP) $(KDIR)/bzImage $(KDIR)/root.grub/boot/vmlinuz
> +
> + grub-mkimage \
> + -p /boot/grub \
> + -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
> + -o $(KDIR)/grub2/core.img \
> + -O i386-pc \
> + -c ./grub-early.cfg \
> + $(GRUB2_MODULES)
> + $(CP) $(STAGING_DIR_HOST)/lib/grub/i386-pc/*.img $(KDIR)/grub2/
> +
> + grub-mkimage \
> + -p /boot/grub \
> + -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
> + -o $(KDIR)/grub2/eltorito.img \
> + -O i386-pc \
> + -c ./grub-early.cfg \
> + $(GRUB2_MODULES_ISO)
> + cat \
> + $(STAGING_DIR_HOST)/lib/grub/i386-pc/cdboot.img \
> + $(KDIR)/grub2/eltorito.img \
> + > $(KDIR)/root.grub/boot/grub/eltorito.img
> +endef
But now you're executing the iso commands even if ISO images aren't
being used. Also, since you're making the code duplication painfully
obvious by moving the duplicated lines next to each other, maybe it's
time to address that as well ;)

- Felix


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev

[LEDE-DEV] [PATCH] x86: image: avoid duplicating some ops by using Image/Prepare/grub2

[Rafał Miłecki]

From: Rafał Miłecki 

These commands aren't root filesystem image specific so it's OK to
execute them just once. It saves some time when building images.

Signed-off-by: Rafał Miłecki 
---
 target/linux/x86/image/Makefile | 50 +
 1 file changed, 26 insertions(+), 24 deletions(-)

diff --git a/target/linux/x86/image/Makefile b/target/linux/x86/image/Makefile
index f34788d67b..8a153e6392 100644
--- a/target/linux/x86/image/Makefile
+++ b/target/linux/x86/image/Makefile
@@ -51,6 +51,32 @@ ifneq ($(CONFIG_TARGET_x86_xen_domu),)
   GRUB_ROOT = xen/xvda,msdos1
 endif
 
+define Image/Prepare/grub2
+   $(INSTALL_DIR) $(KDIR)/root.grub/boot/grub $(KDIR)/grub2
+   $(CP) $(KDIR)/bzImage $(KDIR)/root.grub/boot/vmlinuz
+
+   grub-mkimage \
+   -p /boot/grub \
+   -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
+   -o $(KDIR)/grub2/core.img \
+   -O i386-pc \
+   -c ./grub-early.cfg \
+   $(GRUB2_MODULES)
+   $(CP) $(STAGING_DIR_HOST)/lib/grub/i386-pc/*.img $(KDIR)/grub2/
+
+   grub-mkimage \
+   -p /boot/grub \
+   -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
+   -o $(KDIR)/grub2/eltorito.img \
+   -O i386-pc \
+   -c ./grub-early.cfg \
+   $(GRUB2_MODULES_ISO)
+   cat \
+   $(STAGING_DIR_HOST)/lib/grub/i386-pc/cdboot.img \
+   $(KDIR)/grub2/eltorito.img \
+   > $(KDIR)/root.grub/boot/grub/eltorito.img
+endef
+
 ifneq ($(CONFIG_GRUB_IMAGES),)
 
   BOOTOPTS:=$(call qstrip,$(CONFIG_GRUB_BOOTOPTS))
@@ -64,17 +90,6 @@ ifneq ($(CONFIG_GRUB_IMAGES),)
   endef
 
   define Image/Build/grub2
-   # left here because the image builder doesnt need these
-   $(INSTALL_DIR) $(KDIR)/root.grub/boot/grub $(KDIR)/grub2
-   $(CP) $(KDIR)/bzImage $(KDIR)/root.grub/boot/vmlinuz
-   grub-mkimage \
-   -p /boot/grub \
-   -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
-   -o $(KDIR)/grub2/core.img \
-   -O i386-pc \
-   -c ./grub-early.cfg \
-   $(GRUB2_MODULES)
-   $(CP) $(STAGING_DIR_HOST)/lib/grub/i386-pc/*.img $(KDIR)/grub2/
echo '(hd0) $(BIN_DIR)/$(IMG_PREFIX)-combined-$(1).img' > 
$(KDIR)/grub2/device.map
sed \
-e 's#@SERIAL_CONFIG@#$(strip $(GRUB_SERIAL_CONFIG))#g' \
@@ -101,19 +116,6 @@ define Image/Build/squashfs
 endef
 
 define Image/Build/iso
-   $(INSTALL_DIR) $(KDIR)/root.grub/boot/grub $(KDIR)/grub2
-   $(CP) $(KDIR)/bzImage $(KDIR)/root.grub/boot/vmlinuz
-   grub-mkimage \
-   -p /boot/grub \
-   -d $(STAGING_DIR_HOST)/lib/grub/i386-pc \
-   -o $(KDIR)/grub2/eltorito.img \
-   -O i386-pc \
-   -c ./grub-early.cfg \
-   $(GRUB2_MODULES_ISO)
-   cat \
-   $(STAGING_DIR_HOST)/lib/grub/i386-pc/cdboot.img \
-   $(KDIR)/grub2/eltorito.img \
-   > $(KDIR)/root.grub/boot/grub/eltorito.img
sed \
-e 's#@SERIAL_CONFIG@#$(strip $(GRUB_SERIAL_CONFIG))#g' \
-e 's#@TERMINAL_CONFIG@#$(strip $(GRUB_TERMINAL_CONFIG))#g' \
-- 
2.11.0


___
Lede-dev mailing list
Lede-dev@lists.infradead.org
http://lists.infradead.org/mailman/listinfo/lede-dev