[pacman-dev] Fixing command line argument parsing in makepkg

newer
[pacman-dev] [GIT] The official...

Sebastian Nowicki

19 May 2008 19 May '08

5:54 p.m.

I have not tested this on many systems, but when running into another "bug", I noticed that makepkg doesn't parse command line options correctly on Mac OS X:

...

$ sudo makepkg --asroot ==> ERROR: Running makepkg as root is a BAD idea and can cause permanent, catastrophic damage to your system. If you wish to run as root, please use the --asroot option.

I'm not sure if this has something to do with my settings, or zsh (I did test in bash), but there is a problem. After running getopt, it appears $@ gets altered. The following is the output of makepkg with an additional "echo $@" just after running getopt:

...

$ makepkg -h -- AbcCdefFghiLmop:rRsSV -l ignorearch,asroot,builddeps,clean,cleancache,nodeps,noextract,force,fo rcever:,geninteg,help,holdver,install,log,nocolor,nobuild,rmdeps,repac kage,source,syncdeps,usesudo,version,noconfirm,noprogressbar -n makepkg -- -h

makepkg proceeds as if -h was not supplied. The patch in the following mail attempts to fix this by using $ARGLIST, which does have the correct command line arguments. I did not test this patch on linux or other systems, or really tested it much at all, but the command line arguments do seem to get noticed and parsed properly. I didn't really write the patch as a solution, I just wanted to bring something to the table. I am not familiar with makepkg so it'd be best for other people to look into this. After applying the patch, $@ no longer gets shifted, and I saw use of OPT_IND, which never gets used, but it did scare me off a bit. Not shifting the command line arguments may result in regressions, but the solution provided is a simple one.

Show replies by date

Sebastian Nowicki

19 May 19 May

5:54 p.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

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 --- 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;; *) usage; exit 1 ;; # E_INVALID_OPTION esac - shift done if [ "$HOLDVER" = "1" -a "$FORCE_VER" != "" ]; then -- 1.5.4.5

Dan McGee

20 May 20 May

9:54 p.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

...

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 --- -p and --forcever have required arguments. This patch still works with

On Mon, May 19, 2008 at 10:54 AM, Sebastian Nowicki wrote: 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

Xavier

21 May 21 May

1:27 a.m.

New subject: [pacman-dev] [PATCH] 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.

Sebastian Nowicki

6:21 p.m.

New subject: [pacman-dev] [PATCH] 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

Xavier

7:13 p.m.

New subject: [pacman-dev] [PATCH] 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.

Sebastian Nowicki

10:03 p.m.

New subject: [pacman-dev] [PATCH] 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

Xavier

22 May 22 May

1:13 a.m.

New subject: [pacman-dev] [PATCH] 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?

Sebastian Nowicki

7:33 a.m.

New subject: [pacman-dev] [PATCH] 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

Xavier

8:01 a.m.

New subject: [pacman-dev] [PATCH] 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=54b71f0427e87e6d5... 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

Aaron Griffin

7:27 p.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On Thu, May 22, 2008 at 1:01 AM, Xavier 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=54b71f0427e87e6d5...

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

Xavier

8:34 p.m.

New subject: [pacman-dev] [PATCH] 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?

Sebastian Nowicki

23 May 23 May

8:30 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On 23/05/2008, at 2:34 AM, Xavier wrote:

...

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?

--noprogressbar could probably go into makepkg.conf, but I think the rest needs to stay. -H could be used for --holdver, -n or -N for -- noconfirm (the other could be used for --noprogressbar). -S is deprecated so it could be for --source instead. I'm not sure about -- asroot. There's also --forcever. -- Sebastian Nowicki

Allan McRae

9:09 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

Sebastian Nowicki wrote:

...

On 23/05/2008, at 2:34 AM, Xavier wrote:

...
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?

--noprogressbar could probably go into makepkg.conf, but I think the rest needs to stay. -H could be used for --holdver, -n or -N for -- noconfirm (the other could be used for --noprogressbar). -S is deprecated so it could be for --source instead. I'm not sure about -- asroot. There's also --forcever.

