board/pc: improve image generation

PATCH New Grégoire Delattre 4 participants 8 messages Sep 20, 2018 – Jan 23, 2019 1 message not in the archive

lore

Share & tools

lore

Grégoire Delattre

author

2018-09-20 14:33 UTC #0 lore raw

PATCH 1/1 board/pc: improve image generation

From: Gr?goire Delattre <[email protected]>

Copy the grub configuration files before creating the filesystem image.

Signed-off-by Gr?goire Delattre <[email protected]>

---
 board/pc/post-build.sh           | 12 ++++++++++++
 board/pc/post-image.sh           | 14 --------------
 board/pc/pre-genimage.sh         |  8 ++++++++
 configs/pc_x86_64_bios_defconfig |  3 ++-
 configs/pc_x86_64_efi_defconfig  |  3 ++-
 5 files changed, 24 insertions(+), 16 deletions(-)
 create mode 100755 board/pc/post-build.sh
 delete mode 100755 board/pc/post-image.sh
 create mode 100755 board/pc/pre-genimage.sh

board/pc/post-build.sh new file +12 -0

new file mode 100755
index 0000000000..55f4cf1a96
@@ -0,0 +1,12 @@
+#!/bin/sh
+
+set -e
+
+BOARD_DIR=$(dirname "$0")
+
+# Detect boot strategy, EFI or BIOS
+if [ -f "$BINARIES_DIR/efi-part/startup.nsh" ]; then
+  cp -f "$BOARD_DIR/grub-efi.cfg" "$BINARIES_DIR/efi-part/EFI/BOOT/grub.cfg"
+else
+  cp -f "$BOARD_DIR/grub-bios.cfg" "$TARGET_DIR/boot/grub/grub.cfg"
+fi

board/pc/post-image.sh deleted +0 ~~-14~~

deleted file mode 100755
index bdd0847ec5..0000000000
@@ -1,14 +0,0 @@
-#!/bin/sh
-
-BOARD_DIR="$(dirname $0)"
-
-# Detect boot strategy, EFI or BIOS
-if [ -f ${BINARIES_DIR}/efi-part/startup.nsh ]; then
-  cp -f ${BOARD_DIR}/grub-efi.cfg ${BINARIES_DIR}/efi-part/EFI/BOOT/grub.cfg
-else
-  cp -f ${BOARD_DIR}/grub-bios.cfg ${TARGET_DIR}/boot/grub/grub.cfg
-  # Copy grub 1st stage to binaries, required for genimage
-  cp -f ${HOST_DIR}/lib/grub/i386-pc/boot.img ${BINARIES_DIR}
-fi
-
-exit $?

board/pc/pre-genimage.sh new file +8 -0

new file mode 100755
index 0000000000..21066c0296
@@ -0,0 +1,8 @@
+#!/bin/sh
+
+set -e
+
+[ -f "$TARGET_DIR/boot/grub/grub.cfg" ] || exit 0
+
+# Copy grub 1st stage to binaries, required for genimage
+cp -f "$HOST_DIR/lib/grub/i386-pc/boot.img" "$BINARIES_DIR"

configs/pc_x86_64_bios_defconfig +2 -1

index 47787bc118..8c3e8994d9 100644
@@ -19,7 +19,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/pre-genimage.sh support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-bios.cfg"
 
 # Linux headers same as kernel, a 4.13 series

configs/pc_x86_64_efi_defconfig +2 -1

index 67ba5bcf5b..6a93eacfa3 100644
@@ -22,7 +22,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-efi.cfg"
 
 # Linux headers same as kernel, a 4.13 series

Signature · 1 line

2.19.0

Thomas Petazzoni

2018-10-11 19:49 UTC #1 lore raw

PATCH 1/1 board/pc: improve image generation

Hello Gr?goire,

Thanks a lot for this patch, sorry for the slow answer.

Quoted 4 lines · Gr?goire Delattre

From: Gr?goire Delattre <[email protected]>
Copy the grub configuration files before creating the filesystem image.

I think the commit title is a bit vague, "improve" doesn't mean much and in fact, your patch fixes a real bug in this defconfig!

So perhaps:

board/pc: ensure grub.cfg is copied to target filesystem

and then in the commit log, explain that the grub.cfg file was copied to TARGET_DIR in a post-image hook, i.e after the filesystem image has been generated, and that in practice it worked because the board/pc/grub-bios.cfg file is the same as boot/grub2/grub.cfg, which *is* copied to TARGET_DIR as part of the grub2 build process.

Some more comments below.

board/pc/post-build.sh line 11

Quoted 5 lines

board/pc/post-build.sh new file +5 -0

