On 02/03/2012 10:07 AM, Peter Rosin wrote: > Stefano Lattarini skrev 2012-02-03 09:35: >> Hi Peter. >> >> On 02/03/2012 08:58 AM, Peter Rosin wrote: >>> Commit Release-1-7-2b-2-gf03ceab "Cope with DOS filenames in >>> dependencies." inadvertently converted tabs into spaces. >>> >>> * lib/depcomp (dashmstdout): Add a tab character to all sets >>> matching whitespace. >>> --- >>> lib/depcomp | 6 +++--- >>> 1 files changed, 3 insertions(+), 3 deletions(-) >>> >>> Ok for msvc? >>> >> Almost... to avoid similar regressions in the future, I think we >> could introduce a new '$tab' variable and use that instead... and >> then, for extra safety, we might even add a sanity check like: >> >> case "$tab" in *\ *) fatal "\$tab is not a TAB";; esac >> >> early in the script, to ensure $tab is not messed up by editors or >> plain old carelessness. >> >> WDYT? > > I'd rather do that in a follow-up, like below, so that the real change > in the above patch isn't hidden in the churn. > Sounds sensible; agreed.
> Ok? > Not exactly, there are several problems with the patch. See below. > No sanity checks included, but that seems way overkill to me... > OK, no problem. > --- 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. > ## Some versions of gcc put a space before the `:'. On the theory > ## that the space means something, we add a space to the output as > ## well. hp depmode also adds that space, but also prefixes the VPATH > @@ -205,16 +210,13 @@ sgi) > # IRIX 6.2 sed, 8192 in IRIX 6.5). We also remove comment lines; > # the IRIX cc adds comments like `#:fec' to the end of the > # dependency line. > - tr ' ' ' > -' < "$tmpdepfile" \ > + tr ' ' $nl < "$tmpdepfile" \ > | sed -e 's/^.*\.o://' -e 's/#.*$//' -e '/^$/ d' | \ > - tr ' > -' ' ' >> "$depfile" > + tr $nl ' ' >> "$depfile" > echo >> "$depfile" > > # The second pass generates a dummy entry for each header file. > - tr ' ' ' > -' < "$tmpdepfile" \ > + tr ' ' $nl < "$tmpdepfile" \ > | sed -e 's/^.*\.o://' -e 's/#.*$//' -e '/^$/ d' -e 's/$/:/' \ > >> "$depfile" > else > @@ -264,7 +266,7 @@ aix) > # `$object: dependent.h' and one to simply `dependent.h:'. > sed -e "s,^.*\.[a-z]*:,$object:," < "$tmpdepfile" > "$depfile" > # That's a tab and a space in the []. > This comment is now redundant, and IMO could be safely deleted. > - sed -e 's,^.*\.[a-z]*:[ ]*,,' -e 's,$,:,' < "$tmpdepfile" >> "$depfile" > + sed -e 's,^.*\.[a-z]*:['$tab' ]*,,' -e 's,$,:,' < "$tmpdepfile" >> > "$depfile" > This will break the first argument to '-e' in half, and cause an error in sed like "sed: -e expression #1, char 15: unterminated `s' command". I'd suggest to do something like this instead: sed -e "s,^.*\\.[a-z]*:[$tab ]*,," -e 's,$,:,' < "$tmpdepfile" >> "$depfile" Several similar instances in the rest of the patch. > else > # The sourcefile does not contain any dependencies, so just > # store a dummy comment line, to avoid errors with the Makefile > @@ -408,7 +410,7 @@ tru64) > if test -f "$tmpdepfile"; then > sed -e "s,^.*\.[a-z]*:,$object:," < "$tmpdepfile" > "$depfile" > # That's a tab and a space in the []. > - sed -e 's,^.*\.[a-z]*:[ ]*,,' -e 's,$,:,' < "$tmpdepfile" >> > "$depfile" > + sed -e 's,^.*\.[a-z]*:['$tab' ]*,,' -e 's,$,:,' < "$tmpdepfile" >> > "$depfile" > else > echo "#dummy" > "$depfile" > fi > @@ -443,11 +445,11 @@ msvc7) > p > }' | $cygpath_u | sort -u | sed -n ' > s/ /\\ /g > -s/\(.*\)/ \1 \\/p > +s/\(.*\)/'$tab'\1 \\/p > This too will cause unwanted breakup of the argument to 'sed -n'; you want something like this instead: s/\(.*\)/'"$tab"'\1 \\/p More similar instances in the rest of the patch. Regards, Stefano