I would be in favour of a shortened version of --noconfirm. I use "makepkg -s" a lot and often wander away while the dependencies install only to come back and find pacman asking for confirmation about installing the makedepends. However, --noprogressbar would be a bit strange in makepkg.conf given it is really a pacman option.

Xavier

9:19 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On Fri, May 23, 2008 at 9:09 AM, Allan McRae wrote:

...

Sebastian Nowicki wrote:

...
--noprogressbar could probably go into makepkg.conf, but I think the rest needs to stay. -H could be used for --holdver, -n or -N for -- noconfirm (the other could be used for --noprogressbar). -S is deprecated so it could be for --source instead. I'm not sure about -- asroot. There's also --forcever.

Sounds good, thanks!

...

I would be in favour of a shortened version of --noconfirm. I use "makepkg -s" a lot and often wander away while the dependencies install only to come back and find pacman asking for confirmation about installing the makedepends.

However, --noprogressbar would be a bit strange in makepkg.conf given it is really a pacman option.

No problem, so here is the current list : --source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ? Only two left :)

Allan McRae

9:45 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

Xavier wrote:

...

On Fri, May 23, 2008 at 9:09 AM, Allan McRae wrote:

...
Sebastian Nowicki wrote:

...
--noprogressbar could probably go into makepkg.conf, but I think the rest needs to stay. -H could be used for --holdver, -n or -N for -- noconfirm (the other could be used for --noprogressbar). -S is deprecated so it could be for --source instead. I'm not sure about -- asroot. There's also --forcever.

Sounds good, thanks!

...
I would be in favour of a shortened version of --noconfirm. I use "makepkg -s" a lot and often wander away while the dependencies install only to come back and find pacman asking for confirmation about installing the makedepends.

However, --noprogressbar would be a bit strange in makepkg.conf given it is really a pacman option.

No problem, so here is the current list :

--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

Hmmm, --forcever needs to be added to the usage function, which leads me to suspect possibly the man page too but I haven't checked. I have put tidying this up in my TODO list but I will wait until the shortened versions are fully chosen. We could use -O for --asroot because root tends to have user id 0. Entirely obscure but it is the best I can come up with. We can use -F for --forcever and the INFAKEROOT option is only used internally to makepkg and thus can use a slightly more obscure abbreviation. Allan

Xavier

9:55 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On Fri, May 23, 2008 at 9:45 AM, Allan McRae wrote:

...

...
--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

Hmmm, --forcever needs to be added to the usage function, which leads me to suspect possibly the man page too but I haven't checked. I have put tidying this up in my TODO list but I will wait until the shortened versions are fully chosen.

We could use -O for --asroot because root tends to have user id 0. Entirely obscure but it is the best I can come up with. We can use -F for --forcever and the INFAKEROOT option is only used internally to makepkg and thus can use a slightly more obscure abbreviation.

Ah maybe that is why I originally skipped --forcever. It is internal option as well, so no usage function / no man page : #hidden opt used by fakeroot call for svn/cvs/etc PKGBUILDs to set pkgver --forcever) shift; FORCE_VER=$1;; But I believe we still need to find a shortopt for it. It can be obscure so it should be easy. I guess we could use -O for --asroot. It isn't a big deal, that option is not recommended anyway.

Allan McRae

10:07 a.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

Xavier wrote:

...

Ah maybe that is why I originally skipped --forcever. It is internal option as well, so no usage function / no man page : #hidden opt used by fakeroot call for svn/cvs/etc PKGBUILDs to set pkgver --forcever) shift; FORCE_VER=$1;;

I can't believe that I knew -F was and internal option without any comment and missed --forcever was when it had a comment right above it. It seems my brain has given up... Oh well, it Friday evening here and I'm going to happy hour! Allan

Xavier

28 May 28 May

2:18 p.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On Fri, May 23, 2008 at 9:19 AM, Xavier wrote:

