| Message ID | 1433430330-2166-12-git-send-email-thomas.petazzoni@free-electrons.com |
|---|---|
| State | Superseded |
| Headers | show |
Thomas, all, On Thu, Jun 4, 2015 at 5:05 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > In order to support ISO9660 bootable images that rely on Grub 2, this > commit modifies thr Grub 2 makefile to generate and install an El s/thr/the/ > Torito image. Such an image is simply produced by concatenating the > cdboot.img provided by Grub 2, and the Grub 2 image generated by > Buildroot using grub-mkimage. > > Since this action is so simple and cost-free, we don't bother adding a > Grub 2 sub-option for that, and simply generate the El Torito image > unconditionally. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> Reviewed-by: Samuel Martin <s.martin49@gmail.com> Regards,
Thomas, All, On 2015-06-04 17:05 +0200, Thomas Petazzoni spake thusly: > In order to support ISO9660 bootable images that rely on Grub 2, this > commit modifies thr Grub 2 makefile to generate and install an El > Torito image. Such an image is simply produced by concatenating the > cdboot.img provided by Grub 2, and the Grub 2 image generated by > Buildroot using grub-mkimage. > > Since this action is so simple and cost-free, we don't bother adding a > Grub 2 sub-option for that, and simply generate the El Torito image > unconditionally. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > boot/grub2/grub2.mk | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk > index 697f0ad..dd04bb3 100644 > --- a/boot/grub2/grub2.mk > +++ b/boot/grub2/grub2.mk > @@ -76,6 +76,8 @@ define GRUB2_IMAGE_INSTALLATION > -p "$(GRUB2_PREFIX)" \ > $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ > $(GRUB2_BUILTIN_MODULES) > + cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img \ > + $(GRUB2_IMAGE) > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img Purely cosmetic comment: since we anyway have long lines, I'd rather see the two input file on the same line, like: cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img $(GRUB2_IMAGE) \ > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img Otherwise: Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr> Regards, Yann E. MORIN. > mkdir -p $(dir $(GRUB2_CFG)) > $(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_CFG) > endef > -- > 2.1.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Thomas, All, On 2015-06-04 17:05 +0200, Thomas Petazzoni spake thusly: > In order to support ISO9660 bootable images that rely on Grub 2, this > commit modifies thr Grub 2 makefile to generate and install an El > Torito image. Such an image is simply produced by concatenating the > cdboot.img provided by Grub 2, and the Grub 2 image generated by > Buildroot using grub-mkimage. > > Since this action is so simple and cost-free, we don't bother adding a > Grub 2 sub-option for that, and simply generate the El Torito image > unconditionally. > > Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> > --- > boot/grub2/grub2.mk | 2 ++ > 1 file changed, 2 insertions(+) > > diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk > index 697f0ad..dd04bb3 100644 > --- a/boot/grub2/grub2.mk > +++ b/boot/grub2/grub2.mk > @@ -76,6 +76,8 @@ define GRUB2_IMAGE_INSTALLATION > -p "$(GRUB2_PREFIX)" \ > $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ > $(GRUB2_BUILTIN_MODULES) > + cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img \ > + $(GRUB2_IMAGE) > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img In retrospect, I think you should install that in $(BINARIES_DIR), like is done for $(GRUB2_IMAGE), not in $(HOST_DIR). Regards, Yann E. MORIN. > mkdir -p $(dir $(GRUB2_CFG)) > $(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_CFG) > endef > -- > 2.1.0 > > _______________________________________________ > buildroot mailing list > buildroot@busybox.net > http://lists.busybox.net/mailman/listinfo/buildroot
Dear Yann E. MORIN, On Sat, 6 Jun 2015 01:09:11 +0200, Yann E. MORIN wrote: > > diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk > > index 697f0ad..dd04bb3 100644 > > --- a/boot/grub2/grub2.mk > > +++ b/boot/grub2/grub2.mk > > @@ -76,6 +76,8 @@ define GRUB2_IMAGE_INSTALLATION > > -p "$(GRUB2_PREFIX)" \ > > $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ > > $(GRUB2_BUILTIN_MODULES) > > + cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img \ > > + $(GRUB2_IMAGE) > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img > > In retrospect, I think you should install that in $(BINARIES_DIR), like > is done for $(GRUB2_IMAGE), not in $(HOST_DIR). Well, I had a reason to install it in HOST_DIR rather than BINARIES_DIR. Since I'm producing this image unconditionally, even if the user is not interested in ISO9660, I didn't want to clutter BINARIES_DIR with this file, which would appear as soon as the user enables grub2. So I preferred to install it together with all the other "internal" Grub images in HOST_DIR, and pick it up from there in the iso9660.mk logic. Now, if you think cluttering BINARIES_DIR with this file only useful for ISO booting as soon as Grub 2 is enabled is fine, then I'll happily change it. Thanks for the review! Thomas
Thomas, All, On 2015-06-06 02:57 +0200, Thomas Petazzoni spake thusly: > On Sat, 6 Jun 2015 01:09:11 +0200, Yann E. MORIN wrote: > > > > diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk > > > index 697f0ad..dd04bb3 100644 > > > --- a/boot/grub2/grub2.mk > > > +++ b/boot/grub2/grub2.mk > > > @@ -76,6 +76,8 @@ define GRUB2_IMAGE_INSTALLATION > > > -p "$(GRUB2_PREFIX)" \ > > > $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ > > > $(GRUB2_BUILTIN_MODULES) > > > + cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img \ > > > + $(GRUB2_IMAGE) > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img > > > > In retrospect, I think you should install that in $(BINARIES_DIR), like > > is done for $(GRUB2_IMAGE), not in $(HOST_DIR). > > Well, I had a reason to install it in HOST_DIR rather than > BINARIES_DIR. Since I'm producing this image unconditionally, even if > the user is not interested in ISO9660, I didn't want to clutter > BINARIES_DIR with this file, which would appear as soon as the user > enables grub2. So I preferred to install it together with all the other > "internal" Grub images in HOST_DIR, and pick it up from there in the > iso9660.mk logic. > > Now, if you think cluttering BINARIES_DIR with this file only useful > for ISO booting as soon as Grub 2 is enabled is fine, then I'll happily > change it. Well, yes, I'd prefer that's what we do, for these reasons: - first, that file is really small, - second, we already do that for other bootloaders (like rpi-firmware), - finally, we never said we only installed only the strictly minimum set of files, - if the user really does not want it, there's still the post-build scripts to remove that extra file. > Thanks for the review! My pleasure! :-) Regards, Yann E. MORIN.
diff --git a/boot/grub2/grub2.mk b/boot/grub2/grub2.mk index 697f0ad..dd04bb3 100644 --- a/boot/grub2/grub2.mk +++ b/boot/grub2/grub2.mk @@ -76,6 +76,8 @@ define GRUB2_IMAGE_INSTALLATION -p "$(GRUB2_PREFIX)" \ $(if $(GRUB2_BUILTIN_CONFIG),-c $(GRUB2_BUILTIN_CONFIG)) \ $(GRUB2_BUILTIN_MODULES) + cat $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/cdboot.img \ + $(GRUB2_IMAGE) > $(HOST_DIR)/usr/lib/grub/$(GRUB2_TUPLE)/grub-eltorito.img mkdir -p $(dir $(GRUB2_CFG)) $(INSTALL) -D -m 0644 boot/grub2/grub.cfg $(GRUB2_CFG) endef
In order to support ISO9660 bootable images that rely on Grub 2, this commit modifies thr Grub 2 makefile to generate and install an El Torito image. Such an image is simply produced by concatenating the cdboot.img provided by Grub 2, and the Grub 2 image generated by Buildroot using grub-mkimage. Since this action is so simple and cost-free, we don't bother adding a Grub 2 sub-option for that, and simply generate the El Torito image unconditionally. Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com> --- boot/grub2/grub2.mk | 2 ++ 1 file changed, 2 insertions(+)