Re: [PATCH] grub-install: Use `GRUB_DISTRIBUTOR` value as EFI boot option title

[Mate Kukri]

We are going to be shipping something like this in Ubuntu as we would
like our boot entries to be called "Ubuntu" instead of "ubuntu".

I wonder if other users of GRUB would find this naming approach
better, I could gate this behind a command line option if the patch
is easier to accept that way.

A note here is that, the Windows platform efivar code should be fixed
if this getting merged, but my first attempt to do so ended in
frustration,
because it definitely does not behave the same way as Unix, namely:
- When looking for an entry to re-use, the name comparison is
case-sensitive prefix match as opposed to a case-insensitive substring
search.
- The Windows code just searches for an entry to re-use, or an empty
entry to fill, but unlike Unix, the existing entries with a matching
name are not deleted.

Mate

On Thu, Feb 1, 2024 at 9:29 AM Mate Kukri <mate.kukri@canonical.com> wrote:
>
> - Allow entries to be called things like "Debian" and "Ubuntu".
> - If there is no `GRUB_DISTRIBUTOR` assume it is "GRUB".
> - When deleting previous boot options, allow matching against both
>   the old "efi_distributor" value and the new "efi_title" one.
>
> Signed-off-by: Mate Kukri <mate.kukri@canonical.com>
> ---
>  grub-core/osdep/unix/platform.c | 13 +++++++++----
>  include/grub/util/install.h     |  3 ++-
>  util/grub-install.c             | 11 ++++++++---
>  3 files changed, 19 insertions(+), 8 deletions(-)
>
> diff --git a/grub-core/osdep/unix/platform.c b/grub-core/osdep/unix/platform.c
> index de712211c..7efeef8a7 100644
> --- a/grub-core/osdep/unix/platform.c
> +++ b/grub-core/osdep/unix/platform.c
> @@ -134,7 +134,8 @@ grub_install_remove_efi_entries_by_distributor (const 
> char *efi_distributor)
>  int
>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>                            const char *efifile_path,
> -                          const char *efi_distributor)
> +                          const char *efi_distributor,
> +                          const char *efi_title)
>  {
>    const char * efidir_disk;
>    int efidir_part;
> @@ -153,8 +154,12 @@ grub_install_register_efi (grub_device_t efidir_grub_dev,
>  #ifdef __linux__
>    grub_util_exec ((const char * []){ "modprobe", "-q", "efivars", NULL });
>  #endif
> -  /* Delete old entries from the same distributor.  */
> +  /* Delete entries named with the previous "efi_distributor" scheme.  */
>    ret = grub_install_remove_efi_entries_by_distributor (efi_distributor);
> +  if (ret)
> +    return ret;
> +  /* Alse delete previous entries with the "efi_title" scheme.  */
> +  ret = grub_install_remove_efi_entries_by_distributor (efi_title);
>    if (ret)
>      return ret;
>
> @@ -164,13 +169,13 @@ grub_install_register_efi (grub_device_t 
> efidir_grub_dev,
>      ret = grub_util_exec ((const char * []){ "efibootmgr", "-q",
>           "-c", "-d", efidir_disk,
>           "-p", efidir_part_str, "-w",
> -         "-L", efi_distributor, "-l",
> +         "-L", efi_title, "-l",
>           efifile_path, NULL });
>    else
>      ret = grub_util_exec ((const char * []){ "efibootmgr",
>           "-c", "-d", efidir_disk,
>           "-p", efidir_part_str, "-w",
> -         "-L", efi_distributor, "-l",
> +         "-L", efi_title, "-l",
>           efifile_path, NULL });
>    free (efidir_part_str);
>    return ret;
> diff --git a/include/grub/util/install.h b/include/grub/util/install.h
> index 35cf17a8d..f1cc9f274 100644
> --- a/include/grub/util/install.h
> +++ b/include/grub/util/install.h
> @@ -227,7 +227,8 @@ grub_install_get_default_x86_platform (void);
>  int
>  grub_install_register_efi (grub_device_t efidir_grub_dev,
>                            const char *efifile_path,
> -                          const char *efi_distributor);
> +                          const char *efi_distributor,
> +                          const char *efi_title);
>
>  void
>  grub_install_register_ieee1275 (int is_prep, const char *install_device,
> diff --git a/util/grub-install.c b/util/grub-install.c
> index 7dc5657bb..c67f28873 100644
> --- a/util/grub-install.c
> +++ b/util/grub-install.c
> @@ -847,7 +847,7 @@ int
>  main (int argc, char *argv[])
>  {
>    int is_efi = 0;
> -  const char *efi_distributor = NULL;
> +  const char *efi_distributor = NULL, *efi_title = NULL;
>    const char *efi_file = NULL;
>    char **grub_devices;
>    grub_fs_t grub_fs;
> @@ -1140,6 +1140,9 @@ main (int argc, char *argv[])
>        */
>        char *t;
>        efi_distributor = bootloader_id;
> +      efi_title = config.grub_distributor;
> +      if (!efi_title || efi_title[0] == '\0')
> +       efi_title = "GRUB";
>        if (removable)
>         {
>           /* The specification makes stricter requirements of removable
> @@ -1958,7 +1961,8 @@ main (int argc, char *argv[])
>               int ret;
>               ret = grub_install_register_efi (efidir_grub_dev,
>                                                
> "\\System\\Library\\CoreServices",
> -                                              efi_distributor);
> +                                              efi_distributor,
> +                                              efi_title);
>               if (ret)
>                 grub_util_error (_("efibootmgr failed to register the boot 
> entry: %s"),
>                                  strerror (ret));
> @@ -2006,7 +2010,8 @@ main (int argc, char *argv[])
>                           (part ? ",": ""), (part ? : ""));
>           grub_free (part);
>           ret = grub_install_register_efi (efidir_grub_dev,
> -                                          efifile_path, efi_distributor);
> +                                          efifile_path, efi_distributor,
> +                                          efi_title);
>           if (ret)
>             grub_util_error (_("efibootmgr failed to register the boot entry: 
> %s"),
>                              strerror (ret));
> --
> 2.39.2
>