+# Detect boot strategy, EFI or BIOS
+if [ -f "$BINARIES_DIR/efi-part/startup.nsh" ]; then
+  cp -f "$BOARD_DIR/grub-efi.cfg" "$BINARIES_DIR/efi-part/EFI/BOOT/grub.cfg"
+else
+  cp -f "$BOARD_DIR/grub-bios.cfg" "$TARGET_DIR/boot/grub/grub.cfg"

I think you could copy the boot.img to ${BINARIES_DIR} here as well. A post-build script is called after all packages have been built, so ${HOST_DIR}/lib/grub/i386-pc/boot.img already exists. This would avoid the need for the pre-genimage.sh.

Could you rework those minor details and send an updated patch?

Thanks a lot for spotting this mistake!

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Grégoire Delattre

author

2018-10-11 22:24 UTC #2 lore raw

PATCH v2 1/1 board/pc: ensure grub.cfg is copied to target filesystem

From: Gr?goire Delattre <[email protected]>

Before this commit, the grub configuration file was copied to the TARGET_DIR in a post-image hook, after the filesystem has been generated. It was kinda working because the board/pc's grub configuration and the default one are the same and the later was copied during the build process of the grub2 package.

This commit ensures the custom board/pc grub configuration is copied at the right time.

Signed-off-by Gr?goire Delattre <[email protected]>

---
 board/pc/post-build.sh           | 15 +++++++++++++++
 board/pc/post-image.sh           | 14 --------------
 configs/pc_x86_64_bios_defconfig |  3 ++-
 configs/pc_x86_64_efi_defconfig  |  3 ++-
 4 files changed, 19 insertions(+), 16 deletions(-)
 create mode 100755 board/pc/post-build.sh
 delete mode 100755 board/pc/post-image.sh

board/pc/post-build.sh new file +15 -0

new file mode 100755
index 0000000000..552d488160
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+set -e
+
+BOARD_DIR=$(dirname "$0")
+
+# Detect boot strategy, EFI or BIOS
+if [ -f "$BINARIES_DIR/efi-part/startup.nsh" ]; then
+    cp -f "$BOARD_DIR/grub-efi.cfg" "$BINARIES_DIR/efi-part/EFI/BOOT/grub.cfg"
+else
+    cp -f "$BOARD_DIR/grub-bios.cfg" "$TARGET_DIR/boot/grub/grub.cfg"
+
+    # Copy grub 1st stage to binaries, required for genimage
+    cp -f "$HOST_DIR/lib/grub/i387-pc/boot.img" "$BINARIES_DIR"
+fi

board/pc/post-image.sh deleted +0 ~~-14~~

deleted file mode 100755
index bdd0847ec5..0000000000
@@ -1,14 +0,0 @@
-#!/bin/sh
-
-BOARD_DIR="$(dirname $0)"
-
-# Detect boot strategy, EFI or BIOS
-if [ -f ${BINARIES_DIR}/efi-part/startup.nsh ]; then
-  cp -f ${BOARD_DIR}/grub-efi.cfg ${BINARIES_DIR}/efi-part/EFI/BOOT/grub.cfg
-else
-  cp -f ${BOARD_DIR}/grub-bios.cfg ${TARGET_DIR}/boot/grub/grub.cfg
-  # Copy grub 1st stage to binaries, required for genimage
-  cp -f ${HOST_DIR}/lib/grub/i386-pc/boot.img ${BINARIES_DIR}
-fi
-
-exit $?

configs/pc_x86_64_bios_defconfig +2 -1

index 1cfc0e424f..59e39a5ae7 100644
@@ -19,7 +19,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-bios.cfg"
 
 # Linux headers same as kernel, a 4.18 series

configs/pc_x86_64_efi_defconfig +2 -1

index bc99ca755b..b4190b9575 100644
@@ -22,7 +22,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-efi.cfg"
 
 # Linux headers same as kernel, a 4.18 series

Signature · 1 line

2.19.1

Matthew Weber

2018-10-21 23:58 UTC #3 lore raw

PATCH v2 1/1 board/pc: ensure grub.cfg is copied to target filesystem

Gr?goire,

Quoted 8 lines · Gr?goire Delattre

From: Gr?goire Delattre <[email protected]>
Before this commit, the grub configuration file was copied to the TARGET_DIR in a post-image hook, after the filesystem has been generated. It was kinda working because the board/pc's grub configuration and the default one are the same and the later was copied during the build process of the grub2 package.

Agree they are the same :-) My opinion is that we could simplify the number of files by removing the grub-bios.cfg from the board/pc folder and replace the copy step in the post script with a comment stating the default grub.cfg is being used. Then the move of the post script from a post-build to post-image isn't necessary required.

NOTE: The grub-efi.cfg has to be copied as we don't have a specific default for that cfg and it changes the path of the Linux image. Right now the same grub config is copied in from boot/grub2/grub.cfg for both types of boot.

Reviewed-by Matt Weber <[email protected]>

