Re: [PATCH] depcomp: recognize tabs as whitespace in the dashmstdout mode

[Peter Rosin]

Stefano Lattarini skrev 2012-02-03 15:03:
> 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.

Hmmm, you're right again.  But I definitely was bitten by something like
that when I worked on Libtool some years ago, and perhaps it was an echo
or something that stripped the quotes in some obscure maner?  Anyway,
that's obviously not applicable here.

>> So, I went with 's/.*['"$tab"' ]//' instead.
>>
> This is fine though (as it avoids potential error from unescaped '\'
> chars into double-quoted string).

Good, I already have the '"$tab"' thing ready to push.

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

.oO(It would surely have caught a patch that massively destroyed depcomp)

>> 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 for the reviews and your patience!

Patches commited on msvc, merged into master...

Wait, there's a (trivial) conflict.  But if I resolve that and go
on with the merge anyway we will end up with two different depcomps
with "scriptversion=2012-02-03.12; # UTC".  Not good.

There is no good place to base this.  It's another instance of trouble
caused by the `quote'-'fix' commit on master.  I want to sync the
depcomp version on msvc with that on master (I forgot that depcomp
on msvc and maint differed when I did v1.11-654-g41404a8
"scripts: cherry-pick recent changes from master") before I push
the above.

So, I'm suggesting the following change on msvc first, then the other
two that you ACKed above, then merge msvc into master and branch-1.11.

Ok?

Cheers,
Peter


>From 0ef94b4e737ddf9b2f2ea47c02fc11ac932f3d1e Mon Sep 17 00:00:00 2001
From: Peter Rosin <address@hidden>
Date: Fri, 3 Feb 2012 15:46:09 +0100
Subject: [PATCH] depcomp: cherry-pick recent changes from master

* lib/depcomp: Quote 'like this', not `like this'.
---
 lib/depcomp |   40 ++++++++++++++++++++--------------------
 1 files changed, 20 insertions(+), 20 deletions(-)

diff --git a/lib/depcomp b/lib/depcomp
index bd0ac08..ff4e08f 100755
--- a/lib/depcomp
+++ b/lib/depcomp
@@ -28,7 +28,7 @@ scriptversion=2011-12-04.11; # UTC
 
 case $1 in
   '')
-     echo "$0: No command.  Try \`$0 --help' for more information." 1>&2
+     echo "$0: No command.  Try '$0 --help' for more information." 1>&2
      exit 1;
      ;;
   -h | --h*)
@@ -40,8 +40,8 @@ as side-effects.
 
 Environment variables:
   depmode     Dependency tracking mode.
-  source      Source file read by `PROGRAMS ARGS'.
-  object      Object file output by `PROGRAMS ARGS'.
+  source      Source file read by 'PROGRAMS ARGS'.
+  object      Object file output by 'PROGRAMS ARGS'.
   DEPDIR      directory where to store dependencies.
   depfile     Dependency file to output.
   tmpdepfile  Temporary file to use when outputting dependencies.
@@ -156,7 +156,7 @@ gcc)
 ## The second -e expression handles DOS-style file names with drive letters.
   sed -e 's/^[^:]*: / /' \
       -e 's/^['$alpha']:\/[^:]*: / /' < "$tmpdepfile" >> "$depfile"
-## This next piece of magic avoids the `deleted header file' problem.
+## This next piece of magic avoids the "deleted header file" problem.
 ## The problem is that when a header file which appears in a .P file
 ## is deleted, the dependency causes make to die (because there is
 ## typically no way to rebuild the header).  We avoid this by adding
@@ -164,7 +164,7 @@ gcc)
 ## this for us directly.
   tr ' ' '
 ' < "$tmpdepfile" |
-## Some versions of gcc put a space before the `:'.  On the theory
+## 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
 ## to the object.  Take care to not repeat it in the output.
@@ -203,7 +203,7 @@ sgi)
     # clever and replace this with sed code, as IRIX sed won't handle
     # lines with more than a fixed number of characters (4096 in
     # 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
+    # the IRIX cc adds comments like '#:fec' to the end of the
     # dependency line.
     tr ' ' '
 ' < "$tmpdepfile" \
@@ -229,7 +229,7 @@ sgi)
 aix)
   # The C for AIX Compiler uses -M and outputs the dependencies
   # in a .u file.  In older versions, this file always lives in the
