Re: gnulib unnecessarily enforces git as requirement

[Eric Blake]

On 01/20/2011 09:50 AM, Benjamin Lindner wrote:
>>
>> where the new --no-git option is what makes it explicit that you intend
>> to use an existing directory as-is rather than update a submodule in
>> your local tree.
> 
> Ok, first shot, see attached patch.
> 
> With this,
> this works:    export GNULIB_SRCDIR=path/to/gnulib; ./bootstrap
> --skip-po --copy --no-git
> this likewise: ./bootstrap --gnulib-srcdir=path/to/gnulib --skip-po
> --copy --no-git
> this fails:       ./bootstrap --skip-po --copy --no-git
> 
> (where 'works' means octave is bootstrapping correctly for a unpacked
> .tgz source tree)

Looks like reasonable behavior to me.  Actual comments on the patch:

> diff --git a/bootstrap b/bootstrap

You attached the patch with MIME type application/octet-stream and
base64 encoding; it's much easier to do inline review with text/plain
and no encoding.

You omitted a ChangeLog entry.

This patch is small enough to avoid copyright issues, but you may want
to get assignment in place if you plan on continued contributions to gnulib.

>  
> +# --no-git requires --gnulib-srcdir
> +if $no_git && test -z $GNULIB_SRCDIR; then

Insufficiently quoted (GNULIB_SRCDIR= in the environment will give the
wrong status), and dangerous (there are still shells where
GNULIB_SRCDIR== will cause syntax errors, because 'test -z =' violates
POSIX semantics).  Rather, I'd do:

test -d "$GNULIB_SRCDIR"

> +  echo "Error: --no-git requires --gnulib-srcdir!" >&2

and maybe change this to "...requires valid --gnulib-srcdir directory"

No need for a ! in error messages; just omit any trailing punctuation
per GNU Coding Standards

> +  exit 1
> +fi
> +
>  if test -n "$checkout_only_file" && test ! -r "$checkout_only_file"; then
>    echo "$0: Bootstrapping from a non-checked-out distribution is risky." >&2
>    exit 1
> @@ -384,6 +398,10 @@
>      if test "$app" = libtool; then
>        app=libtoolize
>      fi
> +    # only test for git if not using --no-git
> +    if test "$app" = git && $no_git; then

You used $use_git, not $no_git; but that means you need to update the
logic.  How about:

if test "$app" = git; then
  $use_git || continue
fi

-- 
Eric Blake   address@hidden    +1-801-349-2682
Libvirt virtualization library http://libvirt.org

signature.asc
Description: OpenPGP digital signature