[lxc-devel] [PATCH v3] use which instead of type

[Dwight Engen]

On Tue, 22 Jan 2013 15:06:44 -0600
Serge Hallyn <serge.hallyn at canonical.com> wrote:

> Quoting Dwight Engen (dwight.engen at oracle.com):
> > This is for consistency with the rest of lxc, and also because type
> > checks for shell builtins, a behavior that we do not want in these
> > cases. Ensure stderr for which is redirected to /dev/null also.
> > 
> > Signed-off-by: Dwight Engen <dwight.engen at oracle.com>
> 
> Thanks, Dwight :)
> 
> A few more cases below where set -e will cause silent deaths, but
> those aren't introduced by you so we can fix those in a separate
> patch.
> 
> Acked-by: Serge E. Hallyn <serge.hallyn at ubuntu.com>
> 
> > ---
> >  src/lxc/lxc-clone.in     | 2 +-
> >  src/lxc/lxc-create.in    | 2 +-
> >  src/lxc/lxc-shutdown.in  | 4 ++--
> >  templates/lxc-busybox.in | 4 ++--
> >  templates/lxc-ubuntu.in  | 8 ++------
> >  5 files changed, 8 insertions(+), 12 deletions(-)
> > 
> > diff --git a/src/lxc/lxc-clone.in b/src/lxc/lxc-clone.in
> > index 22f0376..76fc2f6 100644
> > --- a/src/lxc/lxc-clone.in
> > +++ b/src/lxc/lxc-clone.in
> > @@ -197,7 +197,7 @@ lxc-info -n $lxc_orig --state-is RUNNING ||
> > container_running=False 
> >  sed -i '/lxc.rootfs/d' $lxc_path/$lxc_new/config
> >  if [ -b $oldroot ]; then
> > -    which vgscan >/dev/null || { echo "$(basename $0): lvm is not
> > installed" >&2; false; }
> > +    which vgscan >/dev/null 2>&1 || { echo "$(basename $0): lvm is
> > not installed" >&2; false; } lvdisplay $oldroot > /dev/null 2>&1 ||
> > { echo "$(basename $0): non-lvm blockdev cloning is not supported"
> > >&2; false; } lvm=TRUE # ok, create a snapshot of the lvm device
> > diff --git a/src/lxc/lxc-create.in b/src/lxc/lxc-create.in
> > index 5cd9fdb..9348d87 100644
> > --- a/src/lxc/lxc-create.in
> > +++ b/src/lxc/lxc-create.in
> > @@ -222,7 +222,7 @@ if [ "$backingstore" = "_unset" ] ||
> > [ "$backingstore" = "btrfs" ]; then fi
> >  
> >  if [ "$backingstore" = "lvm" ]; then
> > -    which vgscan > /dev/null
> > +    which vgscan > /dev/null 2>&1
> 
> (this can be a follow-in patch if so) - does this also need to be
> '|| { echo ...} to accomodate the set -e?

My grep only showed lxc-clone, lxc-shutdown, lxc-ubuntu, and
lxc-ubuntu-cloud as being set -e, so I don't think these are
immediate problems unless they are sourced with . by something else
already set -e (no scripts of ours, that I can see) or we add set -e to
them (in which case there is more to handle than just the which
command!).

> >      if [ $? -ne 0 ]; then
> >          echo "$(basename $0): vgscan not found (is lvm2
> > installed?)" >&2 exit 1
> > diff --git a/src/lxc/lxc-shutdown.in b/src/lxc/lxc-shutdown.in
> > index a033b10..fdcade2 100644
> > --- a/src/lxc/lxc-shutdown.in
> > +++ b/src/lxc/lxc-shutdown.in
> > @@ -108,8 +108,8 @@ if [ "$(id -u)" != "0" ]; then
> >     exit 1
> >  fi
> >  
> > -which lxc-info > /dev/null || { echo "lxc-info not found."; exit
> > 1; } -which lxc-wait > /dev/null || { echo "lxc-wait not found.";
> > exit 1; } +which lxc-info > /dev/null 2>&1 || { echo "lxc-info not
> > found."; exit 1; } +which lxc-wait > /dev/null 2>&1 || { echo
> > "lxc-wait not found."; exit 1; } 
> >  pid=`lxc-info -n $lxc_name -p 2>/dev/null | awk '{ print $2 }'`
> >  if [ "$pid" = "-1" ]; then
> > diff --git a/templates/lxc-busybox.in b/templates/lxc-busybox.in
> > index 439657f..cb425ec 100644
> > --- a/templates/lxc-busybox.in
> > +++ b/templates/lxc-busybox.in
> > @@ -153,7 +153,7 @@ configure_busybox()
> >  {
> >      rootfs=$1
> >  
> > -    type busybox >/dev/null
> > +    which busybox >/dev/null 2>&1
> 
> ... And here
> 
> >  
> >      if [ $? -ne 0 ]; then
> >          echo "busybox executable is not accessible"
> > @@ -194,7 +194,7 @@ configure_busybox()
> >  
> >  
> >      # add ssh functionality if dropbear package available on host
> > -    type dropbear >/dev/null
> > +    which dropbear >/dev/null 2>&1
> 
> ...  And here
> 
> >      if [ $? -eq 0 ]; then
> >          # copy dropbear binary
> >          cp $(which dropbear) $rootfs/usr/sbin
> > diff --git a/templates/lxc-ubuntu.in b/templates/lxc-ubuntu.in
> > index 0ed8808..a2fc952 100644
> > --- a/templates/lxc-ubuntu.in
> > +++ b/templates/lxc-ubuntu.in
> > @@ -609,7 +609,7 @@ arch=$(arch)
> >  # Code taken from debootstrap
> >  if [ -x /usr/bin/dpkg ] && /usr/bin/dpkg --print-architecture
> > >/dev/null 2>&1; then arch=`/usr/bin/dpkg --print-architecture`
> > -elif type udpkg >/dev/null 2>&1 && udpkg --print-architecture
> > >/dev/null 2>&1; then +elif which udpkg >/dev/null 2>&1 && udpkg
> > >--print-architecture >/dev/null 2>&1; then
> >      arch=`/usr/bin/udpkg --print-architecture`
> >  else
> >      arch=$(arch)
> > @@ -666,11 +666,7 @@ if [ $hostarch = "i386" -a $arch = "amd64" ];
> > then exit 1
> >  fi
> >  
> > -type debootstrap
> > -if [ $? -ne 0 ]; then
> > -    echo "'debootstrap' command is missing"
> > -    exit 1
> > -fi
> > +which debootstrap >/dev/null 2>&1 || { echo "'debootstrap' command
> > is missing" >&2; false; } 
> >  if [ -z "$path" ]; then
> >      echo "'path' parameter is required"
> > -- 
> > 1.7.12.3
> > 
> >