-  # current directory.  Also, the AIX compiler puts `$object:' at the
+  # current directory.  Also, the AIX compiler puts '$object:' at the
   # start of each line; $object doesn't have directory information.
   # Version 6 uses the directory in both cases.
   dir=`echo "$object" | sed -e 's|/[^/]*$|/|'`
@@ -259,9 +259,9 @@ aix)
     test -f "$tmpdepfile" && break
   done
   if test -f "$tmpdepfile"; then
-    # Each line is of the form `foo.o: dependent.h'.
+    # Each line is of the form 'foo.o: dependent.h'.
     # Do two passes, one to just change these to
-    # `$object: dependent.h' and one to simply `dependent.h:'.
+    # '$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 [].
     sed -e 's,^.*\.[a-z]*:[     ]*,,' -e 's,$,:,' < "$tmpdepfile" >> "$depfile"
@@ -275,7 +275,7 @@ aix)
   ;;
 
 icc)
-  # Intel's C compiler understands `-MD -MF file'.  However on
+  # Intel's C compiler understands '-MD -MF file'.  However on
   #    icc -MD -MF foo.d -c -o sub/foo.o sub/foo.c
   # ICC 7.0 will fill foo.d with something like
   #    foo.o: sub/foo.c
@@ -300,10 +300,10 @@ icc)
     exit $stat
   fi
   rm -f "$depfile"
-  # Each line is of the form `foo.o: dependent.h',
-  # or `foo.o: dep1.h dep2.h \', or ` dep3.h dep4.h \'.
+  # Each line is of the form 'foo.o: dependent.h',
+  # or 'foo.o: dep1.h dep2.h \', or ' dep3.h dep4.h \'.
   # Do two passes, one to just change these to
-  # `$object: dependent.h' and one to simply `dependent.h:'.
+  # '$object: dependent.h' and one to simply 'dependent.h:'.
   sed "s,^[^:]*:,$object :," < "$tmpdepfile" > "$depfile"
   # Some versions of the HPUX 10.20 sed can't process this invocation
   # correctly.  Breaking it into two sed invocations is a workaround.
@@ -344,7 +344,7 @@ hp2)
   done
   if test -f "$tmpdepfile"; then
     sed -e "s,^.*\.[a-z]*:,$object:," "$tmpdepfile" > "$depfile"
-    # Add `dependent.h:' lines.
+    # Add 'dependent.h:' lines.
     sed -ne '2,${
               s/^ *//
               s/ \\*$//
@@ -359,9 +359,9 @@ hp2)
 
 tru64)
    # The Tru64 compiler uses -MD to generate dependencies as a side
-   # effect.  `cc -MD -o foo.o ...' puts the dependencies into `foo.o.d'.
+   # effect.  'cc -MD -o foo.o ...' puts the dependencies into 'foo.o.d'.
    # At least on Alpha/Redhat 6.1, Compaq CCC V6.2-504 seems to put
-   # dependencies in `foo.d' instead, so we check for that too.
+   # dependencies in 'foo.d' instead, so we check for that too.
    # Subdirectories are respected.
    dir=`echo "$object" | sed -e 's|/[^/]*$|/|'`
    test "x$dir" = "x$object" && dir=
@@ -478,7 +478,7 @@ dashmstdout)
     shift
   fi
 
-  # Remove `-o $object'.
+  # Remove '-o $object'.
   IFS=" "
   for arg
   do
@@ -498,9 +498,9 @@ dashmstdout)
   done
 
   test -z "$dashmflag" && dashmflag=-M
-  # Require at least two characters before searching for `:'
+  # Require at least two characters before searching for ':'
   # in the target name.  This is to cope with DOS-style filenames:
-  # a dependency such as `c:/foo/bar' could be seen as target `c' otherwise.
+  # a dependency such as 'c:/foo/bar' could be seen as target 'c' otherwise.
   "$@" $dashmflag |
     sed 's:^[  ]*[^: ][^:][^:]*\:[    ]*:'"$object"'\: :' > "$tmpdepfile"
   rm -f "$depfile"
@@ -583,7 +583,7 @@ cpp)
     shift
   fi
 
-  # Remove `-o $object'.
+  # Remove '-o $object'.
   IFS=" "
   for arg
   do
-- 
1.7.5.1