Libguestfs - May 2013 - [PATCH] run: Don't fail on missing LIBGUESTFS_PATH if --disable-appliance

Set LIBGUESTFS_PATH to the default value compiled into and output a
warning to STDERR, instead.

The previous behavior caused the build to abort when trying to build
the sysprep documentation -- without much of a hint of what went
wrong.

When LIBGUESTFS_PATH was not set, test-events.sh would fail.
---
 run.in |    8 +++++---
 1 file changed, 5 insertions(+), 3 deletions(-)

diff --git a/run.in b/run.in
index 7545f0b..5adaa3a 100755
--- a/run.in
+++ b/run.in
@@ -66,9 +66,11 @@ chcon --reference=/tmp tmp 2>/dev/null ||:
 if [ "x at ENABLE_APPLIANCE@" = "xyes" ]; then
     export LIBGUESTFS_PATH="$b/appliance"
 elif [ -z "$LIBGUESTFS_PATH" ]; then
-    echo "run: error: You used './configure --disable-appliance'
so you must put an"
-    echo "run: error: appliance somewhere and set LIBGUESTFS_PATH to point
to it."
-    exit 1
+    cat <<'EOF' >&2
+run: warning: LIBGUESTFS_PATH is not set. Setting it to @libdir@/guestfs
+EOF
+    LIBGUESTFS_PATH=@libdir@/guestfs
+    export LIBGUESTFS_PATH
 fi
 
 if [ -z "$LD_LIBRARY_PATH" ]; then
-- 
1.7.10.4

On Sun, May 19, 2013 at 03:53:37PM +0200, Hilko Bengen
wrote:
> Set LIBGUESTFS_PATH to the default value compiled into and output a
> warning to STDERR, instead.
> 
> The previous behavior caused the build to abort when trying to build
> the sysprep documentation -- without much of a hint of what went
> wrong.
> 
> When LIBGUESTFS_PATH was not set, test-events.sh would fail.
> ---
>  run.in |    8 +++++---
>  1 file changed, 5 insertions(+), 3 deletions(-)
> 
> diff --git a/run.in b/run.in
> index 7545f0b..5adaa3a 100755
> --- a/run.in
> +++ b/run.in
> @@ -66,9 +66,11 @@ chcon --reference=/tmp tmp 2>/dev/null ||:
>  if [ "x at ENABLE_APPLIANCE@" = "xyes" ]; then
>      export LIBGUESTFS_PATH="$b/appliance"
>  elif [ -z "$LIBGUESTFS_PATH" ]; then
> -    echo "run: error: You used './configure
--disable-appliance' so you must put an"
> -    echo "run: error: appliance somewhere and set LIBGUESTFS_PATH to
point to it."
> -    exit 1
> +    cat <<'EOF' >&2
> +run: warning: LIBGUESTFS_PATH is not set. Setting it to @libdir@/guestfs
> +EOF
> +    LIBGUESTFS_PATH=@libdir@/guestfs
> +    export LIBGUESTFS_PATH
>  fi
NACK to this version (still ACK to the first version however).

I think this change defeats the point of the original error which was
to force the user to set the path to something.  Also how do we know
that @libdir@/guestfs will be an appliance?  It seems unlikely to
contain anything useful, or it could even contain a different
appliance from an installed copy of libguestfs which would be actively
wrong.

In order to build the virt-sysprep documentation we have to run
virt-sysprep (to dump out the table of operations supported by the
binary).  Specifically it runs:

  ../run ./virt-sysprep --dump-pod[-options]

which are internal flags to get the binary to print out the
documentation.  An appliance should not be needed for this of course.

As you say, this fails at the moment unnecessarily.

Your first version of this patch fixed this just fine (apart from
printing a harmless warning) and works for me.  So I don't understand
the need to have the ./run script setting LIBGUESTFS_PATH as in this
second version.

Rich.

-- 
Richard Jones, Virtualization Group, Red Hat http://people.redhat.com/~rjones
virt-top is 'top' for virtual machines.  Tiny program with many
powerful monitoring features, net stats, disk stats, logging, etc.
http://people.redhat.com/~rjones/virt-top