Re: Unexpected automake version dependency causes build to fail

["Yann E. MORIN" <yann dot morin dot 1998 at anciens dot enib dot fr>]

Michael,
All,

On Wednesday 17 December 2008 11:16:59 Michael Abbott wrote:
> 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:
[--SNIP--]
> 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...

Yes! Quick'n'dirty... But that's won't work once 2.x is out...

> 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='|'; ... )

Yes, you're right. I had the exact same problem in another script later,
and didn't have the oportunity to change ./configure. Will do.

> 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).
Nice.

> 2. Remove spurious \ continuations and break lines on pipes where 
>    convenient.
I find it easier to read something like:
 foo=$(bar                  \
       |buz --with-options  \
       |boo
      )
because you have the closing parenthesis in line with the opening one, plus
starting the line with the pipe | makes it instantly obvious that the line
is a continuation. Also, aligning the trailing \ makes is easier to read
for me. But that are personal feelings... :-)

> 3. Remove (apparently?) spurious `||true` where condition code ignored 
>    anyway.
It once was not the case: I used to have:
    set _do_error ERR
    set -E
in ./configure, so in that case the "||true" was legit. It no longer is.

> 4. Simplify some return code handling: return $? is rather futile!
Yes. Coding at night can some time lead to unexpected code constructs... :-)

> 5. Quote *all* values, as much as possible.
OK.

> 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).
./configure must be able to _run_, even if bash is not present. The only
shell we know for sure is present is /bin/sh. Whether that be bash or
anything else is irrelevant to ./configure. The purpose of ./configure is
to check presence of required tools, of which bash is only one.

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

I'll look at this tonight when I'm home...

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

Yes, please. First patches with code changes, last patch with indent.

> 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.

But once IFS is correctly "jailed", that should no longer be a problem.

> 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.

As I said earlier, ./configure should *not* be a bash script. It shall be
entirely POSIX because it does not know on what it is going to run.

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

Yes it's helpful! Thank you!

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +0/33 662376056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| --==< ^_^ >==-- `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
`------------------------------^-------^------------------^--------------------'


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