[PATCH 1/3] tests: udevadm settle before losetup -d

[PATCH 1/3] tests: udevadm settle before losetup -d

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Fails on Debian 7 (wheezy), Kernel 3.2.

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
CC: Stanislav Brabec <sbrabec@suse.cz>
---
 tests/ts/losetup/losetup-loop | 10 ++++++++++
 1 file changed, 10 insertions(+)

diff --git a/tests/ts/losetup/losetup-loop b/tests/ts/losetup/losetup-loop
index 839f100..310e76e 100755
--- a/tests/ts/losetup/losetup-loop
+++ b/tests/ts/losetup/losetup-loop
@@ -43,12 +43,14 @@ BACKFILE="$TS_DEVICE"
 ts_init_subtest "find-race-condition"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
 $TS_CMD_LOSETUP -d $LODEV
+# The loop device may or may not exist here because no "udevadm settle".
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
 sleep 3
 dd if=/dev/zero of=$LODEV count=1 bs=1 >/dev/null 2>&1
 sleep 3
 $TS_CMD_LOSETUP --list | grep -q $LODEV
 ts_log $?
+udevadm settle
 $TS_CMD_LOSETUP -d $LODEV >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
@@ -64,6 +66,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -87,6 +90,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=1MiB --sizelimit=1M
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -110,6 +114,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=2MiB --sizelimit=2M
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -133,6 +138,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=2MiB --sizelimit=2M
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -156,6 +162,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=2MiB --sizelimit=2M
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -180,6 +187,7 @@ LODEVR=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=2MiB $BACKFILE )
 if [ -z "$LODEVR" ]; then
 	ts_log "Failed to create loop device"
 fi
+udevadm settle
 if test "$LODEV" = "$LODEVR" ; then
 	echo "equal" >>$TS_OUTPUT
 else
@@ -202,6 +210,7 @@ fi
 LODEVR=$( $TS_CMD_LOSETUP --find )
 $TS_CMD_LOSETUP --nooverlap $LODEVR $BACKFILE >/dev/null 2>&1
 ts_log $?
+udevadm settle
 $TS_CMD_LOSETUP -d $LODEV
 $TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
 ts_log "Success"
@@ -217,6 +226,7 @@ fi
 LODEVR=$( $TS_CMD_LOSETUP --find )
 $TS_CMD_LOSETUP --nooverlap $LODEVR $BACKFILE >/dev/null 2>&1
 ts_log $?
+udevadm settle
 $TS_CMD_LOSETUP -d $LODEV
 $TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
 ts_log "Success"
-- 
1.8.5.6

[PATCH 2/3] tests: replace sleep by udevadm

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
CC: Stanislav Brabec <sbrabec@suse.cz>
---
 tests/ts/losetup/losetup-loop | 21 ++++++++++-----------
 1 file changed, 10 insertions(+), 11 deletions(-)

diff --git a/tests/ts/losetup/losetup-loop b/tests/ts/losetup/losetup-loop
index 310e76e..cff12c9 100755
--- a/tests/ts/losetup/losetup-loop
+++ b/tests/ts/losetup/losetup-loop
@@ -45,9 +45,8 @@ LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
 $TS_CMD_LOSETUP -d $LODEV
 # The loop device may or may not exist here because no "udevadm settle".
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
-sleep 3
+udevadm settle
 dd if=/dev/zero of=$LODEV count=1 bs=1 >/dev/null 2>&1
-sleep 3
 $TS_CMD_LOSETUP --list | grep -q $LODEV
 ts_log $?
 udevadm settle
@@ -55,7 +54,7 @@ $TS_CMD_LOSETUP -d $LODEV >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-re-use"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
@@ -79,7 +78,7 @@ $TS_CMD_LOSETUP -d $LODEV
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-no-re-use"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=0 --sizelimit=1MiB $BACKFILE )
@@ -103,7 +102,7 @@ $TS_CMD_LOSETUP -d $LODEV
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-conflict"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=1MiB --sizelimit=2MiB $BACKFILE )
@@ -127,7 +126,7 @@ $TS_CMD_LOSETUP -d $LODEV
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-conflict-no-sizelimit"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=1MiB $BACKFILE )
@@ -151,7 +150,7 @@ $TS_CMD_LOSETUP -d $LODEV
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-conflict-readonly"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --read-only $BACKFILE )
@@ -176,7 +175,7 @@ $TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "find-ok-no-sizelimit"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --offset=1MiB --sizelimit=1MiB $BACKFILE )
@@ -200,7 +199,7 @@ $TS_CMD_LOSETUP -d $LODEV
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "plain-conflict"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
@@ -216,7 +215,7 @@ $TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_init_subtest "plain-readonly"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show --read-only $BACKFILE )
@@ -232,7 +231,7 @@ $TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
-sleep 3
+udevadm settle
 
 ts_log "Success"
 ts_finalize
