FAQ Search Today's Posts Mark Forums Read
» Video Reviews

» Linux Archive

Linux-archive is a website aiming to archive linux email lists and to make them easily accessible for linux users/developers.


» Sponsor

» Partners

» Sponsor

Go Back   Linux Archive > ArchLinux > ArchLinux Pacman Development

 
 
LinkBack Thread Tools
 
Old 05-20-2008, 07:54 PM
"Dan McGee"
 
Default Fix command line argument parsing in makepkg

On Mon, May 19, 2008 at 10:54 AM, Sebastian Nowicki <sebnow@gmail.com> wrote:
> On some systems, namely Mac OSX, command line parsing simply does not
> work. It appears $@ gets altered at some stage. This patch uses $ARGLIST
> instead, which contains the actual command line arguments
>
> Signed-off-by: Sebastian Nowicki <sebnow@gmail.com>
> ---
-p and --forcever have required arguments. This patch still works with
those, correct? (Meaning it reads and uses the next arg for the value,
and does not attempt to parse the arg as a flag.)

> scripts/makepkg.sh.in | 7 +++----
> 1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/scripts/makepkg.sh.in b/scripts/makepkg.sh.in
> index cc44c68..f56bcda 100644
> --- a/scripts/makepkg.sh.in
> +++ b/scripts/makepkg.sh.in
> @@ -1143,8 +1143,8 @@ fi
> eval set -- "$OPT_TEMP"
> unset OPT_SHORT OPT_LONG OPT_TEMP
>
> -while true; do
> - case "$1" in
> +for arg in ${ARGLIST[@]}; do
> + case "$arg" in
> # Pacman Options
> --noconfirm) PACMAN_OPTS="$PACMAN_OPTS --noconfirm" ;;
> --noprogressbar) PACMAN_OPTS="$PACMAN_OPTS --noprogressbar" ;;
> @@ -1180,10 +1180,9 @@ while true; do
> -h|--help) usage; exit 0 ;; # E_OK
> -V|--version) version; exit 0 ;; # E_OK
>
> - --) OPT_IND=0; shift; break;;
> + --) OPT_IND=0; continue; break;;
Is it just me, or does this make no sense at all (continue followed by
break)? And the problem now is our $@ is not properly shifted for this
flag to even do anything.

> *) usage; exit 1 ;; # E_INVALID_OPTION
> esac
> - shift
> done
>
> if [ "$HOLDVER" = "1" -a "$FORCE_VER" != "" ]; then
> --
I guess I'm thinking this is a well-intentioned patch, but should we
try to attack the faulty $@ problem instead?

-Dan

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-20-2008, 11:27 PM
Xavier
 
Default Fix command line argument parsing in makepkg

Dan McGee wrote:
>>
>> - --) OPT_IND=0; shift; break;;
>> + --) OPT_IND=0; continue; break;;
> Is it just me, or does this make no sense at all (continue followed by
> break)? And the problem now is our $@ is not properly shifted for this
> flag to even do anything.
>

Yeah, I noticed this funny continue; break thing, but then I guess I got
disturbed trying to find out what OPT_IND was and failing.

>> *) usage; exit 1 ;; # E_INVALID_OPTION
>> esac
>> - shift
>> done
>>
>> if [ "$HOLDVER" = "1" -a "$FORCE_VER" != "" ]; then
>> --
> I guess I'm thinking this is a well-intentioned patch, but should we
> try to attack the faulty $@ problem instead?
>

Agreed, that was exactly my sentiment.
Before finding a workaround, please try to find more about what the
issue actually is. Try to write a simple testcase reproducing the problem.

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-21-2008, 04:21 PM
"Sebastian Nowicki"
 
Default Fix command line argument parsing in makepkg

It's definitely not a good patch, I just needed to get it fixed quickly so I could use makepkg, and thought I might as well submit it to provoke some discussion. It seems -p does brake, and I assume --forcever does as well. I didn't have enough knowledge of makepkg, nor the time, to tackle the problem properly. I suppose filing a bug report would have been better .