...

No problem, so here is the current list :

--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

So what do people think about a short opt only makepkg? I would like more feedbacks on the idea as I have a patch mostly working but there is no reason to polish it if the idea is wrong and if the patch will be rejected in any cases.

Mark Constable

3:04 p.m.

New subject: [pacman-dev] Fix command line argument parsing in makepkg

On 2008-05-28, Xavier wrote:

...

...
--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

Someone else suggested -0 and -F. The only extra I would suggest is an additional check for "--help" so for those occasions when longopts do get used, for assistance, that both -h and --help shows the new shortopts usage list.

...

So what do people think about a short opt only makepkg?

I think it's a great idea and would simplify the argument passing in makepkg. Longopts are politcially correct but shortopts, especially if it streamlines code, still works just fine once the changes are understood by end users. There are 62 possibilities... a-zA-Z0-9.

...

I would like more feedbacks on the idea as I have a patch mostly working but there is no reason to polish it if the idea is wrong and if the patch will be rejected in any cases.

Not that my vote counts for anything but I'm all for it. +1 as they say --markc

Allan McRae

3:48 p.m.

New subject: [pacman-dev] Fix command line argument parsing in makepkg

Mark Constable wrote:

...

On 2008-05-28, Xavier wrote:

...
...
--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

Someone else suggested -0 and -F. The only extra I would suggest is an additional check for "--help" so for those occasions when longopts do get used, for assistance, that both -h and --help shows the new shortopts usage list.

I just did a grep though my history and I tend to always use long options for --help and --version. So I am not so sure about a short option only makepkg. I'd get used to it, but I expect all software to have those long options. Also, I would find it a bit weird passing only short options for --noprogressbar and --noconfirm which do not have short options in pacman which is their eventual target. I went back through the thread and looked at the reason for switching to short options only and it seems the cleanest solution to the problem. However, I just am not a fan of the idea. I really need to think on this some more but I'd be in favour of a dirtier solution (perhaps the addition of a specific option parser function) in order to keep them. As already discovered, you can only go so far before the one letter abbreviations become a bit obscure. Allan

Mark Constable

4:58 p.m.

New subject: [pacman-dev] Fix command line argument parsing in makepkg

On 2008-05-28, Allan McRae wrote:

...

Also, I would find it a bit weird passing only short options for --noprogressbar and --noconfirm which do not have short options in pacman which is their eventual target.

. an experienced user will get used to any new options . a new user will always have to refer to the usage or man page so whether they are intuitive or not doesn't really matter, they are whatever they are when viewing usage For example, -e and -o always confuse me and after a year I still have to look at usage to make sure what to use to not extract the the tarball or the opposite. In other words I rely on the usage to guide me constantly for less frequently used options so what ever the options are doesn't matter (to me). It's only the first most used options that have any chance of being intuitive or natural, anything after that usually, in my experience, becomes more and more obscure and requires a lookup. I simply never use longopts anyway, but that just my 2c POV. --markc

Xavier

3:48 p.m.

New subject: [pacman-dev] Fix command line argument parsing in makepkg

On Wed, May 28, 2008 at 3:04 PM, Mark Constable wrote:

...

On 2008-05-28, Xavier wrote:

...
...
--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

Someone else suggested -0 and -F. The only extra I would suggest is an additional check for "--help" so for those occasions when longopts do get used, for assistance, that both -h and --help shows the new shortopts usage list.

Yeah, I used -O and -F for the two missing ones. Oh wait, I just realize there is a confusion here between O and 0. Here is what Allan said : "We could use -O for --asroot because root tends to have user id 0." He suggested using the o letter because it looks like 0 number. In my mind, short opts were always letter too, so I followed his suggestion. I renamed the internal fakeroot -F option to -X. About --help : as long as you write an invalid option, usage will be displayed. And when using a longopt, I guess it will try to use the second - as an option which is invalid.

...

...
So what do people think about a short opt only makepkg?