-- 
1.8.5.6

Re: [PATCH 2/3] tests: replace sleep by udevadm

[Stanislav Brabec]

Ruediger Meier wrotek:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
>
> -sleep 3
> +udevadm settle

Thanks for the research. I was not aware that the race comes from udev.

Do you think that it would make sense to add udevadm settle directly to 
losetup?

-- 
Best Regards / S pozdravem,

Stanislav Brabec
software developer
---------------------------------------------------------------------
SUSE LINUX, s. r. o.                         e-mail: sbrabec@suse.com
Křižíkova 148/34 (Corso IIa)                  tel: +49 911 7405384547
186 00 Praha 8-Karlín                          fax:  +420 284 084 001
Czech Republic                                    http://www.suse.cz/
PGP: 830B 40D5 9E05 35D8 5E27 6FA3 717C 209F A04F CD76

[PATCH 3/3] tests: always quote $LODEV

[Ruediger Meier]

From: Ruediger Meier <ruediger.meier@ga-group.nl>

Since there is no error handling in this test $LODEV
may be empty.

Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
---
 tests/ts/losetup/losetup-loop | 46 +++++++++++++++++++++----------------------
 1 file changed, 23 insertions(+), 23 deletions(-)

diff --git a/tests/ts/losetup/losetup-loop b/tests/ts/losetup/losetup-loop
index cff12c9..13ccc65 100755
--- a/tests/ts/losetup/losetup-loop
+++ b/tests/ts/losetup/losetup-loop
@@ -42,15 +42,15 @@ BACKFILE="$TS_DEVICE"
 
 ts_init_subtest "find-race-condition"
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 # The loop device may or may not exist here because no "udevadm settle".
 LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
 udevadm settle
-dd if=/dev/zero of=$LODEV count=1 bs=1 >/dev/null 2>&1
-$TS_CMD_LOSETUP --list | grep -q $LODEV
+dd if=/dev/zero of="$LODEV" count=1 bs=1 >/dev/null 2>&1
+$TS_CMD_LOSETUP --list | grep -q "$LODEV"
 ts_log $?
 udevadm settle
-$TS_CMD_LOSETUP -d $LODEV >/dev/null 2>&1
+$TS_CMD_LOSETUP -d "$LODEV" >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
@@ -71,10 +71,10 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 ts_log "Success"
 ts_finalize_subtest
 
@@ -95,10 +95,10 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 ts_log "Success"
 ts_finalize_subtest
 
@@ -119,10 +119,10 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 ts_log "Success"
 ts_finalize_subtest
 
@@ -143,10 +143,10 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 ts_log "Success"
 ts_finalize_subtest
 
@@ -167,11 +167,11 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
-$TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
+$TS_CMD_LOSETUP -d "$LODEV"
+$TS_CMD_LOSETUP -d "$LODEVR" >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
@@ -192,10 +192,10 @@ if test "$LODEV" = "$LODEVR" ; then
 else
 	echo "different" >>$TS_OUTPUT
 	if test -n "$LODEVR" ; then
-		$TS_CMD_LOSETUP -d $LODEVR
+		$TS_CMD_LOSETUP -d "$LODEVR"
 	fi
 fi
-$TS_CMD_LOSETUP -d $LODEV
+$TS_CMD_LOSETUP -d "$LODEV"
 ts_log "Success"
 ts_finalize_subtest
 
@@ -207,11 +207,11 @@ if [ -z "$LODEV" ]; then
 	ts_log "Failed to create loop device"
 fi
 LODEVR=$( $TS_CMD_LOSETUP --find )
-$TS_CMD_LOSETUP --nooverlap $LODEVR $BACKFILE >/dev/null 2>&1
+$TS_CMD_LOSETUP --nooverlap "$LODEVR" $BACKFILE >/dev/null 2>&1
 ts_log $?
 udevadm settle
-$TS_CMD_LOSETUP -d $LODEV
-$TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
+$TS_CMD_LOSETUP -d "$LODEV"
+$TS_CMD_LOSETUP -d "$LODEVR" >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
@@ -223,11 +223,11 @@ if [ -z "$LODEV" ]; then
 	ts_log "Failed to create loop device"
 fi
 LODEVR=$( $TS_CMD_LOSETUP --find )
