All of lore.kernel.org
 help / color / mirror / Atom feed
From: Thomas Petazzoni <thomas.petazzoni@bootlin.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH 1/1] board/pc: improve image generation
Date: Thu, 11 Oct 2018 21:49:32 +0200	[thread overview]
Message-ID: <20181011214932.350ef220@windsurf> (raw)
In-Reply-To: <20180920143318.21284-1-greg@quimbo.fr>

Hello Gr?goire,

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

On Thu, 20 Sep 2018 16:33:19 +0200, Gr?goire Delattre wrote:
> From: Gr?goire Delattre <gregoire.delattre@gmail.com>
> 
> 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.

> +# 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
-- 
Thomas Petazzoni, CTO, Bootlin
Embedded Linux and Kernel engineering
https://bootlin.com

  reply	other threads:[~2018-10-11 19:49 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-20 14:33 [Buildroot] [PATCH 1/1] board/pc: improve image generation Grégoire Delattre
2018-10-11 19:49 ` Thomas Petazzoni [this message]
2018-10-11 22:24   ` [Buildroot] [PATCH v2 1/1] board/pc: ensure grub.cfg is copied to target filesystem Grégoire Delattre
2018-10-21 23:58     ` Matthew Weber
2018-12-31 17:48     ` Thomas Petazzoni
2019-01-23 15:19     ` Peter Korsgaard
     [not found]   ` <20181011223946.cqtbipkwm2gyviiu@lois>
2018-10-12 11:04     ` [Buildroot] [PATCH 1/1] board/pc: improve image generation Thomas Petazzoni
2018-10-12 11:21       ` Grégoire Delattre

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20181011214932.350ef220@windsurf \
    --to=thomas.petazzoni@bootlin.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.