I narrowed down the problem to the eval command. It appears eval changes $@ for some reason, as the following shell scripts shows:
$ cat test-eval.sh
#!/bin/bash
echo "$@ before eval: "
echo $@

eval set -- "-p test abcd"
echo "$@ after eval:"
echo $@

$ ./test-eval.sh this is a test
$@ before eval:
this is a test
$@ after eval:
-p test abcd

--
Sebastian Nowicki




_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-21-2008, 05:13 PM
Xavier
 
Default Fix command line argument parsing in makepkg

Sebastian Nowicki wrote:
> It's definitely not a good patch, I just needed to get it fixed quickly so I
> could use makepkg, and thought I might as well submit it to provoke some
> discussion. It seems -p does brake, and I assume --forcever does as well. I
> didn't have enough knowledge of makepkg, nor the time, to tackle the problem
> properly. I suppose filing a bug report would have been better .
>
> I narrowed down the problem to the eval command. It appears eval changes $@
> for some reason, as the following shell scripts shows:
> $ cat test-eval.sh
> #!/bin/bash
> echo "$@ before eval: "
> echo $@
> eval set -- "-p test abcd"
> echo "$@ after eval:"
> echo $@
>
> $ ./test-eval.sh this is a test
> $@ before eval:
> this is a test
> $@ after eval:
> -p test abcd
>

It is not eval, it is set. As far as I can tell, it is perfectly normal,
that call seems to be made exactly for that purpose: changing the args
($@, $1, etc). And I get the same behavior here anyway.

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-21-2008, 08:03 PM
Sebastian Nowicki
 
Default Fix command line argument parsing in makepkg

On 22/05/2008, at 1:13 AM, Xavier wrote:

> It is not eval, it is set. As far as I can tell, it is perfectly
> normal,
> that call seems to be made exactly for that purpose: changing the args
> ($@, $1, etc). And I get the same behavior here anyway.

I see. After some playing around it seems that this version of getopt
simply doesn't support any parameters. Using "getopt abc $@" works
fine. When running the test script with "-a -b -c", $@ becomes "-a -b -
c --". If "getopt -o abs $@" is used, $@ becomes "-- abc -a -b -c", in
which case makepkg would immediately break out of the loop.

> $ cat test.sh
> #!/bin/bash
> echo "$@=$@"
> args=$(getopt abc $@)
> if [ $? != 0 ]; then
> echo "Invalid arguments"
> exit 1
> fi
> eval set -- $args
> echo "$@=$@"
> while true; do
> case "$1" in
> -a|-b|-c)
> echo "Got $1"; shift;;
> *)
> echo "Invalid: $1"; break;;
> esac
> done
>
> $ ./test.sh -a -b -c
> $@=-a -b -c
> $@=-a -b -c --
> Got -a
> Got -b
> Got -c
> Invalid: --

After changing "getopt abc $@" to "getopt -o abc $@":
> $ ./test.sh -a -b -c
> $@=-a -b -c
> $@=-- abc -a -b -c
Invalid: --

---
Sebastian Nowicki


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-21-2008, 11:13 PM
Xavier
 
Default Fix command line argument parsing in makepkg