-$TS_CMD_LOSETUP --nooverlap $LODEVR $BACKFILE >/dev/null 2>&1
+$TS_CMD_LOSETUP --nooverlap "$LODEVR" $BACKFILE >/dev/null 2>&1
 ts_log $?
 udevadm settle
-$TS_CMD_LOSETUP -d $LODEV
-$TS_CMD_LOSETUP -d $LODEVR >/dev/null 2>&1
+$TS_CMD_LOSETUP -d "$LODEV"
+$TS_CMD_LOSETUP -d "$LODEVR" >/dev/null 2>&1
 ts_log "Success"
 ts_finalize_subtest
 
-- 
1.8.5.6

Re: [PATCH 3/3] tests: always quote $LODEV

[Bernhard Voelker]

On 11/05/2016 04:08 PM, Ruediger Meier wrote:
> From: Ruediger Meier <ruediger.meier@ga-group.nl>
> 
> Since there is no error handling in this test $LODEV
> may be empty.
> 
> Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> ---
>  tests/ts/losetup/losetup-loop | 46 +++++++++++++++++++++----------------------
>  1 file changed, 23 insertions(+), 23 deletions(-)
> 
> diff --git a/tests/ts/losetup/losetup-loop b/tests/ts/losetup/losetup-loop
> index cff12c9..13ccc65 100755
> --- a/tests/ts/losetup/losetup-loop
> +++ b/tests/ts/losetup/losetup-loop
> @@ -42,15 +42,15 @@ BACKFILE="$TS_DEVICE"
>  
>  ts_init_subtest "find-race-condition"
>  LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
> -$TS_CMD_LOSETUP -d $LODEV
> +$TS_CMD_LOSETUP -d "$LODEV"

So what happens now when losetup is fed with ""?
Wouldn't it be better to handle the error instead?

Have a nice day,
Berny

Re: [PATCH 3/3] tests: always quote $LODEV

[Ruediger Meier]

On Sunday 06 November 2016, Bernhard Voelker wrote:
> On 11/05/2016 04:08 PM, Ruediger Meier wrote:
> > From: Ruediger Meier <ruediger.meier@ga-group.nl>
> >
> > Since there is no error handling in this test $LODEV
> > may be empty.
> >
> > Signed-off-by: Ruediger Meier <ruediger.meier@ga-group.nl>
> > ---
> >  tests/ts/losetup/losetup-loop | 46
> > +++++++++++++++++++++---------------------- 1 file changed, 23
> > insertions(+), 23 deletions(-)
> >
> > diff --git a/tests/ts/losetup/losetup-loop
> > b/tests/ts/losetup/losetup-loop index cff12c9..13ccc65 100755
> > --- a/tests/ts/losetup/losetup-loop
> > +++ b/tests/ts/losetup/losetup-loop
> > @@ -42,15 +42,15 @@ BACKFILE="$TS_DEVICE"
> >
> >  ts_init_subtest "find-race-condition"
> >  LODEV=$( $TS_CMD_LOSETUP --find --nooverlap --show $BACKFILE )
> > -$TS_CMD_LOSETUP -d $LODEV
> > +$TS_CMD_LOSETUP -d "$LODEV"
>
> So what happens now when losetup is fed with ""?
> Wouldn't it be better to handle the error instead?

Actually the failure is generally handled in terms that the test will 
fail and exit 1. I did not want to change more of the test logic 
shortly before release. Also note that we are testing a possibly broken 
losetup here which makes any "good" error handling questionable 
anyways. For example if something goes wrong it would be of course nice 
to do at least some cleanup and remove all loop devices again to not 
let other tests fail too. But how should we do that if losetup is 
broken?

Regarding quoting. IMO we are missing a lot quotation marks in our 
test-suite. This is specially annoying since our programs are printing 
the whole --help output on usage errors. I really hate that auto-help 
feature. Don't understand why we are doing that instead of just 
printing "unkown option" or "missing argument".

cu,
Rudi

Re: [PATCH 3/3] tests: always quote $LODEV

[Karel Zak]

On Mon, Nov 07, 2016 at 09:58:25AM +0100, Ruediger Meier wrote:
> On Sunday 06 November 2016, Bernhard Voelker wrote:
> > So what happens now when losetup is fed with ""?
> > Wouldn't it be better to handle the error instead?
> 
> Actually the failure is generally handled in terms that the test will 
> fail and exit 1. 

This is the basic idea of the tests; it would be nice to keep it
simple and stupid :-)

> the whole --help output on usage errors. I really hate that auto-help 
> feature. Don't understand why we are doing that instead of just 
> printing "unkown option" or "missing argument".

I'd like to change it for v2.30.

