[Date Prev][Date Next] [Thread Prev][Thread Next] [Date Index] [Thread Index]

an intresting problem (Now it worls) [PATCH]



Peter Skogstr?m <peter.skogstrom at bitrunner.com> writes:

> This patch makes usb-hdd work, both with and without LH_SYSLINUX_MENU
> enabled.

Good. Let's do a review of the patch :-)

> I have only checked usb-hdd not pxeboot, I also found some minor errors
> afterwards (I think it was that I dont see the live-safe label with the
> vesamenu.)

Strange. This (linux-failsafe) works for me.

> This is my first patch to debian, I have found bugs and done oneliners
> before, but have some indulgence if I have done something wrong.

No problem. We all need to learn ;-)

Your patch didn't use a new code as basis. You can check it on:

http://git.debian.org/?p=users/otavio/live-helper.git;a=blob;f=helpers/lh_binary_syslinux
on line 353. 

> --- lh_binary_syslinux	2007-10-07 09:03:36.000000000 +0200
> +++ lh_binary_syslinux_ok	2007-10-08 11:13:40.000000000 +0200
> @@ -584,18 +584,34 @@
>  		# Copying syslinux
>  		mkdir -p "${DESTDIR}"
>  
> -		case "${LH_CHROOT_BUILD}" in
> -			enabled)
> -				cp chroot/usr/lib/syslinux/isolinux.bin "${DESTDIR}"/syslinux.bin
> -				;;
> -
> -			disabled)
> -				cp /usr/lib/syslinux/isolinux.bin "${DESTDIR}"/syslinux.bin
> -				;;
> -		esac
> +
> +
> +	#	case "${LH_CHROOT_BUILD}" in
> +	#		enabled)
> +	#			cp chroot/usr/lib/syslinux/syslinux.bin "${DESTDIR}"/syslinux.bin
> +	#			;;
> +	#
> +	#		disabled)
> +	#			cp /usr/lib/syslinux/syslinux.bin "${DESTDIR}"/syslinux.bin
> +	#			;;
> +	#	esac

On line 353 it's different of what you've commented out and it's required.
  
>  		cp -r "${TEMPLATES}"/common/* "${DESTDIR}"
> -		cp -r "${TEMPLATES}"/"${LH_LANGUAGE}"/* "${DESTDIR}"
> +	
> +		if [ "${LH_SYSLINUX_MENU}" = "disabled" ]
> +		then
> +			cp -r "${TEMPLATES}"/normal/* "${DESTDIR}"
> +		else
> +			cp -r "${TEMPLATES}"/menu/* "${DESTDIR}"
> +		fi
> +
> +		if [ -d "${TEMPLATES}"/"${LH_LANGUAGE}" ]
> +		then
> +			cp -r "${TEMPLATES}"/"${LH_LANGUAGE}"/* "${DESTDIR}"
> +		else
> +			cp -r "${TEMPLATES}"/en/* "${DESTDIR}"
> +		fi
> +

Similar change already done, line 364 of same URI.

>  		for FILE in "${DESTDIR}"/*.live
>  		do
> @@ -628,6 +644,18 @@
>  			cp config/binary_syslinux/syslinux.cfg "${DESTDIR}"/syslinux.cfg
>  		fi
>  
> +
> +		# Copying menu module
> +		if [ "${LH_SYSLINUX_MENU}" != "disabled" ]
> +		then
> +			MENUPATH="$(grep 'menu.c32' "${DESTDIR}"/header.cfg | sed 's,default\s*\(.*menu.c32\)$,\1,g')"
> +			MENUMODULE="$(basename ${MENUPATH})"
> +
> +			#mkdir -p binary/isolinux/"$(dirname ${MENUPATH})"
> +			cp chroot/usr/lib/syslinux/"${MENUMODULE}" "${DESTDIR}"/"$(dirname ${MENUPATH})"
> +		fi
> +
> +

Done too.

All rest looks to be already taken care. It would be nice if you could
try my lastest snapshot[1] and use _this_ as basis for a new patch
otherwise is very difficult to identify what's really required to
change.

1. http://alioth.debian.org/~otavio/snapshot/live-helper/

TIA.

-- 
        O T A V I O    S A L V A D O R
---------------------------------------------
 E-mail: otavio at debian.org      UIN: 5906116
 GNU/Linux User: 239058     GPG ID: 49A5F855
 Home Page: http://otavio.ossystems.com.br
---------------------------------------------
"Microsoft sells you Windows ... Linux gives
 you the whole house."



Reply to: