BTS

Issue431

Title wm-ng on grml-medium 0.1 [grml-exec-wrapper]
Priority bug Status resolved
Superseder Nosy List ft, maddi, mika
Assigned To mika Topics release-stopper

Created on 2008-03-06.15:54:49 by maddi, last changed 2009-04-06.08:00:49 by mika.

Messages
msg2133 (view) Author: mika Date: 2009-04-06.08:00:48
We believe that your issue has been closed by the upload of
Version 0.2.3 of grml-desktop from Michael Prokop <mika@grml.org>.
The explanation is attached below

-----BEGIN PGP SIGNED MESSAGE-----
Hash: SHA1

Format: 1.8
Date: Mon, 06 Apr 2009 09:50:08 +0200
Source: grml-desktop
Binary: grml-desktop
Architecture: source all
Version: 0.2.3
Distribution: unstable
Urgency: low
Maintainer: Michael Prokop <mika@grml.org>
Changed-By: Michael Prokop <mika@grml.org>
Description: 
 grml-desktop - configuration files for the X desktop at grml
Changes: 
 grml-desktop (0.2.3) unstable; urgency=low
 .
   * Use grml-exec-wrapper in menu of fluxbox and in idesktop.
     Add grml-scripts >=1.1.18 to depends. [Closes: issue431]
Checksums-Sha1: 
 2da2d1c9f6d9364eae12d8ee4a8e617fb3bcba0d 857 grml-desktop_0.2.3.dsc
 1731a79956ce93a476c6fbe8212bda93c88d8eb1 448150 grml-desktop_0.2.3.tar.gz
 640fb17c269a8419888e45d95524fd2898b87bf6 455678 grml-desktop_0.2.3_all.deb
Checksums-Sha256: 
 b056f59af0540cc714110c4813f49489bffcbe6db2d10713c71ae04ba1aef2ea 857 grml-desktop_0.2.3.dsc
 7d2b3899f91725799cb74c1525819810b0d4a205cc2f20f2b6fe4ceb975255d1 448150 grml-desktop_0.2.3.tar.gz
 737bbd06c318be0d6f2d6a94e535e3e66fa2b49d2510a06ea119b2ccac0b185a 455678 grml-desktop_0.2.3_all.deb
Files: 
 c44c9d9b12d9b5a4e82256e2b4b8c36c 857 grml extra grml-desktop_0.2.3.dsc
 7fc7eebbbc2b0e3b36421e874f9153e5 448150 grml extra grml-desktop_0.2.3.tar.gz
 30b43e214c72c051deb0bd0cd2b15106 455678 grml extra grml-desktop_0.2.3_all.deb

-----BEGIN PGP SIGNATURE-----
Version: GnuPG v1.4.9 (GNU/Linux)

iD8DBQFJ2bUl2N9T+zficugRAkTUAJ9+5ZArtKzcO5bwyf9JllMWDjgq+gCfQBSd
OKkK4yZ4qt7RBPfF+X2IEBg=
=wxyN
-----END PGP SIGNATURE-----
msg2105 (view) Author: mika Date: 2009-03-31.21:34:16
Ok, my plan is to merge grml-exec-wrapper into master of grml-scripts and 
adjust all related configs to use it from now on.

regards,
-mika-
msg1896 (view) Author: mika Date: 2009-02-15.16:13:47
* Frank Terbeck <bts@bts.grml.org> [20090215 17:03]:

> > Cool. thanks! Applied and pushed:

> Nice, however:

> +if is_installed "$PROG" 1>/dev/null 2>&1 ; then

> is_installed() should never produce any output, so the redirection can
> be dropped.

Thanks! Pushed.

regards,
-mika-
msg1895 (view) Author: ft Date: 2009-02-15.16:02:57
Michael Prokop <bts@bts.grml.org>:
> Cool. thanks! Applied and pushed:

Nice, however:

+if is_installed "$PROG" 1>/dev/null 2>&1 ; then

is_installed() should never produce any output, so the redirection can
be dropped.

Regards, Frank
msg1894 (view) Author: mika Date: 2009-02-15.15:52:19
* Frank Terbeck <bts@bts.grml.org> [20090215 14:11]:

