boot: Change boot entry sorting

There are a few undesirable properties to how boot entries are
currently sorted.

First, it sorts by entry file name only, which may not correspond
to the title that is shown (for exmaple because it is prefixed by
machine-id). The file ending will also create unexpected ordering
("arch-lts.conf" would come before "arch.conf").

While the list is sorted alphabetically ascending, it is also
lower version/priority first, which is unintuitive. In particular,
a boot-counted entry that is bad (0 tries left) will be at the very
top.

Additionally, the Windows and Mac loaders should be sorted with
the rest of the loaders.

diff --git a/man/systemd-boot.xml b/man/systemd-boot.xml
index ceea368ef2470264e2c7d05bf1e4e2f0edc12d7c..d503f095488fdb5bf599f1081bcb2acd0a161984 100644 (file)
--- a/man/systemd-boot.xml
+++ b/man/systemd-boot.xml
@@ -501,10 +501,10 @@
     considered 'good' from then on.</para>
 
     <para>The boot menu takes the 'tries left' counter into account when sorting the menu entries: entries in 'bad'
-    state are ordered at the beginning of the list, and entries in 'good' or 'indeterminate' at the end. The user can
-    freely choose to boot any entry of the menu, including those already marked 'bad'. If the menu entry to boot is
-    automatically determined, this means that 'good' or 'indeterminate' entries are generally preferred (as the bottom
-    item of the menu is the one booted by default), and 'bad' entries will only be considered if there are no 'good' or
+    state are ordered towards the end of the list, and entries in 'good' or 'indeterminate' towards the beginning.
+    The user can freely choose to boot any entry of the menu, including those already marked 'bad'. If the menu entry
+    to boot is automatically determined, this means that 'good' or 'indeterminate' entries are generally preferred as
+    boot entries are tried in sort order, and 'bad' entries will only be considered if there are no 'good' or
     'indeterminate' entries left.</para>
 
     <para>The <citerefentry><refentrytitle>kernel-install</refentrytitle><manvolnum>8</manvolnum></citerefentry> kernel

diff --git a/src/boot/efi/boot.c b/src/boot/efi/boot.c
index fc346956df8ea91e3c5bd30640150c4b221ad753..189461974baa42e6ce0a5b4babb55cd8fc77462f 100644 (file)
--- a/src/boot/efi/boot.c
+++ b/src/boot/efi/boot.c
@@ -1492,7 +1492,6 @@ static void config_entry_add_from_file(
 
         entry->device = device;
         entry->id = xstrdup(file);
-        StrLwr(entry->id);
 
         config_add_entry(config, entry);
 
@@ -1603,13 +1602,24 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
         assert(a);
         assert(b);
 
-        /* Order entries that have no tries left to the beginning of the list */
+        /* Order entries that have no tries left towards the end of the list. They have
+         * proven to be bad and should not be selected automatically. */
         if (a->tries_left != 0 && b->tries_left == 0)
-                return 1;
-        if (a->tries_left == 0 && b->tries_left != 0)
                 return -1;
+        if (a->tries_left == 0 && b->tries_left != 0)
+                return 1;
+
+        r = strcasecmp_ptr(a->title ?: a->id, b->title ?: b->id);
+        if (r != 0)
+                return r;
 
-        r = strverscmp_improved(a->id, b->id);
+        /* Sort by machine id now so that different installations don't interleave their versions. */
+        r = strcasecmp_ptr(a->machine_id, b->machine_id);
+        if (r != 0)
+                return r;
+
+        /* Reverse version comparison order so that higher versions are preferred. */
+        r = strverscmp_improved(b->version, a->version);
         if (r != 0)
                 return r;
 
@@ -1617,19 +1627,20 @@ static INTN config_entry_compare(const ConfigEntry *a, const ConfigEntry *b) {
             b->tries_left == UINTN_MAX)
                 return 0;
 
-        /* If both items have boot counting, and otherwise are identical, put the entry with more tries left last */
+        /* If both items have boot counting, and otherwise are identical, put the entry with more tries left first */
         if (a->tries_left > b->tries_left)
-                return 1;
-        if (a->tries_left < b->tries_left)
                 return -1;
+        if (a->tries_left < b->tries_left)
+                return 1;
 
         /* If they have the same number of tries left, then let the one win which was tried fewer times so far */
         if (a->tries_done < b->tries_done)
-                return 1;
-        if (a->tries_done > b->tries_done)
                 return -1;
+        if (a->tries_done > b->tries_done)
+                return 1;
 
-        return 0;
+        /* As a last resort, use the id (file name). */
+        return strverscmp_improved(a->id, b->id);
 }
 
 static UINTN config_entry_find(Config *config, const CHAR16 *needle) {
@@ -1638,7 +1649,7 @@ static UINTN config_entry_find(Config *config, const CHAR16 *needle) {
         if (!needle)
                 return IDX_INVALID;
 
-        for (INTN i = config->entry_count - 1; i >= 0; i--)
+        for (UINTN i = 0; i < config->entry_count; i++)
                 if (MetaiMatch(config->entries[i]->id, (CHAR16*) needle))
                         return i;
 
@@ -1673,9 +1684,8 @@ static void config_default_entry_select(Config *config) {
                 return;
         }
 
-        /* select the last suitable entry */
-        i = config->entry_count;
-        while (i--) {
+        /* Select the first suitable entry. */
+        for (i = 0; i < config->entry_count; i++) {
                 if (config->entries[i]->type == LOADER_AUTO || config->entries[i]->call)
                         continue;
                 config->idx_default = i;
@@ -1830,8 +1840,6 @@ static ConfigEntry *config_entry_add_loader(
                 .tries_left = UINTN_MAX,
         };
 
-        StrLwr(entry->id);
-
         config_add_entry(config, entry);
         return entry;
 }
@@ -2326,12 +2334,14 @@ static void config_load_all_entries(
         /* Similar, but on any XBOOTLDR partition */
         config_load_xbootldr(config, loaded_image->DeviceHandle);
 
+        /* Add these now, so they get sorted with the rest. */
+        config_entry_add_osx(config);
+        config_entry_add_windows(config, loaded_image->DeviceHandle, root_dir);
+
         /* sort entries after version number */
         sort_pointer_array((void **) config->entries, config->entry_count, (compare_pointer_func_t) config_entry_compare);
 
         /* if we find some well-known loaders, add them to the end of the list */
-        config_entry_add_osx(config);
-        config_entry_add_windows(config, loaded_image->DeviceHandle, root_dir);
         config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, NULL,
                                      L"auto-efi-shell", 's', L"EFI Shell", L"\\shell" EFI_MACHINE_TYPE_NAME ".efi");
         config_entry_add_loader_auto(config, loaded_image->DeviceHandle, root_dir, loaded_image_path,