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 [email protected] +1-801-349-2682 Libvirt virtualization library http://libvirt.org
signature.asc
Description: OpenPGP digital signature