> Let's face it, I'm an idiot. :-)
> Missed one improvement I mentioned earlier (the '&& break').

> [snip]
> is_installed() {
>     prog="$1"
>     [ -z "$prog" ] && return 1

>     ret=1
>     oifs="$IFS"
>     IFS=:
>     for dir in $PATH; do
>         [ -z "$dir" ] && continue
>         [ -x "$dir/$prog" ] && ret=0 && break
>     done

>     IFS="$oifs"
>     unset oifs
>     return "$ret"
> }
> [snap]

> [...]
> > So, I'm happy with this now.

> No, now I am. :)

Cool. thanks! Applied and pushed:

  http://git.grml.org/?p=grml-scripts.git;a=commit;h=7760a19        Add ist_installed(), thanks ft! | 2009-02-15 16:51:19 +0100

regards,
-mika-
msg1893 (view) Author: ft Date: 2009-02-15.13:11:43
Let's face it, I'm an idiot. :-)
Missed one improvement I mentioned earlier (the '&& break').

[snip]
is_installed() {
    prog="$1"
    [ -z "$prog" ] && return 1

    ret=1
    oifs="$IFS"
    IFS=:
    for dir in $PATH; do
        [ -z "$dir" ] && continue
        [ -x "$dir/$prog" ] && ret=0 && break
    done

    IFS="$oifs"
    unset oifs
    return "$ret"
}
[snap]

[...]
> So, I'm happy with this now.

No, now I am. :)

Regards, Frank
msg1892 (view) Author: ft Date: 2009-02-15.12:51:25
Well, let me make a fresh start... :-)
I think this will make a final usable version.

[snip]
is_installed() {
    prog="$1"
    [ -z "$prog" ] && return 1

    ret=1
    oifs="$IFS"
    IFS=:
    for dir in $PATH; do
        [ -z "$dir" ] && continue
        [ -x "$dir/$prog" ] && ret=0
    done

    IFS="$oifs"
    unset oifs
    return "$ret"
}
[snap]

Works in posh, dash, bash ksh93 and mksh and in zsh's sh mode(at
least, I did test those). Cannot work in zsh mode, because $PATH
won't split without sh_word_split set.

Then again, in zsh mode, this would be done a *lot* differently.
So, I'm happy with this now.

Sometimes, sleep really helps. :)

Regards, Frank
msg1889 (view) Author: ft Date: 2009-02-14.21:47:26
Frank Terbeck <ft@bewatermyfriend.org>:
> Frank Terbeck <ft@bewatermyfriend.org>:
> > is_installed() {
> >     prog="$1"
> > 
> >     ret=1
> >     oifs="$IFS"
> >     IFS=:
> >     for dir in $PATH; do
> >         [ -x "${dir:-.}/$1" ] && ret=0
> 
> Thinking more about it, this is probably better done like this:
> 
>     
>         [ -x "${dir:-/bin}/$1" ] && ret=0

And another improvement:
        [ -x "${dir:-/bin}/$1" ] && ret=0 && break

For obvious reasons. :-)

Regards, Frank
msg1888 (view) Author: ft Date: 2009-02-14.21:43:23
Frank Terbeck <ft@bewatermyfriend.org>:
> is_installed() {
>     prog="$1"
> 
>     ret=1
>     oifs="$IFS"
>     IFS=:
>     for dir in $PATH; do
>         [ -x "${dir:-.}/$1" ] && ret=0

Thinking more about it, this is probably better done like this:

    
        [ -x "${dir:-/bin}/$1" ] && ret=0

Why? Well, /bin is probably in everybody's $PATH and using '.' would
sneak . into $PATH (well, not really but for the test if $PATH is
something like PATH=/bin:/sbin::/usr/bin) which is something *nobody*
wants.

>     done
> 
>     IFS="$oifs"
>     return "$ret"
> }

Regards, Frank
msg1887 (view) Author: ft Date: 2009-02-14.21:32:22
Michael Prokop <bts@bts.grml.org>:
> * Frank Terbeck <bts@bts.grml.org> [20090214 21:59]:
> 
> > > if type -a "$PROG" 1>/dev/null 2>&1 ; then
> 
> > > => Is this really a bashism?
> 
> > 'type -a' is probably. I don't even know what it does.
> > So, bash it is.
> 
> Just using 'which $PROG" would fix this issue, right? :)

Well, I don't think 'which' is covered by SUSv3 either. I think
originally, that was a csh script. If you really want POSIX
conformance, you'd do a loop over the dirs in $PATH and check if the
program in question is there and executable. That could go into a
shell library and would actually be cleaner than the
which-and-forget-about-the-output approach.

Such a function would look like this:

[snip]
is_installed() {
    prog="$1"

    ret=1
    oifs="$IFS"
    IFS=:
    for dir in $PATH; do
        [ -x "${dir:-.}/$1" ] && ret=0
    done

    IFS="$oifs"
    return "$ret"
}
[snap]

'which' isn't a builtin in shells like posh, dash or even bash.
Those rely on /bin/which or similar. And of course, an additional
fork() screws performance. :-)

Regards, Frank
msg1884 (view) Author: mika Date: 2009-02-14.21:01:27
* Frank Terbeck <bts@bts.grml.org> [20090214 21:59]:

> > if type -a "$PROG" 1>/dev/null 2>&1 ; then

> > => Is this really a bashism?

> 'type -a' is probably. I don't even know what it does.
> So, bash it is.

Just using 'which $PROG" would fix this issue, right? :)

regards,
-mika-
msg1883 (view) Author: ft Date: 2009-02-14.20:59:27
Michael Prokop <bts@bts.grml.org>:
[...]
> if type -a "$PROG" 1>/dev/null 2>&1 ; then
> 
> => Is this really a bashism?

'type -a' is probably. I don't even know what it does.
So, bash it is.

Regards, Frank
msg1881 (view) Author: mika Date: 2009-02-14.20:49:04
* Frank Terbeck <bts@bts.grml.org> [20090214 20:29]:

> [snip]
>     #!/bin/bash

> Seems like a plain sh script to me.
> Bash can handle that of course. But it's not needed, AFAICS.

I was afraid due to:

possible bashism in usr_bin/grml-exec-wrapper line 11 (type):
if type -a Xdialog 1>/dev/null 2>&1 && test -n "$DISPLAY" ; then
possible bashism in usr_bin/grml-exec-wrapper line 31 (type):
if type -a "$PROG" 1>/dev/null 2>&1 ; then

=> Is this really a bashism?

>     # use Xdisplay only if present and running under X:
>     display_info() {
>     if type -a Xdialog 1>/dev/null 2>&1 && test -n "$DISPLAY" ; then
>         Xdialog --title "grml-exec-wrapper" --msgbox "$1" 0 0 0
>     else
>         print "$1">&2

> stderr? really?

Yeah, I guess it makes sense as display_info() is invoked in error
situations only.

>     # make sure to support 'grml-exec-wrapper sudo wireshark' as well:
>     case $PROG in
>         *sudo*) PROG="$2" ;;
>     esac

>     if type -a "$PROG" 1>/dev/null 2>&1 ; then
>         exec $@

> Rather:
> exec "$@"

Thanks.

>     else
>         RC=1
>         display_info "Sorry: ${PROG} not available.

>     Looks like the grml flavour you are using does not ship ${PROG}. :(

>     You can search for ${PROG} executing:

>     apt-get update && apt-cache search ${PROG}
>         "
>     fi

> The whole RC variable is unneeded.
> The only way, you end up down here is if you come down from the else
> path above (which is unneeded, for the very same reason.

> Just 'exit 1' should be good enough (and much clearer).

Ah right, good catch. :)

Thanks for reviewing!

regards,
-mika-
msg1877 (view) Author: ft Date: 2009-02-14.19:29:26
Michael Prokop <bts@bts.grml.org>:
> Michael Prokop <mika@grml.org> added the comment:

[snip]
    #!/bin/bash

Seems like a plain sh script to me.
Bash can handle that of course. But it's not needed, AFAICS.

    # use Xdisplay only if present and running under X:
    display_info() {
    if type -a Xdialog 1>/dev/null 2>&1 && test -n "$DISPLAY" ; then
        Xdialog --title "grml-exec-wrapper" --msgbox "$1" 0 0 0
    else
        print "$1">&2

stderr? really?

    fi
    }

    if [ -z "$1" ] ; then
        display_info "Usage: $0 <program> [<arguments>]"
        exit 1
    fi

    RC='0'
    PROG="$1"

    # make sure to support 'grml-exec-wrapper sudo wireshark' as well:
    case $PROG in
        *sudo*) PROG="$2" ;;
    esac

    if type -a "$PROG" 1>/dev/null 2>&1 ; then
        exec $@

