On Fri, Jan 15, 2010 at 4:07 PM, Ulrich Dangel <bts@bts.grml.org> wrote:

Ulrich Dangel <mru@grml.org> added the comment:

On Friday 15 January 2010 17:47:36 Tong Sun wrote:

Hi,

> Referring to "Remove duplicate : in getopt definition."
> http://git.grml.org/?p=grml-debootstrap.git;a=commit;h=30d47e014a7ce7a0f48e
> 21a8d27420ba28fe8082
>
> When grml-debootstrap parameter handling was first fully implemented, the
> "duplicated" : in getopt definition worked follows.
>
> - they are switches with *optional* parameters

Hm? From grml-debootstrap usage():

     --debopt <params>  Extra parameters passed to the debootstrap command.
     --packages <file>    Install packages defined in specified list file.

For me, it seems that they must have a parameter.

You are right. Now it seems so, but not when grml-debootstrap parameter handling was first fully implemented. The man page is my 3rd step in fixing it.
 
And at least after a short look it seems that the $_opt_{debopt,packages}_set
variables are not really needed.

Yes, unless we keep the optional parameters feature.
 
> - their corresponding configuration variable values are set to off in
> /etc/debootstrap/config
> - this means that the --packages, --debconf features are off by default
>  unless turned on on the commandline, using  --packages, or --debconf
>  (without the optional parameters)

I honestly do not see the usecase for that feature. IMHO it is also a little
bit problematic,

because the following? see my comments below.
 
e.g. you set DEBOOTSTRAP_OPT in the config file, but it is
not used/executed. From my point of view a little bit problematic.

I'll take a look at it.
 
> - if optional parameters are giving on the commandline, eg
> --debconf=/some/file/else.conf, the given file is used (eg, as the pre-seed
>  db file) instead of the default one.
>
> This gives the maximum flexibility. I hope you can agree with it as well.

The problem with your solution is that it does not work as expected if you use
extra parameters, e.g.:

$ grep :: cmdlineopts.clp
CMDLINE_OPTS=mirror:,iso:,release:,target:,mntpoint:,debopt:,interactive,nodebootstrap,config:,confdir:,packages::,chroot-
scripts:,scripts:,pre-
scripts:,debconf::,keep_src_list,hostname:,password:,bootappend:,grub:,arch:,insecure,verbose,help,version
$ . ./cmdlineopts.clp --package 445 --bootappend append
$ echo $_opt_packages

$ echo $_opt_bootappend

# now as one parameter
$ ./cmdlineopts.clp --package=445 --bootappend append
$ echo $_opt_packages
445
$ echo $_opt_bootappend
append

Yes, that's exactly the expected behaviour, and why $_opt_{debopt,packages}_set is necessary.
 
From 'man getopt'

,-----
| A long option normally begins with `--' followed by the long
| option name.  If the option has a required argument, it may be
| written directly after the long option name, separated by `=', or
| as the next argument (ie. separated by whitespace on the command
| line).  If the option has an optional argument, it must be
| written directly after the long option name, separated by `=', if
| present (if you add the `=' but nothing behind it, it is
| interpreted as if no argument was present; this is a slight bug,
| see the BUGS).
`-----

cheers