Option handling in grub-mkconfig

Option handling in grub-mkconfig

[Grégoire Sutre]

Hi,

The processing of option `-o' in grub-mkconfig only works when
it is the first option.  The code is:

# Check the arguments.
for option in "$@"; do
     case "$option" in
[...]
     -o)
	shift
	grub_cfg=$1
	;;
     --output=*)
	grub_cfg=`echo "$option" | sed 's/--output=//'`
	;;


For instance:

grub-mkconfig -o /tmp/tata -o /tmp/titi
rm: invalid option -- 'o'
Try `rm --help' for more information.

grub-mkconfig --output=/tmp/tata -o /tmp/titi
rm: invalid option -- 'o'
Try `rm --help' for more information.


AFAICS, other scripts (grub-install, grub-mkrescue) only support the
long option (--output=FILE).

The simpler solution would be to get rid of `-o' in grub-mkconfig,
but this would break some user/distribution scripts (e.g. Debian's
update-grub).

What do you think?

Grégoire

Re: Option handling in grub-mkconfig

[Colin Watson]

On Fri, May 21, 2010 at 12:08:48PM +0200, Grégoire Sutre wrote:
> The processing of option `-o' in grub-mkconfig only works when
> it is the first option.  The code is:
>
> # Check the arguments.
> for option in "$@"; do
>     case "$option" in
> [...]
>     -o)
> 	shift
> 	grub_cfg=$1
> 	;;
>     --output=*)
> 	grub_cfg=`echo "$option" | sed 's/--output=//'`
> 	;;

How about:

=== modified file 'util/grub-mkconfig.in'
--- util/grub-mkconfig.in	2010-04-19 19:25:41 +0000
+++ util/grub-mkconfig.in	2010-05-21 10:54:18 +0000
@@ -50,7 +50,13 @@ EOF
 }
 
 # Check the arguments.
+next_grub_cfg=false
 for option in "$@"; do
+    if $next_grub_cfg; then
+	grub_cfg=$option
+	next_grub_cfg=false
+	continue
+    fi
     case "$option" in
     -h | --help)
 	usage
@@ -59,8 +65,7 @@ for option in "$@"; do
 	echo "$0 (GNU GRUB ${package_version})"
 	exit 0 ;;
     -o)
-	shift
-	grub_cfg=$1
+	next_grub_cfg=:
 	;;
     --output=*)
 	grub_cfg=`echo "$option" | sed 's/--output=//'`
@@ -72,6 +77,11 @@ for option in "$@"; do
 	;;
     esac
 done
+if $next_grub_cfg; then
+    echo "Missing argument to \`-o'" 1>&2
+    usage
+    exit 1
+fi
 
 . ${libdir}/grub/grub-mkconfig_lib
 

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: Option handling in grub-mkconfig

[Grégoire Sutre]

On 05/21/2010 12:57 PM, Colin Watson wrote:

Thanks for the patch, it works fine.  I'm just wondering: Is keeping
`-o' worth the extra complication?

Grégoire

> How about:
>
> === modified file 'util/grub-mkconfig.in'
> --- util/grub-mkconfig.in	2010-04-19 19:25:41 +0000
> +++ util/grub-mkconfig.in	2010-05-21 10:54:18 +0000
> @@ -50,7 +50,13 @@ EOF
>   }
>
>   # Check the arguments.
> +next_grub_cfg=false
>   for option in "$@"; do
> +    if $next_grub_cfg; then
> +	grub_cfg=$option
> +	next_grub_cfg=false
> +	continue
> +    fi
>       case "$option" in
>       -h | --help)
>   	usage
> @@ -59,8 +65,7 @@ for option in "$@"; do
>   	echo "$0 (GNU GRUB ${package_version})"
>   	exit 0 ;;
>       -o)
> -	shift
> -	grub_cfg=$1
> +	next_grub_cfg=:
>   	;;
>       --output=*)
>   	grub_cfg=`echo "$option" | sed 's/--output=//'`
> @@ -72,6 +77,11 @@ for option in "$@"; do
>   	;;
>       esac
>   done
> +if $next_grub_cfg; then
> +    echo "Missing argument to \`-o'" 1>&2
> +    usage
> +    exit 1
> +fi
>
>   . ${libdir}/grub/grub-mkconfig_lib
>
>

Re: Option handling in grub-mkconfig

[Colin Watson]

On Fri, May 21, 2010 at 02:19:36PM +0200, Grégoire Sutre wrote:
> Thanks for the patch, it works fine.  I'm just wondering: Is keeping
> `-o' worth the extra complication?

I think so.  It's probably stuck in a number of people's heads by now.

If we want to simplify things, we should use something like getopt(1) as
I think I proposed a while back, although that's not entirely without
its problems.  TBH I don't think the complication here is a big deal;
it's just one option and it doesn't change often.

-- 
Colin Watson                                       [cjwatson@ubuntu.com]

Re: Option handling in grub-mkconfig

[Vladimir 'φ-coder/phcoder' Serbinenko]

[-- Attachment #1: Type: text/plain, Size: 748 bytes --]

Colin Watson wrote:
> On Fri, May 21, 2010 at 02:19:36PM +0200, Grégoire Sutre wrote:
>   
>> Thanks for the patch, it works fine.  I'm just wondering: Is keeping
>> `-o' worth the extra complication?
>>     
>
> I think so.  It's probably stuck in a number of people's heads by now.
>
> If we want to simplify things, we should use something like getopt(1) as
> I think I proposed a while back, although that's not entirely without
> its problems.
What are the problems with getopt? Cross-vendor compatibility? Can it be
alleviated the gnulib way?
>   TBH I don't think the complication here is a big deal;
> it's just one option and it doesn't change often.
>
>   


-- 
Regards
Vladimir 'φ-coder/phcoder' Serbinenko



[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 293 bytes --]

Re: Option handling in grub-mkconfig

[BVK Chaitanya]

2010/5/21 Grégoire Sutre <gregoire.sutre@gmail.com>:
>
> The processing of option `-o' in grub-mkconfig only works when
> it is the first option.
>

IMO this issue might have been fixed in experimental branch, by
fix-cmdline-parsing (or similar) branch.  I am on travel, so I cannot
confirm this now :-(



-- 
bvk.chaitanya

Re: Option handling in grub-mkconfig

[Grégoire Sutre]

On 05/24/2010 06:42 AM, BVK Chaitanya wrote:
> 2010/5/21 Grégoire Sutre<gregoire.sutre@gmail.com>:
>>
>> The processing of option `-o' in grub-mkconfig only works when
>> it is the first option.
>>
>
> IMO this issue might have been fixed in experimental branch, by
> fix-cmdline-parsing (or similar) branch.  I am on travel, so I cannot
> confirm this now :-(

I confirm that the fix-cmdline-arg-parsing branch does not have this
problem.

Grégoire