Quoted 86 lines

This commit ensures the custom board/pc grub configuration is copied at the right time.

Signed-off-by Gr?goire Delattre <[email protected]>

---
 board/pc/post-build.sh           | 15 +++++++++++++++
 board/pc/post-image.sh           | 14 --------------
 configs/pc_x86_64_bios_defconfig |  3 ++-
 configs/pc_x86_64_efi_defconfig  |  3 ++-
 4 files changed, 19 insertions(+), 16 deletions(-)
 create mode 100755 board/pc/post-build.sh
 delete mode 100755 board/pc/post-image.sh

board/pc/post-build.sh new file +15 -0

new file mode 100755
index 0000000000..552d488160
@@ -0,0 +1,15 @@
+#!/bin/sh
+
+set -e
+
+BOARD_DIR=$(dirname "$0")
+
+# Detect boot strategy, EFI or BIOS
+if [ -f "$BINARIES_DIR/efi-part/startup.nsh" ]; then
+    cp -f "$BOARD_DIR/grub-efi.cfg" "$BINARIES_DIR/efi-part/EFI/BOOT/grub.cfg"
+else
+    cp -f "$BOARD_DIR/grub-bios.cfg" "$TARGET_DIR/boot/grub/grub.cfg"
+
+    # Copy grub 1st stage to binaries, required for genimage
+    cp -f "$HOST_DIR/lib/grub/i387-pc/boot.img" "$BINARIES_DIR"
+fi

board/pc/post-image.sh deleted +0 ~~-14~~

deleted file mode 100755
index bdd0847ec5..0000000000
@@ -1,14 +0,0 @@
-#!/bin/sh
-
-BOARD_DIR="$(dirname $0)"
-
-# Detect boot strategy, EFI or BIOS
-if [ -f ${BINARIES_DIR}/efi-part/startup.nsh ]; then
-  cp -f ${BOARD_DIR}/grub-efi.cfg ${BINARIES_DIR}/efi-part/EFI/BOOT/grub.cfg
-else
-  cp -f ${BOARD_DIR}/grub-bios.cfg ${TARGET_DIR}/boot/grub/grub.cfg
-  # Copy grub 1st stage to binaries, required for genimage
-  cp -f ${HOST_DIR}/lib/grub/i386-pc/boot.img ${BINARIES_DIR}
-fi
-
-exit $?

configs/pc_x86_64_bios_defconfig +2 -1

index 1cfc0e424f..59e39a5ae7 100644
@@ -19,7 +19,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-bios.cfg"
 
 # Linux headers same as kernel, a 4.18 series

configs/pc_x86_64_efi_defconfig +2 -2

index bc99ca755b..b4190b9575 100644
@@ -22,7 +22,8 @@ BR2_TARGET_ROOTFS_EXT2=y
 BR2_TARGET_ROOTFS_EXT2_4=y
 BR2_TARGET_ROOTFS_EXT2_SIZE="120M"
 # BR2_TARGET_ROOTFS_TAR is not set
-BR2_ROOTFS_POST_IMAGE_SCRIPT="board/pc/post-image.sh support/scripts/genimage.sh"
+BR2_ROOTFS_POST_BUILD_SCRIPT="board/pc/post-build.sh"
+BR2_ROOTFS_POST_IMAGE_SCRIPT="support/scripts/genimage.sh"
 BR2_ROOTFS_POST_SCRIPT_ARGS="-c board/pc/genimage-efi.cfg"
 
 # Linux headers same as kernel, a 4.18 series
--

2.19.1

Quoted 4 lines

_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot

--
Matthew L Weber / Pr Software Engineer
Airborne Information Systems / RC Linux Secure Platforms MS 131-100, C Ave NE, Cedar Rapids, IA, 52498, USA www.rockwellcollins.com

Note: Any Export License Required Information and License Restricted Third Party Intellectual Property (TPIP) content must be encrypted and sent to matthew.weber at corp.rockwellcollins.com.

Thomas Petazzoni

Applied

2018-12-31 17:48 UTC #4 lore raw

PATCH v2 1/1 board/pc: ensure grub.cfg is copied to target filesystem

In reply to Grégoire Delattre

Hello,

Quoted 12 lines · Gr?goire Delattre

From: Gr?goire Delattre <[email protected]>
Before this commit, the grub configuration file was copied to the TARGET_DIR in a post-image hook, after the filesystem has been generated. It was kinda working because the board/pc's grub configuration and the default one are the same and the later was copied during the build process of the grub2 package.
This commit ensures the custom board/pc grub configuration is copied at the right time.
Signed-off-by Gr?goire Delattre <[email protected]>

Quoted 8 lines

---
 board/pc/post-build.sh           | 15 +++++++++++++++
 board/pc/post-image.sh           | 14 --------------
 configs/pc_x86_64_bios_defconfig |  3 ++-
 configs/pc_x86_64_efi_defconfig  |  3 ++-
 4 files changed, 19 insertions(+), 16 deletions(-)
 create mode 100755 board/pc/post-build.sh
 delete mode 100755 board/pc/post-image.sh

Applied to master, thanks.

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Peter Korsgaard

2019-01-23 15:19 UTC #5 lore raw

PATCH v2 1/1 board/pc: ensure grub.cfg is copied to target filesystem

In reply to Grégoire Delattre

Quoted 9 lines

Quoted 1 line
Quoted 1 line
Quoted 1 line
Quoted 1 line
"Gr?goire" == Gr?goire Delattre <[email protected]> writes:
From: Gr?goire Delattre <[email protected]> Before this commit, the grub configuration file was copied to the TARGET_DIR in a post-image hook, after the filesystem has been generated. It was kinda working because the board/pc's grub configuration and the default one are the same and the later was copied during the build process of the grub2 package.

Quoted 4 lines

This commit ensures the custom board/pc grub configuration is copied at the right time.
Signed-off-by Gr?goire Delattre <[email protected]>

Committed to 2018.02.x and 2018.11.x, thanks.

Signature · 1 line

Bye, Peter Korsgaard

A message that is not in the archive (sent off-list or not yet received): 20181011223946.cqtbipkwm2gyviiu@lois

Thomas Petazzoni

2018-10-12 11:04 UTC #7 lore raw

PATCH 1/1 board/pc: improve image generation

Hello,

(Please keep the mailing list in Cc, thanks!)

Quoted 7 lines · Gr?goire Delattre

Quoted 5 lines
and then in the commit log, explain that the grub.cfg file was copied to TARGET_DIR in a post-image hook, i.e after the filesystem image has been generated, and that in practice it worked because the board/pc/grub-bios.cfg file is the same as boot/grub2/grub.cfg, which *is* copied to TARGET_DIR as part of the grub2 build process.
I fixed the comment in the v2 patch.

Thanks, I had a quick look at your v2, and it looks good to me. If no other maintainer beats me at it, I'll try to merge it tonight.

Quoted 5 lines

I've changed that too.
I have two questions regarding the coding style of shell scripts in buildroot in general:
1) Should we use 2/4 spaces, tab ?

Good question, we have not standardized very well our shell scripts in general right now. Carlos Santos has started an effort to standardize our init scripts (see series at
http://patchwork.ozlabs.org/project/buildroot/list/?series=69419), I think we should use the same indentation.

Quoted 1 line

2) ${MY_VAR} vs "$MY_VAR" ?

Here as well, we don't have a very strict rule. My own personal rule is that I use $i for simple variable names, and I switch to ${foo_bar} when the variable name becomes more complex. But again, that's just my own personal taste. I believe Yann uses ${foo} all the time for example.

Best regards,

Thomas

Signature · 3 lines

Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

Grégoire Delattre

author

2018-10-12 11:21 UTC #8 lore raw

PATCH 1/1 board/pc: improve image generation

Hello,

Quoted 12 lines · Thomas Petazzoni

Hello,
(Please keep the mailing list in Cc, thanks!)
Quoted 6 lines · Gr?goire Delattre
Quoted 5 lines
and then in the commit log, explain that the grub.cfg file was copied to TARGET_DIR in a post-image hook, i.e after the filesystem image has been generated, and that in practice it worked because the board/pc/grub-bios.cfg file is the same as boot/grub2/grub.cfg, which *is* copied to TARGET_DIR as part of the grub2 build process.

Quoted 4 lines

Quoted 1 line
I fixed the comment in the v2 patch.
Thanks, I had a quick look at your v2, and it looks good to me. If no other maintainer beats me at it, I'll try to merge it tonight.

Perfect :)

Quoted 13 lines

Quoted 6 lines
I've changed that too.
I have two questions regarding the coding style of shell scripts in buildroot in general:
1) Should we use 2/4 spaces, tab ?
Good question, we have not standardized very well our shell scripts in general right now. Carlos Santos has started an effort to standardize our init scripts (see series at
http://patchwork.ozlabs.org/project/buildroot/list/?series=69419), I think we should use the same indentation.

Quoted 6 lines

Quoted 1 line
2) ${MY_VAR} vs "$MY_VAR" ?
Here as well, we don't have a very strict rule. My own personal rule is that I use $i for simple variable names, and I switch to ${foo_bar} when the variable name becomes more complex. But again, that's just my own personal taste. I believe Yann uses ${foo} all the time for example.

Thanks for the clarification.

Best regards,

Greg

Quoted 12 lines

Best regards,
Thomas
--
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com
_______________________________________________
buildroot mailing list
buildroot at busybox.net
http://lists.busybox.net/mailman/listinfo/buildroot