On Tue, Jun 26, 2018 at 4:33 PM, Peter Jones <pjones@xxxxxxxxxx> wrote: > On Tue, Jun 26, 2018 at 03:46:59PM +0200, Javier Martinez Canillas wrote: >> > That raises two questions: >> > 1. Why isn't just the bls-snippet filename used as the key? It's >> > necessarily unique and should be usable for the purpose of uniquely >> > identifying the boot entry without creating a separate field. >> >> I'll let Peter answer this question since he wrote the grub2 >> implementation. Doing this will be pretty trivial, but in that case we >> should also sort using the filenames if the id field isn't defined. > > I don't know that I have a specific reason in mind when I wrote that bit > of code. Most likely the real answer is: because when I realized we > have to sort them, I was writing parser code not directory iterating > code. > > Waaaaay back when the config files were on VFAT on UEFI machines, it > also meant I didn't have to even worry about the (entirely likely) > situation arising where I have to re-implement this with some other API > where I don't know if I'm going to get long filenames or not. > > For me, having it in the file seems more natural while I'm staring at > the file with my eyes, since that's where I'm reading other values used > in the same context. But I see the point being made as well. Maybe the > thing to do here is prefer "id" if it's in the file, but use the > filename if it isn't, and to disambiguate collisions. > >> This is already the case for Petitboot since the maintainer asked for >> the filename to be used as the key as well. For zipl the key is the >> version field because currently the kernel version is used as the >> zipl.conf section name and the maintainer wanted to keep the same >> behavior with BLS. > > Does this effectively mean we just have a different resolution order > there? Like are we comparing id and linux if version collides, or just > not using them? > Sorry, I should had been more clear on this point. Both Petitboot and zipl sort the entries using the filename and the strverscmp(3) function. The fields are not used for sorting there since the filenames should be unique. The difference I mentioned was that Petitboot uses the filename as the Petitboot boot entry ID and zipl uses the version field as the zipl boot entry ID. For Petitboot there can't be duplicates since the filenames are used, for zipl it will fail to parse the BLS if there are two snippets with the same version, but that's already the case without BLS if two zipl boot sections have the same name, so we won't change that behavior. >> Sorting using the filename won't work with ostree though. The BLS >> snippets are named ostree-$ID-$VARIANT_ID-$index.conf but the entries >> are sorted using the version field, and the index is the inverse of >> the version. >> >> One option is to change ostree so the BLS snippets are named >> ostree-$ID-$VARIANT_ID-$version.conf, but I don't know if there's a >> reason to have the index there. Another option is to give more >> precedence to the version field and only sort using the filenames if >> neither the id nor the version fields are defined. >> >> > 2. Why is "id" supposed to be sortable? What sorting would grub2 do >> > with it? >> >> It's sortable so grub2 can display the boot menu entries in the >> correct order. This is also answered in the Changes page: >> >> id - provides us with a sort key, sorted using rpm's version >> comparison function. Generally of the form: >> fedora-20180530145228-4.16.13-300.fc28.x86_64 > > Put another way: if you don't have a saved_entry set in grub.cfg, or if > it's saved_entry=0, if we don't have a sort field then you're going to > get whatever comes out of your filesystem's directory storage algorithm > first. Sometimes that's a linked-list that may or may not be sorted, > sometimes it's a b+tree, sometimes it's a hash bucket. It's kind of > hard to think anyone wants that behavior. > >> It attempts to sort using the id field, and if this isn't defined >> other fields are used as fallback in the following order: version, >> title, linux. > > Yeah, so maybe we want to make that: > > version, id, <config file name>, title, linux > Yes, although it probably should be: <config file name>, id, version, title, linux That way all bootloaders will default to the filename to sort the entries. That only leaves us with ostree since its BLS filenames can't be used for sorting. Colin, do you think that it may be feasible to change the ostree BLS filenames so the trailing number is the version instead of the index? Best regards, Javier _______________________________________________ devel mailing list -- devel@xxxxxxxxxxxxxxxxxxxxxxx To unsubscribe send an email to devel-leave@xxxxxxxxxxxxxxxxxxxxxxx Fedora Code of Conduct: https://getfedora.org/code-of-conduct.html List Guidelines: https://fedoraproject.org/wiki/Mailing_list_guidelines List Archives: https://lists.fedoraproject.org/archives/list/devel@xxxxxxxxxxxxxxxxxxxxxxx/message/P24M437GSWOF2EOSEMPN43CYGXLYOWP3/
