s-n-ushakov / rename-efi-entry

A Bash script to rename EFI boot entries
BSD 2-Clause "Simplified" License
84 stars 15 forks source link

block device name match: add nvme (and mmc) support #3

Closed jericdeleon closed 4 years ago

jericdeleon commented 4 years ago

I have an NVMe drive, and NVMe block devices use a different naming format (see related Arch doc for more info). With this, NVMe-based boot drives are now supported.

I tried to include SCSI- and MMC-based formats, and while they work with the regex match, I did not run them on a real setup (I can only vouch for NVMe-based match on real hardware).

Please let me know if any improvements can be included, thanks!

s-n-ushakov commented 4 years ago

Hi Jeric, and first thank you for sharing your improvement :)

There is still a small detail that makes me hesitate. I see a p? in your pattern, and I seem to understand the reasoning behind p being optional.

Still, could we make the pattern more robust by including p into related sub-patterns explicitly, maybe like that? - if [[ $device_for_uuid =~ ^(/dev/(sd[a-z]|nvme[[:digit:]]+n[[:digit:]]+p|mmcblk[[:digit:]]+p))([[:digit:]]+)$ ]] ; then

If yes, do you mind making one more pull request, for me to feel happy when accepting it? :)

jericdeleon commented 4 years ago

Thanks for the input!

The p? check was put outside of a capture group because device_name (BASH_REMATCH[1]) needs the disk name, and must have no p character for NVMe / MMC formats; for example:

The suggested fix yields: device_name=/dev/nvme2n5p, so post-cleanup to remove p characters will have to be done; if this is what you want, let me know and I'll add the post-cleanup part!

If I can find a way to include the p check in the subpattern but exclude it from the match, I'd be happy to do it, but I can't find such a feature so far (POSIX bash doesn't support regex lookarounds)...

s-n-ushakov commented 4 years ago

Yeah, got it now :) Merged! Thanks :)

jericdeleon commented 4 years ago

Thanks for the great work! :)