Rather:
exec "$@"

    else
        RC=1
        display_info "Sorry: ${PROG} not available.

    Looks like the grml flavour you are using does not ship ${PROG}. :(

    You can search for ${PROG} executing:

    apt-get update && apt-cache search ${PROG}
        "
    fi

The whole RC variable is unneeded.
The only way, you end up down here is if you come down from the else
path above (which is unneeded, for the very same reason.

Just 'exit 1' should be good enough (and much clearer).

If you're wondering why: if you call exec, the shell is done. It's
gone and won't come back.

    exit $RC
[snap]

Regards, Frank
msg1876 (view) Author: mika Date: 2009-02-14.18:34:39
Hi,

to fix this outstanding issue I just wrote grml-exec-wrapper:

  http://git.grml.org/?p=grml-scripts.git;a=blob_plain;f=usr_bin/grml-exec-
wrapper;h=a826be1fdd9ece15151596e3121a0379bb216137

I plan to use it like 'grml-exec-wrapper x-terminal-emulator' in our windows 
manager configuration, idesk & CO. What's your opinion (as our shell master :)) 
about that, Frank?

thx && regards,
-mika-
msg1245 (view) Author: mika Date: 2008-03-06.15:56:36
Yeah, writing a nice wrapper which checks for availability of the programm in 
question and displays a useful error message if it's not present is on my todo 
list since ages... Well, maybe someone is motivated to write something like 
that...
msg1244 (view) Author: maddi Date: 2008-03-06.15:54:49
After invoking the window manager wm-ng there are
quite some icons on the desktop where clicking does nothing.
Those icons point to programs which are not available in the medium edition.
History
Date User Action Args
2009-04-06 08:00:49mikasetstatus: in-progress -> resolved
nosy: mika, maddi, ft
messages: + msg2133
2009-03-31 21:34:17mikasetnosy: mika, maddi, ft
messages: + msg2105
2009-02-24 09:30:43mikasetnosy: mika, maddi, ft
title: wm-ng on grml-medium 0.1 -> wm-ng on grml-medium 0.1 [grml-exec-wrapper]
2009-02-15 16:13:47mikasetnosy: mika, maddi, ft
messages: + msg1896
2009-02-15 16:02:57ftsetnosy: mika, maddi, ft
messages: + msg1895
2009-02-15 15:52:20mikasetnosy: mika, maddi, ft
messages: + msg1894
2009-02-15 13:11:44ftsetnosy: mika, maddi, ft
messages: + msg1893
2009-02-15 12:51:28ftsetnosy: mika, maddi, ft
messages: + msg1892
2009-02-14 21:47:27ftsetnosy: mika, maddi, ft
messages: + msg1889
2009-02-14 21:43:23ftsetnosy: mika, maddi, ft
messages: + msg1888
2009-02-14 21:32:23ftsetnosy: mika, maddi, ft
messages: + msg1887
2009-02-14 21:01:28mikasetnosy: mika, maddi, ft
messages: + msg1884
2009-02-14 20:59:27ftsetnosy: mika, maddi, ft
messages: + msg1883
2009-02-14 20:49:05mikasetnosy: mika, maddi, ft
messages: + msg1881
2009-02-14 19:29:28ftsetnosy: mika, maddi, ft
messages: + msg1877
2009-02-14 18:34:42mikasetstatus: chatting -> in-progress
nosy: + ft
topic: + release-stopper
messages: + msg1876
assignedto: mika
2008-03-06 15:56:37mikasetstatus: unread -> chatting
assignedto: mika -> (no value)
messages: + msg1245
2008-03-06 15:54:49maddicreate