[arch-releng] [PATCH 3/3] Provide Syslinux as a possible bootloader

[Matthew Gyurgyik]

On 02/28/2011 03:52 PM, Dieter Plaetinck wrote:
> On Sun, 27 Feb 2011 17:11:24 -0500
> pyther at pyther.net wrote:
>> +
>> +	if device_is_raid "$bootpart"; then
>> +		debug FS "Software RAID detected"
>> +		local onraid=true
>> +		# Notify the user or trust the scripts?
> what exactly is this comment supposed to achieve?
Nothing it was left over from some old code. I removed it.
> In addition to the comment above, I'll copy paste some feedback from an earlier review:
>
> +  if ! "$var_TARGET_DIR/usr/lib/syslinux/syslinux-install_update" -i -c /mnt;
>    then
> you should make sure the user can see the output of this command
> something like:>$LOG 2>&1
Fixed
> get_kernel_parameters should get an '|| return 1'
> same thing for generate_syslinux_menu and generate_grub_menulst in
> interactive_bootloader_menu()
Fixed for generate_syslinux_menu... I will submit a separate patch for grub
> Furthermore:
> - We can probably remove GRUB_OK (I can look after that myself also)
I agree, although that should be a separate patch, right?
> - To show warnings, use show_warning, not notify
Of course, fixed...
> Dieter

I haven't tested the changes yet. Although I don't see them causing any 
troubles, I do want to give it a test run. However, I won't be able to 
do that for a couple of days though.

A diff of the changes:

diff --git a/src/core/libs/lib-ui-interactive.sh 
b/src/core/libs/lib-ui-interactive.sh
index 6b6260b..c3686e4 100644
--- a/src/core/libs/lib-ui-interactive.sh
+++ b/src/core/libs/lib-ui-interactive.sh
@@ -1164,12 +1164,11 @@ interactive_syslinux() {
         if device_is_raid "$bootpart"; then
                 debug FS "Software RAID detected"
                 local onraid=true
-               # Notify the user or trust the scripts?
         fi

         debug FS "Installing Syslinux 
($var_TARGET_DIR/usr/sbin/syslinux-install_update -i -c /mnt)"
         inform "Installing Syslinux..."
-       if ! "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -i -c 
/mnt; then
+       if ! "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -i -c 
/mnt >$LOG 2>&1; then
                 debug FS "FAILED: syslinux-install_update -i -c /mnt 
failed"
                 show_warning "FAILED" "syslinux-install_update -i -c 
/mnt failed"
                 return 1
@@ -1177,26 +1176,26 @@ interactive_syslinux() {

         if ask_yesno "Set boot flag(s) and install the Syslinux MBR?" 
yes; then
                 inform "Setting Boot Flag(s)...\nThis could take a 
while. Please be patient.\n\n" syslinuxprog
-               if "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -a 
-c /mnt; then
+               if "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -a 
-c /mnt >$LOG 2>&1; then
                         debug FS "Successfully set boot flag(s)"
                 else
                         debug FS "Failde to set boot flag(s). 
syslinux-install_update -a failed with Error Code - $?"
-                       notify "Failed to set boot flag(s). MBR not 
installed" && return 1
+                       show_warning "FAILED" "Failed to set boot 
flag(s). MBR not installed" && return 1
                 fi

                 inform "Installing Syslinux MBR..." syslinuxprog
-               if "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -m 
-c /mnt; then
+               if "$var_TARGET_DIR/usr/sbin/syslinux-install_update" -m 
-c /mnt >$LOG 2>&1; then
                         debug FS "Successfully installed MBR(s)"
                 else
                         debug FS "Failed to install MBR. 
syslinux-install_update -m failed with Error Code - $?"
-                       notify "Failed to install the MBR!" && return 1
+                       show_warning "FAILED" "Failed to install the 
MBR!" && return 1
                 fi
         fi
         notify "Syslinux Installation Successful"
  }

  generate_syslinux_menu () {
-       get_kernel_parameters
+       get_kernel_parameters || return 1

         cat >>$syslinuxmenu <<EOF