Sebastian Nowicki wrote:
> On 22/05/2008, at 1:13 AM, Xavier wrote:
>
>> It is not eval, it is set. As far as I can tell, it is perfectly
>> normal,
>> that call seems to be made exactly for that purpose: changing the args
>> ($@, $1, etc). And I get the same behavior here anyway.
>
> I see. After some playing around it seems that this version of getopt
> simply doesn't support any parameters. Using "getopt abc $@" works
> fine. When running the test script with "-a -b -c", $@ becomes "-a -b -
> c --". If "getopt -o abs $@" is used, $@ becomes "-- abc -a -b -c", in
> which case makepkg would immediately break out of the loop.
>
>> $ cat test.sh
>> #!/bin/bash
>> echo "$@=$@"
>> args=$(getopt abc $@)
>> if [ $? != 0 ]; then
>> echo "Invalid arguments"
>> exit 1
>> fi
>> eval set -- $args
>> echo "$@=$@"
>> while true; do
>> case "$1" in
>> -a|-b|-c)
>> echo "Got $1"; shift;;
>> *)
>> echo "Invalid: $1"; break;;
>> esac
>> done
>>
>> $ ./test.sh -a -b -c
>> $@=-a -b -c
>> $@=-a -b -c --
>> Got -a
>> Got -b
>> Got -c
>> Invalid: --
>
> After changing "getopt abc $@" to "getopt -o abc $@":
>> $ ./test.sh -a -b -c
>> $@=-a -b -c
>> $@=-- abc -a -b -c
> Invalid: --
>

Because you need to use -- to separate the getopt options from the
argument, this is all in the manpage actually, I just figured that
getopt -o abc -- $@"
That is what makepkg does.

man getopt:

The parameters getopt is called with can be
divided into two parts: options which modify
the way getopt will parse (options and
-o|--options optstring in the SYNOPSIS), and
the parameters which are to be parsed (parame‐
ters in the SYNOPSIS). The second part will
start at the first non-option parameter that
is not an option argument, or after the first
occurrence of ‘--'. If no ‘-o' or ‘--options'
option is found in the first part, the first
parameter of the second part is used as the
short options string.

If the environment variable GETOPT_COMPATIBLE
is set, or if its first parameter is not an
option (does not start with a ‘-', this is the
first format in the SYNOPSIS), getopt will
generate output that is compatible with that
of other versions of getopt(1). It will still
do parameter shuffling and recognize optional
arguments (see section COMPATIBILITY for more
information).


Actually I am confused. If we want to use that compatible format, getopt
abc $@, then we can't use long options anymore?

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-22-2008, 05:33 AM
Sebastian Nowicki
 
Default Fix command line argument parsing in makepkg

On 22/05/2008, at 7:13 AM, Xavier wrote:

> Because you need to use -- to separate the getopt options from the
> argument, this is all in the manpage actually, I just figured that
> getopt -o abc -- $@"
> That is what makepkg does.
>

Yes, in GNU's getopt it works like that, but not in the BSD getopt.
Even if "getopt -o abc -- $@" is used, you still get something like
"-- abc -- -a -b -c", because it thinks that "-o abc -- $@" is what it
has to process. BSD's getopt only takes the valid short options as the
first parameter, and then the arguments to be processed [1].

> Actually I am confused. If we want to use that compatible format,
> getopt
> abc $@, then we can't use long options anymore?

It appears so, which is very bad.

[1] http://fuse4bsd.creo.hu/localcgi/man-cgi.cgi?getopt+1

--
Sebastian Nowicki


_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-22-2008, 06:01 AM
Xavier
 
Default Fix command line argument parsing in makepkg

Sebastian Nowicki wrote:
> On 22/05/2008, at 7:13 AM, Xavier wrote:
>
>> Because you need to use -- to separate the getopt options from the
>> argument, this is all in the manpage actually, I just figured that
>> getopt -o abc -- $@"
>> That is what makepkg does.
>>
>
> Yes, in GNU's getopt it works like that, but not in the BSD getopt.
> Even if "getopt -o abc -- $@" is used, you still get something like
> "-- abc -- -a -b -c", because it thinks that "-o abc -- $@" is what it
> has to process. BSD's getopt only takes the valid short options as the
> first parameter, and then the arguments to be processed [1].
>
>> Actually I am confused. If we want to use that compatible format,
>> getopt
>> abc $@, then we can't use long options anymore?
>
> It appears so, which is very bad.
>
> [1] http://fuse4bsd.creo.hu/localcgi/man-cgi.cgi?getopt+1
>

