Add submenu support to 10_linux (grub-mkconfig)
PATCH 1/1 Add submenu support to 10_linux (grub-mkconfig)
Below is a patch for adding submenu support to 10_linux (grub-mkconfig). This is *not* the patch which Ubuntu has been carying for submenu support and so I'd especially like feedback from Ubuntu folks.
With GRUB_ENABLE_SUBMENUS=true in /etc/default/grub this patch will create a simple primary menu entry without any information about the kernel version in the title. Something like "Debian GNU/Linux". Below this main entry will be a submenu titled "Advanced options for Debian GNU/Linux". Selecting this entry will get you a list of all of the menu entries as they would be listed for that OS if submenus weren't enabled. So the submenu will contain an entry for every kernel version (including the one which would be booted by the main entry, so there is a duplication there) and recovery mode entries (if GRUB_DISABLE_RECOVERY != true) with the normal inclusion of the kernel version in the titles.
This patch only affects 10_linux. If it's agreed that it's sound I will make similar patches for 20_xen and for the other 10_* where it is appropriate but I won't be able to test those easily myself. I'll also make a patch for 30_os-prober (which I will be able to test easily) and for grub.texi. I would have liked to have those already done but I've been pretty busy. I should be able to get the rest made by the end of this week.
What follows is my reasoning for differing from Ubuntu in this case. It's longer than I expected it to be when I started writing this email, and isn't important to the contents of the patch itself, so if you'd just like to review the changes for sanity you can skip to the patch.
This is contrasted with Ubuntu's patch which has an entry for the newest kernel (with kernel version included in the title), a recovery mode entry for the same version, then a submenu with all previous versions. The main reasons I dislike this approach are 1: Most people don't need to know the kernel version they're booting by default 2: Including the kernel version by default means that a user can't simply use GRUB_DEFAULT="Debian GNU/Linux" to specify that they want the newest Debian kernel to be booted by default.
Other differences from Ubuntu's patch:
This patch makes submenus optional. If you don't add GRUB_ENABLE_SUBMENUS=true to /etc/default/grub then nothing will change because of this patch.
This patch makes defaults and saved defaults more intuative and useful. In addition to being able to simply use GRUB_DEFAULT="Debian GNU/Linux" you can also specify that you would like what *currently* happens to be the newest kernel to be used in the future as well, even if a newer kernel gets installed. With this patch you could use GRUB_DEFAULT="Advanced options for Debian GNU/Linux>Debian GNU/Linux with Linux 3.2.0-17". With Ubuntu you might try GRUB_DEFAULT="Ubuntu with Linux 3.2.0-17" but the moment a newer kernel is installed that entry will need to change to "Previous Linux versions>Ubuntu with Linux 3.2.0-17". This patch also makes saved defaults more intuitive and useful because if a user chooses simply "Debian GNU/Linux", and they're using GRUB_SAVEDEFAULT=true, they probably want and expect that on the next boot this same entry will be selected again, even if it represents a newer kernel version. If however the user decides to choose "Advanced options for Debian GNU/Linux>Debian GNU/Linux with Linux 3.2.0-17" they probably want and expect the exact same kernel version to be the default for the next boot.
This patch adds indentation for "menuentry" commands within the submenu in the grub.cfg, to make the grub.cfg more readable.
This patch creates a predictable number of menu entries in the main menu. One main entry and one submenu. With Ubuntu if you had more than one kernel version you'd get one default entry for the newest, one recovery mode entry for the newest, and a submenu. If you only had one kernel version the submenu would be omitted.
Signature · 100 lines
Jordan Uggla (Jordan_U on irc.freenode.net)
=== modified file 'util/grub-mkconfig.in'
--- util/grub-mkconfig.in 2012-02-26 17:37:54 +0000
+++ util/grub-mkconfig.in 2012-02-27 08:22:48 +0000
@@ -206,7 +206,8 @@
GRUB_INIT_TUNE \
GRUB_SAVEDEFAULT \
GRUB_ENABLE_CRYPTODISK \
- GRUB_BADRAM
+ GRUB_BADRAM \
+ GRUB_ENABLE_SUBMENUS
if test "x${grub_cfg}" != "x"; then
rm -f "${grub_cfg}.new"
=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in 2012-02-26 16:28:05 +0000
+++ util/grub.d/10_linux.in 2012-02-27 08:55:21 +0000
@@ -63,15 +63,25 @@
{
os="$1"
version="$2"
- recovery="$3"
+ type="$3"
args="$4"
- if ${recovery} ; then
- title="$(gettext_quoted "%s, with Linux %s (recovery mode)")"
+
+ case $type in
+ simple)
+ title="$(gettext_quoted "%s")" ;;
+ recovery)
+ title="$(gettext_quoted "%s, with Linux %s (recovery mode)")" ;;
+ *)
+ title="$(gettext_quoted "%s, with Linux %s")" ;;
+ esac
+
+ if [ "x$type" = "xsimple" ]; then
+ printf "menuentry '${title}' ${CLASS} {\n" "${os}"
else
- title="$(gettext_quoted "%s, with Linux %s")"
+ printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
fi
- printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
- if ! ${recovery} ; then
+
+ if [ x$type != xrecovery ] ; then
save_default_entry | sed -e "s/^/\t/"
fi
@@ -145,6 +155,11 @@
prepare_boot_cache=
prepare_root_cache=
+# Extra indentation to add to menu entries in a submenu. We're not in a submenu
+# yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
+submenu_indentation=""
+
+is_first_entry=true
while [ "x$list" != "x" ] ; do
linux=`version_find_latest $list`
gettext_printf "Found linux image: %s\n" "$linux" >&2
@@ -189,12 +204,32 @@
linux_root_device_thisversion=${GRUB_DEVICE}
fi
- linux_entry "${OS}" "${version}" false \
- "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+ if [ "x$GRUB_ENABLE_SUBMENUS" = xtrue -a "x$is_first_entry" = xtrue ]; then
+ linux_entry "${OS}" "${version}" simple \
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+
+ submenu_indentation="\t"
+
+ cat << EOF
+submenu '$(gettext_quoted "Advanced options for ${OS}")' {
+EOF
+ fi
+
+ linux_entry "${OS}" "${version}" advanced \
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" \
+ | sed "s/^/$submenu_indentation/"
if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
- linux_entry "${OS}" "${version}" true \
- "single ${GRUB_CMDLINE_LINUX}"
+ linux_entry "${OS}" "${version}" recovery \
+ "single ${GRUB_CMDLINE_LINUX}" \
+ | sed "s/^/$submenu_indentation/"
fi
list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
+ is_first_entry=false
done
+
+# If submenus are enabled, and at least one kernel was found, then we need to
+# add a closing '}' for the submenu command.
+if [ x"$GRUB_ENABLE_SUBMENUS" = xtrue -a x"$is_first_entry" != xtrue ]; then
+ echo '}'
+fiIn my first patch I rather stupidly asked gettext for a translation of "%s". Updated patch below does not have this mistake.
Signature · 100 lines
Jordan Uggla (Jordan_U on irc.freenode.net)
=== modified file 'util/grub-mkconfig.in'
--- util/grub-mkconfig.in 2012-02-26 17:37:54 +0000
+++ util/grub-mkconfig.in 2012-02-27 08:22:48 +0000
@@ -206,7 +206,8 @@
GRUB_INIT_TUNE \
GRUB_SAVEDEFAULT \
GRUB_ENABLE_CRYPTODISK \
- GRUB_BADRAM
+ GRUB_BADRAM \
+ GRUB_ENABLE_SUBMENUS
if test "x${grub_cfg}" != "x"; then
rm -f "${grub_cfg}.new"
=== modified file 'util/grub.d/10_linux.in'
--- util/grub.d/10_linux.in 2012-02-26 16:28:05 +0000
+++ util/grub.d/10_linux.in 2012-02-29 03:03:01 +0000
@@ -63,15 +63,25 @@
{
os="$1"
version="$2"
- recovery="$3"
+ type="$3"
args="$4"
- if ${recovery} ; then
- title="$(gettext_quoted "%s, with Linux %s (recovery mode)")"
+
+ case $type in
+ simple)
+ title="%s" ;;
+ recovery)
+ title="$(gettext_quoted "%s, with Linux %s (recovery mode)")" ;;
+ *)
+ title="$(gettext_quoted "%s, with Linux %s")" ;;
+ esac
+
+ if [ "x$type" = "xsimple" ]; then
+ printf "menuentry '${title}' ${CLASS} {\n" "${os}"
else
- title="$(gettext_quoted "%s, with Linux %s")"
+ printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
fi
- printf "menuentry '${title}' ${CLASS} {\n" "${os}" "${version}"
- if ! ${recovery} ; then
+
+ if [ x$type != xrecovery ] ; then
save_default_entry | sed -e "s/^/\t/"
fi
@@ -145,6 +155,11 @@
prepare_boot_cache=
prepare_root_cache=
+# Extra indentation to add to menu entries in a submenu. We're not in a submenu
+# yet, so it's empty. In a submenu it will be equal to '\t' (one tab).
+submenu_indentation=""
+
+is_first_entry=true
while [ "x$list" != "x" ] ; do
linux=`version_find_latest $list`
gettext_printf "Found linux image: %s\n" "$linux" >&2
@@ -189,12 +204,32 @@
linux_root_device_thisversion=${GRUB_DEVICE}
fi
- linux_entry "${OS}" "${version}" false \
- "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+ if [ "x$GRUB_ENABLE_SUBMENUS" = xtrue -a "x$is_first_entry" = xtrue ]; then
+ linux_entry "${OS}" "${version}" simple \
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}"
+
+ submenu_indentation="\t"
+
+ cat << EOF
+submenu '$(gettext_quoted "Advanced options for ${OS}")' {
+EOF
+ fi
+
+ linux_entry "${OS}" "${version}" advanced \
+ "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" \
+ | sed "s/^/$submenu_indentation/"
if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then
- linux_entry "${OS}" "${version}" true \
- "single ${GRUB_CMDLINE_LINUX}"
+ linux_entry "${OS}" "${version}" recovery \
+ "single ${GRUB_CMDLINE_LINUX}" \
+ | sed "s/^/$submenu_indentation/"
fi
list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '`
+ is_first_entry=false
done
+
+# If submenus are enabled, and at least one kernel was found, then we need to
+# add a closing '}' for the submenu command.
+if [ x"$GRUB_ENABLE_SUBMENUS" = xtrue -a x"$is_first_entry" != xtrue ]; then
+ echo '}'
+fiQuoted 3 lines · Jordan Uggla
In my first patch I rather stupidly asked gettext for a translation of "%s". Updated patch below does not have this mistake.
I attach the update on the top of menuid.
I think it would be bad to break every time user changes the configuration entry. So I'd prefer to remove this option. Now there is however a problem with migration. I see following possibilities:
1) Check in grub-mkconfig & co that GRUB_DEFAULT/saved_entry from envblk contains old or old Ubuntu-style entries and if it does add a stanza like:
if [ x"$default" = x"<entry>" ]; then
default="<new name>"
fi
and issue a warning
2) Like (1) but replace a warning with error and issue no stanza
3) Like (1) but perform no check and issue no warning and simply generate stanzas for all possible entries. Would result in a lot of clutter
4) Like (3) make check independent of stanzas and use it for issuing the warning
Right now I believe that (1) is the most reasonable
@Colin Watson: given the problems with current Ubuntu submenu patch and a migration way like in (1) is it possible to replace Ubuntu patch with Jordan's one for 12.10?
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'util/grub.d/10_linux.in'
grub.d/10_linux.in +47 -12
@@ -70,18 +70,25 @@ { os="$1" version="$2" - recovery="$3" + type="$3" args="$4" - if ${recovery} ; then - title="$(gettext_quoted "%s, with Linux %s (recovery mode)")" - else - title="$(gettext_quoted "%s, with Linux %s")" - fi + + case $type in + recovery) + title="$(gettext_quoted "%s, with Linux %s (recovery mode)")" ;; + *) + title="$(gettext_quoted "%s, with Linux %s")" ;; + esac if [ -z "$boot_device_id" ]; then boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")" fi - printf "menuentry '${title}' ${CLASS} \$menuentry_id_option 'gnulinux-$version-$recovery-$boot_device_id' {\n" "${os}" "${version}" - if ! ${recovery} ; then + case $type in + if [ x$type != xsimple ] ; then + printf "menuentry '${title}' ${CLASS} \$menuentry_id_option 'gnulinux-$version-$type-$boot_device_id' {\n" "${os}" "${version}" + else + echo "menuentry '${os}' ${CLASS} \$menuentry_id_option 'gnulinux-simple-$boot_device_id' {" + fi + if [ x$type != xrecovery ] ; then save_default_entry | sed -e "s/^/\t/" fi
@@ -156,6 +163,11 @@ prepare_root_cache= boot_device_id= +# Extra indentation to add to menu entries in a submenu. We're not in a submenu +# yet, so it's empty. In a submenu it will be equal to '\t' (one tab). +submenu_indentation="" + +is_first_entry=true while [ "x$list" != "x" ] ; do linux=`version_find_latest $list` gettext_printf "Found linux image: %s\n" "$linux" >&2
@@ -200,12 +212,35 @@ linux_root_device_thisversion=${GRUB_DEVICE} fi - linux_entry "${OS}" "${version}" false \ - "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" + if [ "x$is_first_entry" = xtrue ]; then + linux_entry "${OS}" "${version}" simple \ + "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" + + submenu_indentation="\t" + + if [ -z "$boot_device_id" ]; then + boot_device_id="$(grub_get_device_id "${GRUB_DEVICE}")" + fi + cat << EOF +submenu '$(gettext_quoted "Advanced options for ${OS}")' \$menuentry_id_option 'gnulinux-advanced-$boot_device_id' { +EOF + fi + + linux_entry "${OS}" "${version}" advanced \ + "${GRUB_CMDLINE_LINUX} ${GRUB_CMDLINE_LINUX_DEFAULT}" \ + | sed "s/^/$submenu_indentation/" if [ "x${GRUB_DISABLE_RECOVERY}" != "xtrue" ]; then - linux_entry "${OS}" "${version}" true \ - "single ${GRUB_CMDLINE_LINUX}" + linux_entry "${OS}" "${version}" recovery \ + "single ${GRUB_CMDLINE_LINUX}" \ + | sed "s/^/$submenu_indentation/" fi list=`echo $list | tr ' ' '\n' | grep -vx $linux | tr '\n' ' '` + is_first_entry=false done + +# If at least one kernel was found, then we need to +# add a closing '}' for the submenu command. +if [ x"$is_first_entry" != xtrue ]; then + echo '}' +fi
Quoted 3 lines
Right now I believe that (1) is the most reasonable
Somethine like in the attached patch. I haven't tested it yet in real migration scenario though
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'util/grub-mkconfig.in'
grub-mkconfig.in +8 -1
@@ -40,6 +40,7 @@ self=`basename $0` grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`" +grub_editenv="${bindir}/`echo grub-editenv | sed "${transform}"`" grub_script_check="${bindir}/`echo grub-script-check | sed "${transform}"`" export TEXTDOMAIN=@PACKAGE@
@@ -165,6 +166,11 @@ esac done +GRUB_ACTUAL_DEFAULT="$GRUB_DEFAULT" + +if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then GRUB_ACTUAL_DEFAULT="`"${grub_editenv}" - list | sed -n '/^saved_entry=/ s,^saved_entry=,,p'`" ; fi + + # These are defined in this script, export them here so that user can # override them. export GRUB_DEVICE \
@@ -173,7 +179,8 @@ GRUB_DEVICE_BOOT_UUID \ GRUB_FS \ GRUB_FONT \ - GRUB_PRELOAD_MODULES + GRUB_PRELOAD_MODULES \ + GRUB_ACTUAL_DEFAULT # These are optional, user-defined variables. export GRUB_DEFAULT \
=== modified file 'util/grub.d/10_linux.in'
grub.d/10_linux.in +65 -13
103 lines not on this page · raw message
Update
Quoted 7 lines · Vladimir 'φ-coder/phcoder' Serbinenko
Quoted 3 lines
Right now I believe that (1) is the most reasonable
Somethine like in the attached patch. I haven't tested it yet in real migration scenario though
Signature · 2 lines
Regards Vladimir 'φ-coder/phcoder' Serbinenko
=== modified file 'util/grub-mkconfig.in'
grub-mkconfig.in +8 -1
@@ -40,6 +40,7 @@ self=`basename $0` grub_probe="${sbindir}/`echo grub-probe | sed "${transform}"`" +grub_editenv="${bindir}/`echo grub-editenv | sed "${transform}"`" grub_script_check="${bindir}/`echo grub-script-check | sed "${transform}"`" export TEXTDOMAIN=@PACKAGE@
@@ -165,6 +166,11 @@ esac done +GRUB_ACTUAL_DEFAULT="$GRUB_DEFAULT" + +if [ "x${GRUB_ACTUAL_DEFAULT}" = "xsaved" ] ; then GRUB_ACTUAL_DEFAULT="`"${grub_editenv}" - list | sed -n '/^saved_entry=/ s,^saved_entry=,,p'`" ; fi + + # These are defined in this script, export them here so that user can # override them. export GRUB_DEVICE \
@@ -173,7 +179,8 @@ GRUB_DEVICE_BOOT_UUID \ GRUB_FS \ GRUB_FONT \ - GRUB_PRELOAD_MODULES + GRUB_PRELOAD_MODULES \ + GRUB_ACTUAL_DEFAULT # These are optional, user-defined variables. export GRUB_DEFAULT \
=== modified file 'util/grub-mkconfig_lib.in'
grub-mkconfig_lib.in +1 -1
@@ -249,7 +249,7 @@ gettext_printf () { gettext_printf_format="$1" shift - printf "$(gettext_quoted "$gettext_printf_format")" "$@" + printf "$(gettext "$gettext_printf_format")" "$@" } uses_abstraction () {
=== modified file 'util/grub.d/10_linux.in'
grub.d/10_linux.in +67 -33
163 lines not on this page · raw message