Optionally, it would be also nice to have a way how to suggest the
correct option on typos. I like the way how git react on typos:

        $ git cxmmit
        git: 'cxmmit' is not a git command. See 'git --help'.

        Did you mean this?
            commit



    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/3] tests: always quote $LODEV

[Ruediger Meier]

On Monday 07 November 2016, Karel Zak wrote:
> On Mon, Nov 07, 2016 at 09:58:25AM +0100, Ruediger Meier wrote:
> > On Sunday 06 November 2016, Bernhard Voelker wrote:
> > > So what happens now when losetup is fed with ""?
> > > Wouldn't it be better to handle the error instead?
> >
> > Actually the failure is generally handled in terms that the test
> > will fail and exit 1.
>
> This is the basic idea of the tests; it would be nice to keep it
> simple and stupid :-)
>
> > the whole --help output on usage errors. I really hate that
> > auto-help feature. Don't understand why we are doing that instead
> > of just printing "unkown option" or "missing argument".
>
> I'd like to change it for v2.30.
>
> Optionally, it would be also nice to have a way how to suggest the
> correct option on typos. I like the way how git react on typos:
>
>         $ git cxmmit
>         git: 'cxmmit' is not a git command. See 'git --help'.
>
>         Did you mean this?
>             commit
>

This is nice, allthough for getopt(3) option parsing this functionality 
would IMO belong to getopt() itself. Like we have already this
 
$ mount --make
mount: option '--make' is ambiguous; 
possibilities: '--make-shared' '--make-runbindable' '--make-rprivate' ...

Maybe just patch gnulib/getopt?

BTW would be also very nice to ensure by design that --help, manpage and 
the actual commands are always synchron. For my own projects I've 
used "gengetopt" for this. Unfortunately it has some unfixed bugs and 
it seems unmaintained. One of my colleagues has a nice project "yuck" 
adressing this, http://www.fresse.org/yuck/ It's basically a 
replacement for gengetopt and help2man.

I could send a patch to introduce yuck in UL to see how it would look. 
Maybe I could also convince my colleague to add such generic 
option-guessing feature.

cu,
Rudi

Re: [PATCH 3/3] tests: always quote $LODEV

[Karel Zak]

On Mon, Nov 07, 2016 at 11:21:55AM +0100, Ruediger Meier wrote:
> > > auto-help feature. Don't understand why we are doing that instead
> > > of just printing "unkown option" or "missing argument".
> >
> > I'd like to change it for v2.30.
> >
> > Optionally, it would be also nice to have a way how to suggest the
> > correct option on typos. I like the way how git react on typos:
> >
> >         $ git cxmmit
> >         git: 'cxmmit' is not a git command. See 'git --help'.
> >
> >         Did you mean this?
> >             commit
> >
> 
> This is nice, allthough for getopt(3) option parsing this functionality 
> would IMO belong to getopt() itself. Like we have already this
>  
> $ mount --make
> mount: option '--make' is ambiguous; 
> possibilities: '--make-shared' '--make-runbindable' '--make-rprivate' ...
>
> Maybe just patch gnulib/getopt?

We don't have a problem with getopt_long() from libc, it works as
expected and it's good enough, so I don't think we need some 
complete fallback like gnulib/getopt. 

All we need is to:

 * print error message on unrecognized option (usually done by libc)
 * suggest --help
 * maybe detect typo and suggest correct option (unimportant)

The most trivial way would be to add ul_getopt_long() to c.h and
remove "default:" cases from getopt blocks. In the function we can do
all the funny things.

> BTW would be also very nice to ensure by design that --help, manpage and 
> the actual commands are always synchron.

It depends what do you mean with synchron. All options should be
documented by --help and man page, but the description should be
different. For --help we want really minimalistic and easy to
translate description. The man page is unlimited area for creative
writers :-)

> For my own projects I've 
> used "gengetopt" for this. Unfortunately it has some unfixed bugs and 
> it seems unmaintained. One of my colleagues has a nice project "yuck" 
> adressing this, http://www.fresse.org/yuck/ It's basically a 
> replacement for gengetopt and help2man.

Frankly, all we need is to detect mistakes (undocumented options).

I'd like to be conservative and avoid complex solutions. I think our
--help is nice and works as expected :-)

This is "color of the bikeshed" topic :-) I'm sure there s many ways
and many solutions, but we have no any real problem in this area. All
we need is to avoid auto-help. That's all.

> I could send a patch to introduce yuck in UL to see how it would look. 

Well, I see many another more important tasks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/3] tests: always quote $LODEV

[Ruediger Meier]

