For discussion of Alpine Linux development and developer support

6 2

[alpine-devel] [PATCH 3/4] update-conf: accept long options

Dubiousjim
Details
Message ID
<6d0295ddc265976445cb2d46c6194db304e92386.1372713126.git.dubiousjim@gmail.com>
Sender timestamp
1372713256
DKIM signature
missing
Download raw message
Patch: +25 -15
---
 update-conf.in | 40 +++++++++++++++++++++++++---------------
 1 file changed, 25 insertions(+), 15 deletions(-)

diff --git a/update-conf.in b/update-conf.in
index 6a8ab16..da4208c 100644
--- a/update-conf.in
+++ b/update-conf.in
@@ -10,14 +10,16 @@ init_tmpdir TMPD
 LBUCACHE="$TMPD/lbucache"
 
 usage() {
-	echo "$PROGRAM $VERSION
-Usage: $PROGAM [-aihl]
+	cat >&2 << __EOF__
+$PROGRAM $VERSION - TODO what is it?
+Usage: $PROGRAM [-a|--all] [-i|--initd] [-l|--list] [-h|--help]
+Options:
+  -a, --all    Select all updated files
+  -i, --initd  Use all new init.d scripts
+  -l, --list   List updated files
+  -h, --help   Show this help
 
-  -a  Select all updated files.
-  -h  Show this help.
-  -i  Use all new init.d scripts.
-  -l  List updated files.
-"	
+__EOF__
 }
 
 
@@ -31,15 +33,23 @@ is_initd() {
 	echo "$1" | grep etc/init.d/ > /dev/null
 }
 
-while getopts "alih" opt ; do
-	case "$opt" in
-		a)	aflag="-a" ;;
-		i)	iflag="-i" ;;
-		l)	lflag="-l" ;;
-		h|*)	usage;;
-	esac	
+args=`getopt -o ailh --long all,initd,list,help -n "$PROGRAM" -- "$@"`
+if [ $? -ne 0 ]; then
+	usage
+	exit 2
+fi
+eval set -- "$args"
+while true; do
+	case $1 in
+		-a|--all) aflag="-a";;
+		-i|--initd) iflag="-i";;
+		-l|--list) lflag="-l";;
+		-h|--help) usage; exit;;
+		--) shift; break;;
+		*) exit 1;; # getopt error
+	esac
+	shift
 done
-shift `expr $OPTIND - 1`
 
 for apknew in $(find "$ROOT/etc" -name '*.apk-new') ; do
 	p="${apknew%.apk-new}"
-- 
1.8.3.1



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Dubiousjim
Details
Message ID
<20130701212019.GD1550@zen>
In-Reply-To
<6d0295ddc265976445cb2d46c6194db304e92386.1372713126.git.dubiousjim@gmail.com> (view parent)
Sender timestamp
1372713619
DKIM signature
missing
Download raw message
On Mon, Jul 01, 2013 at 05:14:16PM -0400, Dubiousjim wrote:
> ---
>  update-conf.in | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/update-conf.in b/update-conf.in
> index 6a8ab16..da4208c 100644
> --- a/update-conf.in
> +++ b/update-conf.in
> @@ -10,14 +10,16 @@ init_tmpdir TMPD
>  LBUCACHE="$TMPD/lbucache"
>  
>  usage() {
> -	echo "$PROGRAM $VERSION
> -Usage: $PROGAM [-aihl]
> +	cat >&2 << __EOF__
> +$PROGRAM $VERSION - TODO what is it?

Whoops, so sorry I let that in there. The last quoted line should just be:

+$PROGRAM @VERSION@

-- 
Dubiousjim
dubiousjim@gmail.com


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Dubiousjim
Details
Message ID
<20130703140910.GE1550@zen>
In-Reply-To
<20130703153033.5230023e@ncopa-desktop.alpinelinux.org> (view parent)
Sender timestamp
1372860550
DKIM signature
missing
Download raw message
On Wed, Jul 03, 2013 at 03:30:33PM +0200, Natanael Copa wrote:
> On Mon,  1 Jul 2013 17:14:16 -0400
> Dubiousjim <dubiousjim@gmail.com> wrote:

> > -while getopts "alih" opt ; do
> > -	case "$opt" in
> > -		a)	aflag="-a" ;;
> > -		i)	iflag="-i" ;;
> > -		l)	lflag="-l" ;;
> > -		h|*)	usage;;
> > -	esac	
> > +args=`getopt -o ailh --long all,initd,list,help -n "$PROGRAM" -- "$@"`
> 
> I don't think this is posix shell compatible.
> 
> We already use various extensions like 'local' and shell replace
> expansion ${var/search/replace} so we already depend on an extended shell.
> 
> I think this is ok for now, but we should be aware of that this is
> moving away from posix shell compat.
> 
> Is long options worth it?

I've got a bunch of patches for the abuild package too that I'm cleaning up,
including long options for many of the small scripts.

I'm pretty sure you're right that POSIX doesn't require any getopt shell
command or utility. So this is extra-POSIX. BusyBox (as we configure it)
provides getopt as an external utility, so in our case we do go beyond
Posix but we're not thereby requiring an extra-POSIX *shell*.

Perhaps Bash implements getopt as an internal shell command, I don't
remember.

> I also made a posix shell implementation for parsing options for lxc
> templates which we could use instead.

Where is that?

-- 
Dubiousjim
dubiousjim@gmail.com


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Natanael Copa
Details
Message ID
<20130703153033.5230023e@ncopa-desktop.alpinelinux.org>
In-Reply-To
<6d0295ddc265976445cb2d46c6194db304e92386.1372713126.git.dubiousjim@gmail.com> (view parent)
Sender timestamp
1372858233
DKIM signature
missing
Download raw message
On Mon,  1 Jul 2013 17:14:16 -0400
Dubiousjim <dubiousjim@gmail.com> wrote:

