Hello Michael,

(In reply to Michal Sekletar from comment #0)
> 
> Additional info:
> I realize that my setup is non-default, however, I still believe that grub
> package shouldn't delete $ESP/$MACHINE_ID directory even when package
> upgrade fails. Note that I was upgrading the same machine in the past using
> distro-sync to older Fedora releases and I didn't observe such major failure
> in the past.

The problem is that grub2 now also uses BLS snippets, but the kernel and initramfs images are in /boot instead of $ESP/$MACHINE_ID.

The dracut kernel-install scripts (/usr/lib/kernel/install.d/5?-dracut*.install) are also use by grub2 with BLS. And these scripts uses the existence of $ESP/$MACHINE_ID as an indication of where the initramfs has to be generated. If $ESP/$MACHINE_ID exists, then is generated there and if it doesn't exist, is generated in /boot.

When grub2 is used, the /usr/lib/kernel/install.d/20-grub.install script deletes that directory so the dracut kernel-install scripts create the initramfs in /boot as expected by grub2.

The 20-grub.install script is provided by the grub2-common package. So you can remove that package to prevent the directory to be deleted and keep the old behavior.

I think that sd-boot should be split as a subpackage and make it conflict with grub2.

Another option is to make "bootctl install" create the $ESP/$MACHINE_ID directory (and make "bootctl remove" delete it) instead of being created unconditionally by kernel-install (since it shouldn't be created if it won't be used).

That way $ESP/$MACHINE_ID will only exist when sd-boot was installed and it won't exist otherwise. So the kernel-install scripts will be compatible with both grub2 and sd-boot depending on which bootloader is used.

Hi,

removing a directory your package does not own and fully control is always very very risky, and something that should be thought over three times and discussed widely before being implemented. And removing a directory with stuff that is used to boot a machine is ... just something that should not happen. This should never have gone out anywhere in an update that users can install.

I prepared a patch to remove this part. Pull requests seem to be disabled in dist-git for grub2, so please pull the patch from https://src.fedoraproject.org/fork/zbyszek/rpms/grub2/c/72e91982880b8eb30a6854655cc075444a5c0e8a?branch=play-nice-with-others. Please also consider re-enabling pull requests.

> The 20-grub.install script is provided by the grub2-common package. So you can remove that package to prevent the directory to be deleted and keep the old behavior.

grub2-common is required by grubby, which in turn in Recommended by crypto-policies, so it's very likely that users can will install grub2-common through dependencies automatically. But even if it those dependencies were not there, just installing a package should not cause non-reversible changes to the system. Changing boot loader partition must require explicit user action. grub2-common must able to coexist with other boot loader managers.

> Another option is to make "bootctl install" create the $ESP/$MACHINE_ID directory (and make "bootctl remove" delete it) instead of being created unconditionally by kernel-install (since it shouldn't be created if it won't be used).

Yes, that's a reasonable solution. See https://github.com/systemd/systemd/pull/11971.

Proposed as a Blocker for 30-beta by Fedora user zbyszek using the blocker tracking app because:

 Installing grub2-common (which is among other things pulled in by crypto-policies which is pulled in by pretty much anything) causes all kernels to be removed from the boot partition upon next kernel update if the standard sd-boot layout is used.

sd-boot is not the default in Fedora, but it is used, including for multi-boot installations. I'm not sure if this bug violates any specific criterion, but since it gratuitously makes machines unbootable, I think this needs to be fixed for the beta images which are used by many end-users.

Hello Zbigniew,

(In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> Hi,
> 
> removing a directory your package does not own and fully control is always
> very very risky, and something that should be thought over three times and
> discussed widely before being implemented. And removing a directory with

This was indeed discussed between Peter, Harald, you and me a long time ago before it was implemented. The conclusion was that the 50-dracut.install script would check the existence of $BOOT/<machine-id> to decide the path where the initramfs would be created and 90-loaderentry.install to decide whether to generate BLS snippets in the ESP or not. But that directory was unconditionally created by kernel-install...

It's also complicated to coordinate these changes since the scripts are scattered across three different packages.

> stuff that is used to boot a machine is ... just something that should not
> happen. This should never have gone out anywhere in an update that users can
> install.
>

Yes, sorry about that. We didn't know that people using sd-boot were installing grub2 packages, so thought that the change would only affect grub2 users.
 
> I prepared a patch to remove this part. Pull requests seem to be disabled in
> dist-git for grub2, so please pull the patch from
> https://src.fedoraproject.org/fork/zbyszek/rpms/grub2/c/
> 72e91982880b8eb30a6854655cc075444a5c0e8a?branch=play-nice-with-others.

Thanks for the patch, I pulled it and did a grub2-2.02-73.fc30 build. Once there's a systemd build with your changes we can do a bodhi update that includes both packages.

> Please also consider re-enabling pull requests.
>

I'm not sure why it has been disabled, I'll ask Peter about it.
 
> > The 20-grub.install script is provided by the grub2-common package. So you can remove that package to prevent the directory to be deleted and keep the old behavior.
> 
> grub2-common is required by grubby, which in turn in Recommended by
> crypto-policies, so it's very likely that users can will install
> grub2-common through dependencies automatically. But even if it those
> dependencies were not there, just installing a package should not cause
> non-reversible changes to the system. Changing boot loader partition must
> require explicit user action. grub2-common must able to coexist with other
> boot loader managers.
>

Agreed.
 
> > Another option is to make "bootctl install" create the $ESP/$MACHINE_ID directory (and make "bootctl remove" delete it) instead of being created unconditionally by kernel-install (since it shouldn't be created if it won't be used).
> 
> Yes, that's a reasonable solution. See
> https://github.com/systemd/systemd/pull/11971.

Thanks a lot for working on that.

(In reply to Fedora Blocker Bugs Application from comment #3)
> Proposed as a Blocker for 30-beta by Fedora user zbyszek using the blocker
> tracking app because:
> 
>  Installing grub2-common (which is among other things pulled in by
> crypto-policies which is pulled in by pretty much anything) causes all
> kernels to be removed from the boot partition upon next kernel update if the
> standard sd-boot layout is used.
> 
> sd-boot is not the default in Fedora, but it is used, including for
> multi-boot installations. I'm not sure if this bug violates any specific
> criterion, but since it gratuitously makes machines unbootable, I think this
> needs to be fixed for the beta images which are used by many end-users.

Agreed that this should be a Blocker.

This could qualify under the cop-out qualification we rarely use:

"A bug is considered a Beta blocker bug if any of the following criteria are met: 
...
A bug in a Critical Path package that:

    Cannot be fixed with a future stable update
    Has a severity rating of high or greater and no reasonable workaround (see definition of severity and priority)"

This is actually the kind of case I think it makes sense to use that clause for. It doesn't exactly hit any of the explicit criteria, but it's such a big problem if you actually do run into it that it makes sense to me that we should accept it under that clause. The current severity rating of this bug is 'urgent', and I agree that is correct, and grub2 is a critical path package, and I don't see that there is a reasonable workaround once you have actually hit the problem (you *can* recover but it is not easy and I think we can call it not 'reasonable').

So I'd probably be +1 on that basis.

(In reply to Javier Martinez Canillas from comment #4)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #2)
> > Hi,

[snip]

>  
> > I prepared a patch to remove this part. Pull requests seem to be disabled in
> > dist-git for grub2, so please pull the patch from
> > https://src.fedoraproject.org/fork/zbyszek/rpms/grub2/c/
> > 72e91982880b8eb30a6854655cc075444a5c0e8a?branch=play-nice-with-others.
> 
> Thanks for the patch, I pulled it and did a grub2-2.02-73.fc30 build. Once
> there's a systemd build with your changes we can do a bodhi update that
> includes both packages.
> 

Thinking about this, having a kernel-install that doesn't create the directory will be enough for new installs, but no for upgrades. Because, people would already have an $ESP/<machine-id>/<kernel-version> directory created that was created by kernel-install and so the 20-grub.install plugin will just exit after your changes since the ${BOOT_DIR_ABS} directory would exist.

So I think that we should remove the directory if systemd-boot isn't installed as a workaround for F30 and F31 (so people can upgrade from F28 and F29) and the workaround can be removed for F32. I'm not sure what's the best approach to check if sd-boot is installed, maybe something liked the following?

diff --git a/20-grub.install b/20-grub.install
index f2de7760c43..04c9646d1b5 100755
--- a/20-grub.install
+++ b/20-grub.install
@@ -16,6 +16,16 @@ KERNEL_DIR="${KERNEL_IMAGE%/*}"
 
 MACHINE_ID=$KERNEL_INSTALL_MACHINE_ID
 
+# The kernel-install script always creates the ${BOOT_DIR_ABS} directory
+# but the presence of this directory is used by kernel-instals plugins to
+# decide where the kernel and initramfs images will be created and if BLS
+# files will be created or the ones shipped by the kernel package be used.
+#
+# On latest kernel-install, this directory is not created unconditionally
+# anymore and instead is managed by bootctl. But to allow upgrades to work
+# for now, as workaround remove ${BOOT_DIR_ABS} if sd-boot isn't installed.
+bootctl 2>&1 | grep -q "systemd-boot not installed" && rm -rf "${BOOT_DIR_ABS%/*}"
+
 # If ${BOOT_DIR_ABS} exists, some other boot loader is active.
 [[ -d "${BOOT_DIR_ABS}" ]] && exit 0


Zbigniew,

Do you have a better idea to solve the upgrade problem without the above workaround?

If we add this though, we should be able to push your grub2 changes without needing to wait for a systemd package that contains your latest kernel-install and bootctl changes. Since the 20-grub.install plugin will remove the directory, but only when sd-boot isn't installed.

+1 beta blocker per logic in comment 6.

> bootctl 2>&1 | grep -q "systemd-boot not installed" && rm -rf "${BOOT_DIR_ABS%/*}"

Hmm, this is pretty dangerous too. But ignoring this aspect for a moment, I don't think it's going to work at all.

I have a VM with grub2 (grub2-efi-x64-2.02-72.fc30.x86_64, systemd-241-2.gita09c170.fc30.x86_64).
$ bootctl 2>&1 | grep "systemd-boot not installed"
(nothing)

I'd prefer an rpm trigger to remove the directory, without a "permanent" change to 20-grub.install that will run every time kernel-install runs.

Something like this (untested):
%triggerun -- systemd < 242~rc2
if ! find /boot/efi/`cat /etc/machine-id` -type f; then
   rm -rf /boot/efi/`cat /etc/machine-id`
fi

(In reply to Zbigniew Jędrzejewski-Szmek from comment #9)
> > bootctl 2>&1 | grep -q "systemd-boot not installed" && rm -rf "${BOOT_DIR_ABS%/*}"
> 
> Hmm, this is pretty dangerous too. But ignoring this aspect for a moment, I
> don't think it's going to work at all.
>
> I have a VM with grub2 (grub2-efi-x64-2.02-72.fc30.x86_64,
> systemd-241-2.gita09c170.fc30.x86_64).
> $ bootctl 2>&1 | grep "systemd-boot not installed"
> (nothing)
>

Ok, I thought that would always report whether sd-boot was installed or not in the ESP.

What about checking if /boot/efi/EFI/systemd/systemd-bootx64.efi exists? I preferred using bootctl since the ESP may not be mounted in /boot/efi on some systems. Another option is to call efibootmgr and check if there's a boot entry whose file path points to systemd-bootx64.efi.
 
> I'd prefer an rpm trigger to remove the directory, without a "permanent"
> change to 20-grub.install that will run every time kernel-install runs.
> 
> Something like this (untested):
> %triggerun -- systemd < 242~rc2
> if ! find /boot/efi/`cat /etc/machine-id` -type f; then
>    rm -rf /boot/efi/`cat /etc/machine-id`
> fi

I'm not sure if this approach is going to work, since AFAIK the order in which dnf installs / upgrades packages is non-deterministic. So it may be that the grub2 package is updated first, then the kernel package and finally the systemd package. In that case the kernel will be installed in the incorrect path since /usr/lib/kernel/install.d/20-grub.install will check that ${BOOT_DIR_ABS} exists.

Also, the fact that this directory isn't empty doesn't mean that sd-boot (or other bootloader) is being used. For example, a user may had removed grubby by mistake, then did a kernel install (and so the kernel-install plugins assumed that wanted to use the sd-boot path), then install grubby again.

So in this case the directory may not be empty but still the user would want to install kernels in /boot instead of /boot/efi/$(cat machine-id).

> What about checking if /boot/efi/EFI/systemd/systemd-bootx64.efi exists?

This still relies on ESP being on /boot/efi. But we can check for "$(bootctl -p)/EFI/systemd/systemd-bootx64.efi".
I guess this should work. Can you prepare a grub rpm with that, so we can all test it?

(In reply to Zbigniew Jędrzejewski-Szmek from comment #11)
> > What about checking if /boot/efi/EFI/systemd/systemd-bootx64.efi exists?
> 
> This still relies on ESP being on /boot/efi. But we can check for "$(bootctl
> -p)/EFI/systemd/systemd-bootx64.efi".
> I guess this should work. Can you prepare a grub rpm with that, so we can
> all test it?

Thanks, I missed the bootctl -p option. Could you please test the following grub2-common-2.02-74.fc30.noarch.rpm scratch build?

https://kojipkgs.fedoraproject.org//work/tasks/1725/33611725/grub2-common-2.02-74.fc30.noarch.rpm

Discussed during the 2019-03-18 blocker review meeting: [1]

The decision to classify this bug as a "RejectedBlocker" and a "RejectedFreezeException" was made as this does not involve a default or supported bootloader, and the change required looks like it could break stuff badly if it goes wrong, we agree this is not a blocker for Beta and the risk of taking the change as an FE is too high.

[1] https://meetbot.fedoraproject.org/fedora-blocker-review/2019-03-18/f30-blocker-review.2019-03-18-16.03.txt

Zbigniew,

I've tested the grub2-common package shared in Comment 13 and it works with both grub2 and sd-boot. Please let me know if you are OK with it. One advantage is that this allows grub2 and systemd to be updated independently. For reference, following is the change after your suggestions:

diff --git a/20-grub.install b/20-grub.install
index f2de7760c43..256171cd3ba 100755
--- a/20-grub.install
+++ b/20-grub.install
@@ -16,6 +16,16 @@ KERNEL_DIR="${KERNEL_IMAGE%/*}"
 
 MACHINE_ID=$KERNEL_INSTALL_MACHINE_ID
 
+# The kernel-install script always creates the ${BOOT_DIR_ABS} directory
+# but the presence of this directory is used by kernel-instals plugins to
+# decide where the kernel and initramfs images will be created and if BLS
+# files will be created or the ones shipped by the kernel package be used.
+#
+# On latest kernel-install, this directory is not created unconditionally
+# anymore and instead is managed by bootctl. But to allow upgrades to work
+# for now, as workaround remove ${BOOT_DIR_ABS} if sd-boot isn't installed.
+[[ ! -e $(bootctl -p)/EFI/systemd/systemd-bootx64.efi ]] && rm -rf "${BOOT_DIR_ABS%/*}"
+
 # If ${BOOT_DIR_ABS} exists, some other boot loader is active.
 [[ -d "${BOOT_DIR_ABS}" ]] && exit 0

bootctl -p might fail. Maybe something like this:

bootdir="$(bootctl -p)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ]; then
     echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
     rm -rf "${BOOT_DIR_ABS%/*}"
fi

(In reply to Zbigniew Jędrzejewski-Szmek from comment #15)
> bootctl -p might fail. Maybe something like this:
> 
> bootdir="$(bootctl -p)"
> if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ]; then
>      echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see
> https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
>      rm -rf "${BOOT_DIR_ABS%/*}"
> fi

Looks good to me, I've also tested with sd-boot and grub2. Following is a new scratch build with that change:

https://kojipkgs.fedoraproject.org//work/tasks/841/33630841/grub2-common-2.02-74.fc30.noarch.rpm

(In reply to Javier Martinez Canillas from comment #16)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #15)
> > bootctl -p might fail. Maybe something like this:
> > 
> > bootdir="$(bootctl -p)"
> > if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ]; then
> >      echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see
> > https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
> >      rm -rf "${BOOT_DIR_ABS%/*}"
> > fi
> 
> Looks good to me, I've also tested with sd-boot and grub2. Following is a
> new scratch build with that change:

Actually, this will only work on an EFI install but fail on a legacy BIOS install. So probably something like following instead:

[ -d /sys/firmware/efi/efivars/ ] && bootdir="$(bootctl -p)"
if [ -z "$bootdir" ] || [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

That doesn't handle the case where something wrong goes with bootctl.
What about this:

bootdir="$(bootctl -p)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

(In reply to Zbigniew Jędrzejewski-Szmek from comment #18)
> That doesn't handle the case where something wrong goes with bootctl.
> What about this:
> 
> bootdir="$(bootctl -p)"
> if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
>    [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
>     echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see
> https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
>     rm -rf "${BOOT_DIR_ABS%/*}"
> fi

Right, I tried to avoid calling bootctl in non-EFI installs since that will always fail and print an error message. If you agree, I'll do a package update with the following:

bootdir="$(bootctl -p 2>/dev/null)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

BTW, will the systemd changes be in F30 final? It would be good if the above hack is only executed once during the upgrade.

This version looks OK.

> BTW, will the systemd changes be in F30 final?

Yes, the systemd changes will be in F30 final.

> It would be good if the above hack is only executed once during the upgrade.

Agreed. That's why I suggested using "%triggerun -- systemd < 242~rc2" instead of putting this in 20-grub.install.

(In reply to Zbigniew Jędrzejewski-Szmek from comment #20)
> This version looks OK.
>

Ok, thanks a lot for your help.
 
> > BTW, will the systemd changes be in F30 final?
> 
> Yes, the systemd changes will be in F30 final.
>

Great.
 
> > It would be good if the above hack is only executed once during the upgrade.
> 
> Agreed. That's why I suggested using "%triggerun -- systemd < 242~rc2"
> instead of putting this in 20-grub.install.

Yes, agreed. My only worry was that there isn't a dependency between systemd and grub2 packages so this could be an issue. But now thinking about it, it doesn't matter since we just need this to happen before the kernel package is installed and there is a dependency with systemd there.

So then we could do something like the following? (the snippet to figure out $BOOT_DIR_ABS was taken from kernel-install):

%triggerun -- systemd < 242~rc2

if [[ -f /etc/machine-id ]]; then
    read MACHINE_ID < /etc/machine-id
fi

if [[ -d /efi/loader/entries ]] || [[ -d /efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/loader/entries ]] || [[ -d /boot/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/efi/loader/entries ]] || [[ -d /boot/efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /efi; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /boot/efi; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
else
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
fi

bootdir="$(bootctl -p 2>/dev/null)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

LGTM. (The redundancy of both calculating $BOOT_DIR_ABS and $bootdir is pretty ugly, but bootctl -p doesn't support the
non-efi case, so I guess just duplicating the kernel-install logic is the safest solution.)

One more update:
please change '%triggerun -- systemd < 242~rc2' to '%triggerun -- systemd < 241-3'.

(I'm not sure when 242 will be released, and even if it is, if it will go in F30. So I'll just pull those systemd patches into the next systemd update.)

(In reply to Zbigniew Jędrzejewski-Szmek from comment #23)
> One more update:
> please change '%triggerun -- systemd < 242~rc2' to '%triggerun -- systemd <
> 241-3'.
>

Ok, updated version below:

%triggerun -- systemd < 241-3

if [[ -f /etc/machine-id ]]; then
    read MACHINE_ID < /etc/machine-id
fi

if [[ -d /efi/loader/entries ]] || [[ -d /efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/loader/entries ]] || [[ -d /boot/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/efi/loader/entries ]] || [[ -d /boot/efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /efi; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /boot/efi; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
else
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
fi

bootdir="$(bootctl -p 2>/dev/null)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

NOTE: I've tested that running this script on both EFI and non-EFI installs cleanups $BOOT/<machine_id> correctly, and only if sd-boot isn't installed. But I haven't tested upgrading to a new systemd package or doing a system wide upgrade since that would require a systemd package that has your kernel-install and bootctl changes.

> (I'm not sure when 242 will be released, and even if it is, if it will go in
> F30. So I'll just pull those systemd patches into the next systemd update.)

Great, I assume you will pull the %triggerun scritplet in the same change then. Also, when doing the bodhi update you will need to include the grub2-2.02-73.fc30 build that has your change for the 20-grub.install plugin.

Thanks a lot for your help and sorry again for the mess.

One last thought, should the scriptlet exit if there isn't a machine id?, i.e:

%triggerun -- systemd < 241-3

if [[ ! -f /etc/machine-id ]]; then
    read MACHINE_ID < /etc/machine-id
fi

if ! [[ $MACHINE_ID ]]; then
    exit 0
fi

if [[ -d /efi/loader/entries ]] || [[ -d /efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/loader/entries ]] || [[ -d /boot/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/efi/loader/entries ]] || [[ -d /boot/efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /efi; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /boot/efi; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
else
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
fi

bootdir="$(bootctl -p 2>/dev/null)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
    rm -rf "${BOOT_DIR_ABS%/*}"
fi

The $MACHINE_ID check is good. Also, there is no point in warning if the directory doesn't exist. So maybe:


%triggerun -- systemd < 241-3

if [[ ! -f /etc/machine-id ]]; then
    read MACHINE_ID < /etc/machine-id
fi

if ! [[ $MACHINE_ID ]]; then
    exit 0
fi

if [[ -d /efi/loader/entries ]] || [[ -d /efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/loader/entries ]] || [[ -d /boot/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
elif [[ -d /boot/efi/loader/entries ]] || [[ -d /boot/efi/$MACHINE_ID ]]; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /efi; then
    BOOT_DIR_ABS="/efi/$MACHINE_ID/$KERNEL_VERSION"
elif mountpoint -q /boot/efi; then
    BOOT_DIR_ABS="/boot/efi/$MACHINE_ID/$KERNEL_VERSION"
else
    BOOT_DIR_ABS="/boot/$MACHINE_ID/$KERNEL_VERSION"
fi

bootdir="$(bootctl -p 2>/dev/null)"
if [ -n "$bootdir" -a ! -e "$bootdir/EFI/systemd/systemd-bootx64.efi" ] ||
   [ -z "$bootdir" -a ! -d /sys/firmware/efi/efivars/ ]; then
    if [ -d "${BOOT_DIR_ABS%/*}" ]; then
        echo "WARNING: removing ${BOOT_DIR_ABS%/*}, see https://bugzilla.redhat.com/show_bug.cgi?id=1648907"
        rm -rf "${BOOT_DIR_ABS%/*}"
    fi
fi

(In reply to Zbigniew Jędrzejewski-Szmek from comment #26)
> The $MACHINE_ID check is good. Also, there is no point in warning if the
> directory doesn't exist. So maybe:
> 

Indeed, looks good to me.

Hello Zbigniew,

(In reply to Javier Martinez Canillas from comment #24)
> (In reply to Zbigniew Jędrzejewski-Szmek from comment #23)

[snip]

> then. Also, when doing the bodhi update you will need to include the
> grub2-2.02-73.fc30 build that has your change for the 20-grub.install plugin.
> 

I've made a new grub2-2.02-74.fc30 build [0] so please pick that one when doing the systemd bodhi update.

By the way, now that Beta has been delayed maybe we can propose again to get this bug as a Freeze Exception?

[0]: https://koji.fedoraproject.org/koji/buildinfo?buildID=1238129

I can bring it up at the meeting on Monday, but I'd guess people would still vote -1 to bootloader change for Beta at this point.

(In reply to Adam Williamson from comment #29)
> I can bring it up at the meeting on Monday, but I'd guess people would still
> vote -1 to bootloader change for Beta at this point.

I see, thanks for the clarification. Then I think that's not necessary to bring this again.

*** Bug 1693480 has been marked as a duplicate of this bug. ***

Hello Zbigniew,

I did a new build with a fix for another bug, but can't do an update without the systemd changes. So please pick grub2-2.02-75.fc30 instead:

https://koji.fedoraproject.org/koji/taskinfo?taskID=33804676

grub2-2.02-75.fc30 systemd-241-4.gitcbf14c9.fc30 has been submitted as an update to Fedora 30. https://bodhi.fedoraproject.org/updates/FEDORA-2019-616045ca76

grub2-2.02-75.fc30, systemd-241-4.gitcbf14c9.fc30 has been pushed to the Fedora 30 testing repository. If problems still persist, please make note of it in this bug report.
See https://fedoraproject.org/wiki/QA:Updates_Testing for
instructions on how to install test updates.
You can provide feedback for this update here: https://bodhi.fedoraproject.org/updates/FEDORA-2019-616045ca76

grub2-2.02-75.fc30, systemd-241-4.gitcbf14c9.fc30 has been pushed to the Fedora 30 stable repository. If problems still persist, please make note of it in this bug report.