| Message ID | 20211103214718.1149331-1-aduskett@gmail.com |
|---|---|
| State | Changes Requested |
| Headers | show |
| Series | [v2,1/1] package/mender-grubenv: fix grub module checks | expand |
Hello Adam, On Wed, 3 Nov 2021 14:47:18 -0700 Adam Duskett <aduskett@gmail.com> wrote: > diff --git a/package/mender-grubenv/mender-grubenv.mk b/package/mender-grubenv/mender-grubenv.mk > index 07df25512c..ce63323f2a 100644 > --- a/package/mender-grubenv/mender-grubenv.mk > +++ b/package/mender-grubenv/mender-grubenv.mk > @@ -31,9 +31,15 @@ MENDER_GRUBENV_DEFINES = \ > # These grub modules must be built in for the grub scripts to work properly. > # Without them, the system will not boot. > MENDER_GRUBENV_MANDATORY_MODULES=loadenv hashsum echo halt gcry_sha256 test regexp > +ifneq ($(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI),) > MENDER_GRUBENV_MODULES_MISSING = \ > - $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES)),\ > + $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI)),\ > $(MENDER_GRUBENV_MANDATORY_MODULES)) > +else > +MENDER_GRUBENV_MODULES_MISSING = \ > + $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_PC)),\ > + $(MENDER_GRUBENV_MANDATORY_MODULES)) > +endif Now that I further think of this, I think this is still not correct. Indeed, one of the major change recently brought to the grub2 package is the ability in a single configuration to have grub support for x86 PC (legacy BIOS) x86 EFI. So in fact you can have BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI and BR2_TARGET_GRUB2_BUILTIN_MODULES_PC non-empty at the same time. So if you want to do the right thing, you would have to check that BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI doesn't miss any of the mandatory modules if one of the EFI grub2 variant is selected *and* check that BR2_TARGET_GRUB2_BUILTIN_MODULES_PC doesn't miss any of the mandatory modules if one of the legacy BIOS grub2 variant is selected. Thomas
diff --git a/package/mender-grubenv/mender-grubenv.mk b/package/mender-grubenv/mender-grubenv.mk index 07df25512c..ce63323f2a 100644 --- a/package/mender-grubenv/mender-grubenv.mk +++ b/package/mender-grubenv/mender-grubenv.mk @@ -31,9 +31,15 @@ MENDER_GRUBENV_DEFINES = \ # These grub modules must be built in for the grub scripts to work properly. # Without them, the system will not boot. MENDER_GRUBENV_MANDATORY_MODULES=loadenv hashsum echo halt gcry_sha256 test regexp +ifneq ($(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI),) MENDER_GRUBENV_MODULES_MISSING = \ - $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES)),\ + $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI)),\ $(MENDER_GRUBENV_MANDATORY_MODULES)) +else +MENDER_GRUBENV_MODULES_MISSING = \ + $(filter-out $(call qstrip,$(BR2_TARGET_GRUB2_BUILTIN_MODULES_PC)),\ + $(MENDER_GRUBENV_MANDATORY_MODULES)) +endif ifeq ($(BR2_PACKAGE_MENDER_GRUBENV)$(BR_BUILDING),yy) ifneq ($(MENDER_GRUBENV_MODULES_MISSING),)
Commit 3efb5e31fc05705ce3c46b1f0ec031978a5cfab6 broke mender-grubenv by splititng up BR2_TARGET_GRUB2_BUILTIN_MODULES and BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI. Indeed, when a user now builds a system with EFI, the MENDER_GRUBENV_MODULES_MISSING list always returns a full list of grub modules, resulting in the error condition on line 46 to trigger. In addition, BR2_TARGET_GRUB2_BUILTIN_MODULES has been renamed to BR2_TARGET_GRUB2_BUILTIN_MODULES_PC, so this is changed in mender-grubenv.mk as well. Add an ifeq that checks if BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI is not empty, and checks the required modules properly if it is not. Signed-off-by: Adam Duskett <aduskett@gmail.com> --- changes v1 -> v2: - Change ifeq ($(BR2_TARGET_GRUB2_X86_64_EFI),y) to ifneq ($(BR2_TARGET_GRUB2_BUILTIN_MODULES_EFI),) to cover all grub2 efi scenarios. (Thomas) - Change BR2_TARGET_GRUB2_BUILTIN_MODULES to BR2_TARGET_GRUB2_BUILTIN_MODULES_PC (thomas) package/mender-grubenv/mender-grubenv.mk | 8 +++++++- 1 file changed, 7 insertions(+), 1 deletion(-)