> ---
>  update-conf.in | 40 +++++++++++++++++++++++++---------------
>  1 file changed, 25 insertions(+), 15 deletions(-)
> 
> diff --git a/update-conf.in b/update-conf.in
> index 6a8ab16..da4208c 100644
> --- a/update-conf.in
> +++ b/update-conf.in
> @@ -10,14 +10,16 @@ init_tmpdir TMPD
>  LBUCACHE="$TMPD/lbucache"
>  
>  usage() {
> -	echo "$PROGRAM $VERSION
> -Usage: $PROGAM [-aihl]
> +	cat >&2 << __EOF__
> +$PROGRAM $VERSION - TODO what is it?
> +Usage: $PROGRAM [-a|--all] [-i|--initd] [-l|--list] [-h|--help]
> +Options:
> +  -a, --all    Select all updated files
> +  -i, --initd  Use all new init.d scripts
> +  -l, --list   List updated files
> +  -h, --help   Show this help
>  
> -  -a  Select all updated files.
> -  -h  Show this help.
> -  -i  Use all new init.d scripts.
> -  -l  List updated files.
> -"	
> +__EOF__
>  }
>  
>  
> @@ -31,15 +33,23 @@ is_initd() {
>  	echo "$1" | grep etc/init.d/ > /dev/null
>  }
>  
> -while getopts "alih" opt ; do
> -	case "$opt" in
> -		a)	aflag="-a" ;;
> -		i)	iflag="-i" ;;
> -		l)	lflag="-l" ;;
> -		h|*)	usage;;
> -	esac	
> +args=`getopt -o ailh --long all,initd,list,help -n "$PROGRAM" -- "$@"`

I don't think this is posix shell compatible.

We already use various extensions like 'local' and shell replace
expansion ${var/search/replace} so we already depend on an extended shell.

I think this is ok for now, but we should be aware of that this is
moving away from posix shell compat.

Is long options worth it?

I also made a posix shell implementation for parsing options for lxc
templates which we could use instead.

-nc


> +if [ $? -ne 0 ]; then
> +	usage
> +	exit 2
> +fi
> +eval set -- "$args"
> +while true; do
> +	case $1 in
> +		-a|--all) aflag="-a";;
> +		-i|--initd) iflag="-i";;
> +		-l|--list) lflag="-l";;
> +		-h|--help) usage; exit;;
> +		--) shift; break;;
> +		*) exit 1;; # getopt error
> +	esac
> +	shift
>  done
> -shift `expr $OPTIND - 1`
>  
>  for apknew in $(find "$ROOT/etc" -name '*.apk-new') ; do
>  	p="${apknew%.apk-new}"



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Natanael Copa
Details
Message ID
<20130703202009.29625ede@ncopa-laptop.res.nor.wtbts.net>
In-Reply-To
<20130703140910.GE1550@zen> (view parent)
Sender timestamp
1372875609
DKIM signature
missing
Download raw message
On Wed, 3 Jul 2013 10:09:10 -0400
Dubiousjim <dubiousjim@gmail.com> wrote:

...
 
> I've got a bunch of patches for the abuild package too that I'm cleaning up,
> including long options for many of the small scripts.
> 
> I'm pretty sure you're right that POSIX doesn't require any getopt shell
> command or utility. So this is extra-POSIX. BusyBox (as we configure it)
> provides getopt as an external utility, so in our case we do go beyond
> Posix but we're not thereby requiring an extra-POSIX *shell*.

hm 'getops' is in posix.
 
> Perhaps Bash implements getopt as an internal shell command, I don't
> remember.

I think we can use getopt with long opts. it will save us for some work.

> > I also made a posix shell implementation for parsing options for lxc
> > templates which we could use instead.
> 
> Where is that?

https://github.com/lxc/lxc/commit/e60a8164c12d565f70071ff6b32b823dd495df9e

Basically all the src/lxc/*.in scripts in there.

-nc

PS. I'm really happy for your review of the code, questioning the old
code and patches for improvements. Thanks!



---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Natanael Copa
Details
Message ID
<20130703202233.301e382a@ncopa-laptop.res.nor.wtbts.net>
In-Reply-To
<20130703202009.29625ede@ncopa-laptop.res.nor.wtbts.net> (view parent)
Sender timestamp
1372875753
DKIM signature
missing
Download raw message
On Wed, 3 Jul 2013 20:20:09 +0200
Natanael Copa <ncopa@alpinelinux.org> wrote:
 
> hm 'getops' is in posix.

http://pubs.opengroup.org/onlinepubs/009695399/utilities/getopts.html

but its inferior to 'getopt'

-nc


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---
Dubiousjim
Details
Message ID
<20130704004400.GJ1550@zen>
In-Reply-To
<20130703202233.301e382a@ncopa-laptop.res.nor.wtbts.net> (view parent)
Sender timestamp
1372898640
DKIM signature
missing
Download raw message
On Wed, Jul 03, 2013 at 08:22:33PM +0200, Natanael Copa wrote:
> On Wed, 3 Jul 2013 20:20:09 +0200
> Natanael Copa <ncopa@alpinelinux.org> wrote:
>  
> > hm 'getops' is in posix.
> 
> http://pubs.opengroup.org/onlinepubs/009695399/utilities/getopts.html
> 
> but its inferior to 'getopt'

Right, that's for short-option processing only.

-- 
Dubiousjim
dubiousjim@gmail.com


---
Unsubscribe:  alpine-devel+unsubscribe@lists.alpinelinux.org
Help:         alpine-devel+help@lists.alpinelinux.org
---