On 02/03/2012 02:24 PM, Peter Rosin wrote: > Stefano Lattarini skrev 2012-02-03 13:32: >> On 02/03/2012 10:07 AM, Peter Rosin wrote: >> >>> --- a/lib/depcomp >>> +++ b/lib/depcomp >>> @@ -1,7 +1,7 @@ >>> #! /bin/sh >>> # depcomp - compile a program generating dependencies as side-effects >>> >>> -scriptversion=2012-02-03.07; # UTC >>> +scriptversion=2012-02-03.08; # UTC >>> >>> # Copyright (C) 1999, 2000, 2003, 2004, 2005, 2006, 2007, 2009, 2010, >>> # 2011, 2012 Free Software Foundation, Inc. >>> @@ -57,6 +57,12 @@ EOF >>> ;; >>> esac >>> >>> +# A tabulation character. >>> +tab=' ' >>> +# A newline character. >>> +nl=' >>> +' >>> + >>> if test -z "$depmode" || test -z "$source" || test -z "$object"; then >>> echo "depcomp: Variables source, object and depmode must be set" 1>&2 >>> exit 1 >>> @@ -162,8 +168,7 @@ gcc) >>> ## typically no way to rebuild the header). We avoid this by adding >>> ## dummy dependencies for each header file. Too bad gcc doesn't do >>> ## this for us directly. >>> - tr ' ' ' >>> -' < "$tmpdepfile" | >>> + tr ' ' $nl < "$tmpdepfile" | >>> >> Missing quote around "$nl". The result will be an error from 'tr', >> and an empty output passed down the pipe (d'oh!). Several similar >> instances in the rest of the patch. > > *snip* further evidence of <something> > > Man, what a brain-fart, how embarrassing! > > Thanks for the review, it's highly appropriate this time. > > In my defense, the testsuite didn't catch it either (no new FAILs anyway). > But in the closing argument, the prosecution brings up that I didn't even > check a single outcome of the autoconf test "checking dependency style > of..." Big sigh! > > Regarding changing the 's/.*['$tab' ]//' pattern into "s/.*[$tab ]//" > I'm a bit reluctant to have * and ? characters inside " quotes as > that, on occasion, have resulted in bogus filename expansions. And > those bugs are not fun to chase. > Wait, what? Are you sure? I'd consider any shell doing that so severely broken that it is not worth supporting.
> So, I went with 's/.*['"$tab"' ]//' instead. > This is fine though (as it avoids potential error from unescaped '\' chars into double-quoted string). > Maybe depmod.tap should be replaced/rewritten with "compilers" that > simulate the different depmods? I could tinker with that a bit... > Yeah, I had thought about the possibility of such an approach, but was reluctant to suggest it, since it would entail non-trivial work that can only offer brittle and unsure results anyway... Maybe we should simply make it clear that 'decomp' only offers "best-effort" support for non-mainstream compilers (i.e., different from GCC >= 4.x and from recent Sun Studio or MSVC compilers); if problems show up, the user should just use "./configure --disable-dependency-tracking" (and maybe send us a patch if he's kind and motivated enough). > Anyway, let's take this one more round since it is obvious that I can't > be 100% trusted today and on top of that have been reading through this > enough to not see any bugs in it even if clearly marked as such... > OK. I'm already preparing a patch to simplify 'depmod.tap' and decrease the possibility of spurious FAIL or PASS results (will post it this evening or tomorrow). > So, ok to push "depcomp: recognize tabs as whitespace in the dashmstdout mode" > and the below patch to the msvc branch? > ACK. And ACK for a merge of msvc into master as well. Thanks, Stefano
