Re: [PATCH, v2] Fix bootstrap with in-tree ISL

[DJ Delorie <dj at redhat dot com>]

A few minor nits...

Your patch includes many whitespace changes, which makes reviewing
your patch more difficult.  Please limit whitespace changes when
they're unrelated to the patch.

Assuming you've looked at the actual diffs for them, and see nothing
beyond changes related to your patch, it's unneecessary to include the
full patch for regenerated files like Makefile.in and configure.  (if
there *are* unrelated changes, you've done something wrong - likely
used the wrong version of the tools).  You'll still need to check in
these changes, of course, but we don't need to review them separately.

As for the patch itself...

Since none of the projects I see have autogen.sh anyway, I don't see
any impact on them from this patch, although that might change in the
future and I worry that developer's source trees will become cluttered
with "fixed" versions of files.  The GNU projects specify what
versions of auto* tools should be used.  It is not appropriate for a
developer to autogen with the OS's default tools and assume it's OK to
check those files in.  But as I said, this doesn't yet affect any of
the core projects (gcc, binutils+gdb, newlib).

I do worry about autogen.sh failing:

>  	module_srcdir=[+? module_srcdir (get "module_srcdir") (get "module")+]; \
> +	[ ! -f $$s/$$module_srcdir/configure ] && \
> +		[ -f $$s/$$module_srcdir/autogen.sh ] && \
> +			(cd $$s/$$module_srcdir/ && $(SHELL) ./autogen.sh); \
>  	[+ IF no-config-site +]rm -f no-such-file || : ; \

There is no logic in here to abort the build process if, for some
reason, autogen.sh returns a nonzero exit code.  This is likely to
happen if, for example, the source tree is read-only, but could also
happen if there's a mismatch between autotools and the config files.
Note that properly signalling the error up through the subshell is
tricky, so don't assume you've got it right until you prove it through
testing ;-)