I think it's a great idea and would simplify the argument passing in makepkg. Longopts are politcially correct but shortopts, especially if it streamlines code, still works just fine once the changes are understood by end users.

There are 62 possibilities... a-zA-Z0-9.

...
I would like more feedbacks on the idea as I have a patch mostly working but there is no reason to polish it if the idea is wrong and if the patch will be rejected in any cases.

Not that my vote counts for anything but I'm all for it.

+1 as they say

ok, thanks for the comments.

Sebastian Nowicki

3:43 p.m.

New subject: [pacman-dev] [PATCH] Fix command line argument parsing in makepkg

On 28/05/2008, at 8:18 PM, Xavier wrote:

...

On Fri, May 23, 2008 at 9:19 AM, Xavier wrote:

...
No problem, so here is the current list :

--source = -S --noconfirm = -n --noprogressbar = -N --holdver = -H --forcever = ? --asroot = ?

So what do people think about a short opt only makepkg? I would like more feedbacks on the idea as I have a patch mostly working but there is no reason to polish it if the idea is wrong and if the patch will be rejected in any cases.

I was about to start working on that, but I guess I have more time to study now :P. I think it's fine as long as there isn't much breakage in other apps, although that shouldn't really be our concern. -- Sebastian Nowicki

Dan McGee

20 May 20 May

9:58 p.m.

On Mon, May 19, 2008 at 10:54 AM, Sebastian Nowicki wrote:

...

I have not tested this on many systems, but when running into another "bug", I noticed that makepkg doesn't parse command line options correctly on Mac OS X:

...
$ sudo makepkg --asroot ==> ERROR: Running makepkg as root is a BAD idea and can cause permanent, catastrophic damage to your system. If you wish to run as root, please use the --asroot option.

I'm not sure if this has something to do with my settings, or zsh (I did test in bash), but there is a problem. After running getopt, it appears $@ gets altered. The following is the output of makepkg with an additional "echo $@" just after running getopt:

...
$ makepkg -h -- AbcCdefFghiLmop:rRsSV -l ignorearch,asroot,builddeps,clean,cleancache,nodeps,noextract,force,fo rcever:,geninteg,help,holdver,install,log,nocolor,nobuild,rmdeps,repac kage,source,syncdeps,usesudo,version,noconfirm,noprogressbar -n makepkg -- -h

makepkg proceeds as if -h was not supplied.

The patch in the following mail attempts to fix this by using $ARGLIST, which does have the correct command line arguments. I did not test this patch on linux or other systems, or really tested it much at all, but the command line arguments do seem to get noticed and parsed properly. I didn't really write the patch as a solution, I just wanted to bring something to the table. I am not familiar with makepkg so it'd be best for other people to look into this.

After applying the patch, $@ no longer gets shifted, and I saw use of OPT_IND, which never gets used, but it did scare me off a bit. Not shifting the command line arguments may result in regressions, but the solution provided is a simple one.

I just replied to your patch email, but did not mention when OPT_IND actually gets used. I believe the idea was that any remaining args to makepkg would get passed to pacman when/if it is called? But I may just be crazy there. -Dan

Xavier

21 May 21 May

1:24 a.m.

Dan McGee wrote:

...

I just replied to your patch email, but did not mention when OPT_IND actually gets used. I believe the idea was that any remaining args to makepkg would get passed to pacman when/if it is called? But I may just be crazy there.

I have no idea what is that stuff for, and anyone is welcome to investigate :) Just some history I found today : http://projects.archlinux.org/?p=pacman.git;a=commitdiff;h=54b71f0427e87e6d5... So before we were using getops / OPTIND. After that commit, we used getopt / OPT_IND. man getopts seems to mention what OPTIND is for. man getopt doesnt say anything though...

5819

Age (days ago)

5828

Last active (days ago)

List overview

Download

25 comments

6 participants

participants (6)

Aaron Griffin
Allan McRae
Dan McGee
Mark Constable
Sebastian Nowicki
Xavier