On Monday 07 November 2016, Karel Zak wrote:
> On Mon, Nov 07, 2016 at 11:21:55AM +0100, Ruediger Meier wrote:

> > BTW would be also very nice to ensure by design that --help,
> > manpage and the actual commands are always synchron.
>
> It depends what do you mean with synchron. All options should be
> documented by --help and man page, [...]

Yes it should, but we have very often inconsistencies, like

89de71b3
3fabc363
21c6bcbd
ce969b16

Bugs like these are IMO waste of human life time. They can be avoided 
automatically from the beginning.

> For --help we want really minimalistic and easy to 
> translate description. The man page is unlimited area for creative
> writers :-)

It's not a big problem to add manually written text to the autogenerated 
one. Even help2man has such interface (--include and --opt-include).

> Frankly, all we need is to detect mistakes (undocumented options).

Why would you want to detect this but not want to use existing tools?

> I'd like to be conservative and avoid complex solutions. I think our
> --help is nice and works as expected :-)

Even I've just run a stupid shell command line to find out that our 
getopt and flock have broken --help implementations. They print to 
stderr or do not return 0. I'm sure that we would find much more 
problems with the non-trivial options.

whereis has no such options --help/--version. Is this on purpose?

cu,
Rudi

Re: [PATCH 3/3] tests: always quote $LODEV

[Karel Zak]

On Mon, Nov 07, 2016 at 01:48:23PM +0100, Ruediger Meier wrote:
> On Monday 07 November 2016, Karel Zak wrote:
> > On Mon, Nov 07, 2016 at 11:21:55AM +0100, Ruediger Meier wrote:
> 
> > > BTW would be also very nice to ensure by design that --help,
> > > manpage and the actual commands are always synchron.
> >
> > It depends what do you mean with synchron. All options should be
> > documented by --help and man page, [...]
> 
> Yes it should, but we have very often inconsistencies, like
> 
> 89de71b3
> 3fabc363
> 21c6bcbd
> ce969b16
> 
> Bugs like these are IMO waste of human life time. They can be avoided 
> automatically from the beginning.

Depends how expensive (maintenance overhead, extra changes in code)
will be the smart solution. The nice thing on getopt_long is that it
does not add any extra abstract layer -- it just tokenize command 
line and all is in your hands.

> > Frankly, all we need is to detect mistakes (undocumented options).
> 
> Why would you want to detect this but not want to use existing tools?

when I think about external tool, then:

 * is it commonly used tool?
 * is it fine to depend on the tool from bootstrap point of view?
   (for example gtk-docs is absolutely optional and unnecessary for users).
 * is it easy to integrate to our code?
 * how expensive is to maintain it?

For example if there is way how to generate the getopt stuff by m4
without any additional dependence, it's easy to maintain (include
translations by .po files) then it sounds good.

If you believe that yuck is the right way then send a patch for some
new tool (e.g. lsns) and we can test it and try it -- but don't forget
I'm stubborn for such things :-)

> > I'd like to be conservative and avoid complex solutions. I think our
> > --help is nice and works as expected :-)
> 
> Even I've just run a stupid shell command line to find out that our 
> getopt and flock have broken --help implementations.

Bug, like many others.

> stderr or do not return 0. I'm sure that we would find much more 
> problems with the non-trivial options.
> 
> whereis has no such options --help/--version. Is this on purpose?

We maintain many random tools with very different history, not all is
already completely refactored to be compatible with our "current
standard".

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 3/3] tests: always quote $LODEV

[Karel Zak]

On Mon, Nov 07, 2016 at 02:38:10PM +0100, Karel Zak wrote:
> For example if there is way how to generate the getopt stuff by m4
> without any additional dependence, it's easy to maintain (include
> translations by .po files) then it sounds good.

BTW, does yuck generates parser where the result is always yuck
specific struct or you have control on the parsers code and the result
in your hands?

I mean something like GNU Bison (yacc) where the final parser is "syntax
description" + "your code". It would be nice to have

 -f, --foo     This is foo description
 {
    /* code to handle foo */
    mystruct->foo = 1;
    printf("Nice you send foo");
 }

 -c, --ccc     This is coo description
 {
    /* my code for ccc */
    ...
 }


    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com

Re: [PATCH 1/3] tests: udevadm settle before losetup -d

[Karel Zak]

On Sat, Nov 05, 2016 at 04:08:07PM +0100, Ruediger Meier wrote:
>  tests/ts/losetup/losetup-loop | 10 ++++++++++
>  1 file changed, 10 insertions(+)

 All set applied, thanks.

    Karel

-- 
 Karel Zak  <kzak@redhat.com>
 http://karelzak.blogspot.com