Re: Unexpected automake version dependency causes build to fail

[Michael Abbott <michael at araneidae dot co dot uk>]

On Wed, 17 Dec 2008, Yann E. MORIN wrote:
> http://ymorin.is-a-geek.org/download/crosstool-ng/01-fixes/1.3.1/000-check-automake-and-curl-or-wget.patch

I get the following:

Computing version string... 1.3.1
Checking for '/bin/bash'... /bin/bash
Checking for 'make'... /usr/bin/make
Checking for 'gcc'... /usr/bin/gcc
Checking for 'gawk'... /usr/bin/gawk
Checking for 'sed'... /bin/sed
Checking for 'bison'... /usr/bin/bison
Checking for 'flex'... /usr/bin/flex
Checking for 'makeinfo'... /usr/bin/makeinfo
Checking for 'automake'... wrong version string: expecting regexp '\(GNU automake\) [[:digit:]]+\.[[:digit:]]{2,}'
Bailing out...
make: *** [crosstool-ng-1.3.1] Error 1


Looking at ./configure, it looks like that's what we expect.  I see your 
test is for d.dd* -- quick and dirty test for at least 1.10!  Tricky to 
better, alas -- regexp matching won't do numerical comparisons...

Ok, I'll now go and assemble myself an automake 1.10 (which I seem to 
recall depends on a more up to date m4, and so it goes...).


Oh.  I've just noticed the following construct in has_or_abort():

	{ IFS="|"; ...; }

Clearly your intent is to confine the effect of the IFS assignment to the 
bracketed expression.  Unfortunately curly brackets don't do this, they 
only group: if you want to nest the context you need to use round 
brackets:

	( IFS='|'; ... )

Same story further down where you actually call has_or_abort.  Doesn't 
seem to be having any impact, but then I guess we rely on automatic shell 
splitting of arguments as little as possible!  Fortunately IFS isn't 
normally exported, so the impact is minimal.


I can't help it: looking at your ./configure has forced me to create the 
attached patch (sorry, it's a -p0 patch, I got lazy).  It mostly only 
really changes layout, but here's a list of what I've done:

1. Enforce 4 space indent and 80 column lines throughout (where possible).
2. Remove spurious \ continuations and break lines on pipes where 
   convenient.
3. Remove (apparently?) spurious `||true` where condition code ignored 
   anyway.
4. Simplify some return code handling: return $? is rather futile!
5. Quote *all* values, as much as possible.
6. Changed the script to honestly depend on bash (was tempted to use 
   #!/usr/bin/env bash, but you check /bin/bash as an essential tool 
   anyway).

I'm sorry, I haven't tested this thoroughly, and I've probably broken 
something subtle.

Of course reindenting makes the other changes harder to see -- maybe I 
should separate reindents as a separate patch in the future?


I suspect (what with IFS=$'\n' being set) that the contrib list processing 
is actually broken with more than one contribution, but I've not tried to 
address this.  I don't really see why you reckon it's safer to hack the 
list rather than using IFS=, particularly as this is done elsewhere.

One final thought.  The form 
	echo "$blah" | some-program and-its-args
can be replaced by
	some-program <<<"$blah" and-its-args
This avoids spawning a subshell (so if some-program is a built-in, such as 
read, we can set global variables) and avoids the risk of echo 
interpreting "$blah" as switch ... but we don't have the `echo -n` option, 
alas.  And of course it's an essential bashism.  I didn't make this 
change.

Don't know if this is helpful or not.  If not, sorry about the noise and 
distraction!

Attachment: configure.patch
Description: Reformatting patch for ./configure

--
For unsubscribe information see http://sourceware.org/lists.html#faq