Ah I see.. As I said earlier, we were using getopts before getopt :
http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=54b71f0427e87e6d52542 3df06f8a06f2b71c518

So it basically means we need to revert that commit for portability?
The new way looks much nicer...
I would like to have Aaron and Dan inputs here :P

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-22-2008, 05:27 PM
"Aaron Griffin"
 
Default Fix command line argument parsing in makepkg

On Thu, May 22, 2008 at 1:01 AM, Xavier <shiningxc@gmail.com> wrote:
> Sebastian Nowicki wrote:
>> On 22/05/2008, at 7:13 AM, Xavier wrote:
>>
>>> Because you need to use -- to separate the getopt options from the
>>> argument, this is all in the manpage actually, I just figured that
>>> getopt -o abc -- $@"
>>> That is what makepkg does.
>>>
>>
>> Yes, in GNU's getopt it works like that, but not in the BSD getopt.
>> Even if "getopt -o abc -- $@" is used, you still get something like
>> "-- abc -- -a -b -c", because it thinks that "-o abc -- $@" is what it
>> has to process. BSD's getopt only takes the valid short options as the
>> first parameter, and then the arguments to be processed [1].
>>
>>> Actually I am confused. If we want to use that compatible format,
>>> getopt
>>> abc $@, then we can't use long options anymore?
>>
>> It appears so, which is very bad.
>>
>> [1] http://fuse4bsd.creo.hu/localcgi/man-cgi.cgi?getopt+1
>>
>
> Ah I see.. As I said earlier, we were using getopts before getopt :
> http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=54b71f0427e87e6d52542 3df06f8a06f2b71c518
>
> So it basically means we need to revert that commit for portability?
> The new way looks much nicer...
> I would like to have Aaron and Dan inputs here :P

I've been following this thread only peripherally. But my opinion is this:
getopt/getopts in bash is very confusing and annoying. Useful for only
shortopts, but once you introduce long options, it gets annoying. In
all seriousness, it might be easier just to switch to manual parsing
and eschew getopt/getopts altogether. I mean, all our options only
take one arg, right? So parsing should be fairly simply, I'd think

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 
Old 05-22-2008, 06:34 PM
Xavier
 
Default Fix command line argument parsing in makepkg

Aaron Griffin wrote:
>
> I've been following this thread only peripherally. But my opinion is this:
> getopt/getopts in bash is very confusing and annoying. Useful for only
> shortopts, but once you introduce long options, it gets annoying. In
> all seriousness, it might be easier just to switch to manual parsing
> and eschew getopt/getopts altogether. I mean, all our options only
> take one arg, right? So parsing should be fairly simply, I'd think
>

If we are considering to do this manually, I would rather re-use at
least getops, that is switch back to the old way, which was like
semi-manual

I found an alternative getopt function which might solve our problems,
but I am not sure we want to explicitly have all this complexity :
http://www.math.ias.edu/doc/bash-3.0/functions/getoptx.bash

My second favorite way would be to kill all long options. The problem is
that there are some long only option :
--asroot Allow makepkg to run as root user
--holdver Prevent automatic version bumping for development
PKGBUILDs
--source Do not build package; generate a source-only tarball
--noconfirm Do not ask for confirmation when resolving
dependencies
--noprogressbar Do not show a progress bar when downloading files

So for each of these option, we can either :
1) find a decent short option to rename it
2) kill it totally
3) find a replacement, for example a setting in makepkg.conf

What do you think?

_______________________________________________
pacman-dev mailing list
pacman-dev@archlinux.org
http://archlinux.org/mailman/listinfo/pacman-dev
 

Thread Tools




All times are GMT. The time now is 06:47 PM.

VBulletin, Copyright ©2000 - 2014, Jelsoft Enterprises Ltd.
Content Relevant URLs by vBSEO ©2007, Crawlability, Inc.
Copyright 2007 - 2008, www.linux-archive.org