Re: [arch-releng] [PATCH 3/3] Prompt user to pick a bootloader in interactive_select_packages and select the proper package for installation.

On 02/21/2011 03:50 PM, Dieter Plaetinck wrote:
> On Sun, 20 Feb 2011 23:09:37 -0500
> pyther@pyther.net wrote:
>
>> ---
>>   src/core/libs/lib-ui-interactive.sh |   30 +++++++++++++++++++++---------
>>   1 files changed, 21 insertions(+), 9 deletions(-)
>>
>> diff --git a/src/core/libs/lib-ui-interactive.sh b/src/core/libs/lib-ui-interactive.sh
>> index 16a9600..3cbbc00 100644
>> --- a/src/core/libs/lib-ui-interactive.sh
>> +++ b/src/core/libs/lib-ui-interactive.sh
>> @@ -715,7 +715,7 @@ interactive_select_packages() {
>>   	target_prepare_pacman || { show_warning 'Pacman preparation failure' "Pacman preparation failed! Check $LOG for errors."; return 1; }
>>
>>   	repos=`list_pacman_repos target`
>> -	notify "Package selection is split into two stages.  First you will select package groups that contain packages you may be interested in.  Then you will be presented with a full list of packages for each group, allowing you to fine-tune.\n\n
>> +	notify "Package selection is split into three stages. First, you will select a bootloader. Then, you will select package groups that contain packages that you may be interested in. Lastly, you will be presented with a full list of packages for each group, allowing you to fine-tune.\n\n
>>   Note that right now the packages (and groups) selection is limited to the repos available at this time ($repos).  Once you have your Arch system up and running, you have access to more repositories and packages.\n\n
>>   If any previous configuration you've done until now (like fancy filesystems) require extra packages, we've already preselected them for your convenience"
>>
>> @@ -725,6 +725,20 @@ If any previous configuration you've done until now (like fancy filesystems) req
>>   		grouplist+=(${i} - OFF)
>>   	done
>>
>> +	ask_option Grub "Choose bootloader" "Which bootloader would you like to use?" required \
>> +	"Grub" "Use the GRUB bootloader" \
>> +	"Syslinux" "Use the Syslinux bootloader (ext2/3/4, btrfs, and vfat)" \
>> +	"None" "\Zb\Z1Warning\Z0\ZB: you must install your own bootloader!" || return 1
>> +
>> +	# Convert to lower case
>> +	bootloader=${ANSWER_OPTION,,}
> make the options lowercase in the first place.
> like we discussed on IRC, it would be beneficial to:
> * in lib-blockdevices-filesystems.sh put something like:
> supported_bootloaders=(syslinux grub) # must be packagenames, first one will
> be default
> * leverage that array, also leverage syslinux_supported_fs for the menu
>
>
>> +
>> +	if [[ $bootloader = grub ]]; then
>> +		local needed_pkg_bootloader=grub
>> +	elif [[ $bootloader = syslinux ]]; then
>> +		local needed_pkg_bootloader=syslinux
>> +	fi
> how about something like:
> check_is_in $ANSWER_OPTION ('grub' 'syslinux') || return 1
> local needed_pkg_bootloader=$ANSWER_OPTION
>
>> +
>>   	ask_checklist "Select Package groups\nDo not deselect base unless you know what you're doing!" 0 "${grouplist[@]}" || return 1
>>   	grouplist=("${ANSWER_CHECKLIST[@]}")
>>
>> @@ -735,6 +749,7 @@ If any previous configuration you've done until now (like fancy filesystems) req
>>   	# build the list of options, sorted primarily by group, then by packagename (this is already). marking where appropriate
>>   	local pkglist=()
>>   	needed_pkgs=("${needed_pkgs_fs[@]}")
>> +	needed_pkgs+=("$needed_pkg_bootloader")
>>   	while read pkgname pkgver pkggroup pkgdesc; do
>>   		mark=OFF
>>   		if check_is_in "$pkggroup" "${grouplist[@]}" || check_is_in $pkgname "${needed_pkgs[@]}"; then
>> @@ -843,18 +858,15 @@ interactive_runtime_network() {
>>   	return 0
>>   }
>>
>> +# bootloader is global variable that gets set in interactive_select_packages
>>   interactive_install_bootloader () {
>> -	ask_option Grub "Choose bootloader" "Which bootloader would you like to use?  Grub is the Arch default." required \
>> -	"Grub" "Use the GRUB bootloader (default)" \
>> -	"Syslinux" "Use the Syslinux bootloader (ext2/3/4, btrfs, and vfat)" \
>> -	"None" "\Zb\Z1Warning\Z0\ZB: you must install your own bootloader!" || return 1
>> -
>> -	bl=`tr '[:upper:]' '[:lower:]'<<<  "$ANSWER_OPTION"`
>> -	if [[ $bl == grub ]]; then
>> +	if [[ $bootloader = grub ]]; then
>>   		GRUB_OK=0
>>   		interactive_grub
>> -	elif [[ $bl == syslinux ]]; then
>> +	elif [[ $bootloader = syslinux ]]; then
>>   		interactive_syslinux
>> +	else
>> +		show_warning 'No Bootloader' 'You did not select a bootloader. No bootloader will be installed.'&&  return 1
> "no bootloader" is an accepted solution ->  return 0, not 1 ! otherwise the step will register as failed.
>
>
I know we (Dieter and I) chatted about using $supported_bootloaders for 
ask_option but I couldn't figure out any useful way to provide 
additional information next to the bootloader without creating an array 
like this. supported_bootloaders=(syslinux "some info" grub "some more 
info") which would seem to defeat the purpose of supported_bootloader.

Here is a DIFF of the changes I made:

--- a/src/core/libs/lib-ui-interactive.sh
+++ b/src/core/libs/lib-ui-interactive.sh
@@ -711,6 +711,7 @@ device type label size type create? mountpoint 
options label params" required "$
  # params: none
  # returns: 1 on error
  interactive_select_packages() {
+       local needed_pkgs=()

         # set up our install location if necessary and sync up so we 
can get package lists
         target_prepare_pacman || { show_warning 'Pacman preparation 
failure' "Pacman preparation failed! Check $LOG for errors."; return 1; }
@@ -726,19 +727,12 @@ If any previous configuration you've done until 
now (like fancy filesystems) req
                 grouplist+=(${i} - OFF)
         done

-       ask_option Grub "Choose bootloader" "Which bootloader would you 
like to use?" required \
-       "Grub" "Use the GRUB bootloader" \
-       "Syslinux" "Use the Syslinux bootloader (ext2/3/4, btrfs, and 
vfat)" \
-       "None" "\Zb\Z1Warning\Z0\ZB: you must install your own 
bootloader!" || return 1
+       ask_option no "Choose bootloader" "Which bootloader would you 
like to use?" optional \
+       "grub" "GRUB bootloader" \
+       "syslinux" "Syslinux bootloader (${syslinux_supported_fs[@]})"

-       # Convert to lower case
-       bootloader=${ANSWER_OPTION,,}
-
-       if [[ $bootloader = grub ]]; then
-               local needed_pkg_bootloader=grub
-       elif [[ $bootloader = syslinux ]]; then
-               local needed_pkg_bootloader=syslinux
-       fi
+       # Make sure selected bootloader is a supported_bootloader and 
mark bootloader for installation
+       check_is_in $ANSWER_OPTION "${supported_bootloaders[@]}" && 
needed_pkgs+=("$ANSWER_OPTION")

         ask_checklist "Select Package groups\nDo not deselect base 
unless you know what you're doing!" 0 "${grouplist[@]}" || return 1
         grouplist=("${ANSWER_CHECKLIST[@]}")
@@ -747,10 +741,11 @@ If any previous configuration you've done until 
now (like fancy filesystems) req
         local pkgall=($(list_packages repo core | cut -d ' ' -f2))
         pkginfo "${pkgall[@]}"

+       # packages that should be marked for installation
+       needed_pkgs=("${needed_pkgs_fs[@]}")
+
         # build the list of options, sorted primarily by group, then by 
packagename (this is already). marking where appropriate
         local pkglist=()
-       needed_pkgs=("${needed_pkgs_fs[@]}")
-       needed_pkgs+=("$needed_pkg_bootloader")
         while read pkgname pkgver pkggroup pkgdesc; do
                 mark=OFF
                 if check_is_in "$pkggroup" "${grouplist[@]}" || 
check_is_in $pkgname "${needed_pkgs[@]}"; then
@@ -867,7 +862,7 @@ interactive_install_bootloader () {
         elif [[ $bootloader = syslinux ]]; then
                 interactive_syslinux
         else
-               show_warning 'No Bootloader' 'You did not select a 
bootloader. No bootloader will be installed.' && return 1
+               show_warning 'No Bootloader' 'You did not select a 
bootloader. No bootloader will be installed.' && return 0
         fi
  }