Re: [PATCH] depcomp: add new one-pass depmode for MSVC 7 and later.

[Ralf Wildenhues]

Hi Peter,

first off, apologies to Stefano and you for my glacier-like review
progress; I hope to get a bigger chunk of time this weekend, to be
able to look at some of the longer threads.

* Peter Rosin wrote on Wed, Oct 06, 2010 at 05:55:44PM CEST:
> I noticed that MSVC 7 and above sports the option -showIncludes

> cl -showIncludes foo.c
> Microsoft (R) 32-bit C/C++ Optimizing Compiler Version 14.00.50727.762 for 
> 80x86
> Copyright (C) Microsoft Corporation.  All rights reserved.
> 
> foo.c
> Note: including file: c:\cygwin\home\peda\junk\bar.h
[...]
> So, taking advantage of that I'm proposing this depcomp update.

Nice.

> The patch is based on the msvc branch.

> The old MSVC depmodes are still
> needed for MSVC 6 so can't be deleted without causing a regression.
> 
> The testsuite seems happy (but I only ran the depcomp*.test tests).

OK with minor nits below addressed.

Thanks,
Ralf

> Subject: [PATCH] depcomp: add new one-pass depmode for MSVC 7 and later.
> 
> * lib/depcomp: Add new depmodes 'msvc7' and 'msvc7msys' which
> make use of the -showIncludes option added in MSVC 7.
> * m4/depend.m4 (_AM_DEPENDENCIES): Handle the new depmodes
> similarly to 'msvisualcpp' and 'msvcmsys' as MSVC does not
> support the -o option.

> --- a/lib/depcomp
> +++ b/lib/depcomp

> +  echo "$object : \\" > "$depfile"
> +  sed -n '/^Note: including file:  *\(.*\)/ { s//\1/; s/\\/\\\\/g; p; }' < 
> "$tmpdepfile" \

You need a newline after {.
Between { and }, replace all ; with newlines.

> +  | $cygpath_u

> | sort -u | sed -n -e '
> +s/ /\\ /g
> +s/\(.*\)/    \1 \\/p
> +s/.\(.*\) \\/\1:/
> +H
> +$ {
> +  s/.*/      /
> +  G
> +  p
> +}' >> "$depfile"

This sed script is fairly sophisticated (and relies on sed having
large-enough pattern and hold spaces.  It might deserve a comment
or two (but note that comments inside sed scripts are not portable).
Due to the space requirement, I wouldn't want to reuse it for some
of the other (non-w32) vendor depcomp modes.

> +  rm -f "$tmpdepfile"
> +  ;;
[...]

in the other mail:

* Peter Rosin wrote on Thu, Oct 07, 2010 at 09:57:40AM CEST:
> Den 2010-10-06 17:55 skrev Peter Rosin:
> > @@ -90,10 +90,18 @@ if test "$depmode" = msvcmsys; then
> >     # This is just like msvisualcpp but w/o cygpath translation.
> >     # Just convert the backslash-escaped backslashes to single forward
> >     # slashes to satisfy depend.m4
> > -   cygpath_u="sed s,\\\\\\\\,/,g"
> > +   cygpath_u='sed s,\\\\,/,g'
> >     depmode=msvisualcpp
> >  fi
> >  
> > +if test "$depmode" = msvc7msys; then
> > +   # This is just like msvc7 but w/o cygpath translation.
> > +   # Just convert the backslashes to forward slashes to satisfy
> > +   # depend.m4
> > +   cygpath_u='sed s,\\\\,/,g'
> > +   depmode=msvc7
> > +fi
> 
> The above comment should be the same as the one in the previous block.
> I'm fixing locally, in wait for a review.

The comments should be the same, except the lower one should still state
"just like msvc7" rather than "just like msvisualcpp".  (I'm guessing
you're doing it just that way.)