[PATCH] Fix dependency file escaping

[Sean Silva]

On Wed, May 13, 2015 at 2:23 PM, Robinson, Paul <
Paul_Robinson at playstation.sony.com> wrote:

>  r237296.  I believe I've answered all the questions, there have been no
> new comments for two days, and Sean already gave approval.
>

One person's approval does not override another person's concerns. Please
back out the `#` part until Nico's concerns have been fully addressed.


>  Thanks!
>
> --paulr
>
>
>
> *From:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Robinson, Paul
> *Sent:* Monday, May 11, 2015 11:17 AM
> *To:* Nico Weber
>
> *Cc:* reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* RE: [PATCH] Fix dependency file escaping
>
>
>
> One would naively think that GNU Make ought to be the program that can
> successfully parse gcc-produced depfiles, but it demonstrably does not (in
> the obviously obscure corner-case of a filename that has a '#' immediately
> after a '\' which, one must admit, is not inconceivable on Windows).
>
> So, shall we imitate gcc behavior that even the gcc people think doesn't
> look right (gcc.gnu.org/bugzilla/show_bug.cgi?id=65852) and can't be
> handled correctly even by GNU Make?  Or shall we implement behavior that
> GNU Make can parse correctly, even though it's not exactly what gcc
> produces?
>
>
>
> I am more inclined to produce a depfile that can be handled correctly by
> current tools than to produce one that cannot because of what even the gcc
> people admit looks wrong.  The definition of what a Makefile looks like
> more properly belongs to GNU Make than to GCC.
>
> --paulr
>
>
>
> *From:* thakis at google.com [mailto:thakis at google.com <thakis at google.com>] *On
> Behalf Of *Nico Weber
> *Sent:* Monday, May 11, 2015 10:21 AM
> *To:* Robinson, Paul
> *Cc:* Sean Silva; reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] Fix dependency file escaping
>
>
>
> I don't think ninja's current behavior is interesting. My point is more
> that it should be possible to write one program that can parse .d files
> produced by both gcc and clang. If gcc and clang agree on some "escaping",
> that's possible, else it isn't. If this patch makes clang match gcc's
> output for '\'s in path names that's cool. I'm skeptical if it's worth
> diverging on '#' characters.
>
>
>
> On Mon, May 11, 2015 at 8:19 AM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
>
> Okay, I worked out how to persuade ninja to read a depfile.  And in fact
> ninja 1.3.4 on Ubuntu 14.04 will treat
>
>     tspace.o: tspace.c bar\\#foo.h
>
> and
>
>     tspace.o: tspace.c bar\\\#foo.h
>
> as both expressing a dependency on the file named "bar\#foo.h" so the
> clang patch works for ninja.
>
> --paulr
>
>
>
> *From:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Robinson, Paul
> *Sent:* Monday, May 11, 2015 7:58 AM
> *To:* Sean Silva
> *Cc:* reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* RE: [PATCH] Fix dependency file escaping
>
>
>
> Is this right? GCC and the current patch coincide on the handling of `\ `.
> The only deviation is in the `#` case, no?
>
>
>
> Sorry, you are correct, GCC was getting "\ " right but "\#" wrong.  Clang
> was doing both wrong.
>
> So, for Nico's example, "\ " would be converted to "\\\ " by gcc, but not
> by Clang, and that's being fixed by the patch.
>
> But GCC would emit "\#" as "\\#" which GNU make would interpret as "\"
> followed by a comment, and if that failed, try to interpret it as "\\"
> followed by a comment, neither of which would look for the correct file.
> The patch causes Clang to emit this as "\\\#" which lets GNU make find the
> right file.
>
>
>
> Regarding ninja, I don't think ninja can directly read Make-style
> dependency files?  Its syntax naively looks different enough that I'm not
> really sure that's relevant here.
>
> --paulr
>
>
>
> *From:* Sean Silva [mailto:chisophugis at gmail.com <chisophugis at gmail.com>]
> *Sent:* Friday, May 08, 2015 6:40 PM
> *To:* Robinson, Paul
> *Cc:* Nico Weber; reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org;
> cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] Fix dependency file escaping
>
>
>
>
>
>
>
> On Fri, May 8, 2015 at 5:57 PM, Robinson, Paul <
> Paul_Robinson at playstation.sony.com> wrote:
>
> If a program (say, ninja) tries to be compatible with gnu make's depfile
> parsing, it would previously convert "\ " to a space from what I
> understand. Now it's going to get "\\\ " and think that that's "\ ".
>
>
>
> You're mixing things up.  #include "\ " would be converted by gcc to "\\ "
> (because it escapes the space but not the backslash) which would be
> de-escaped by GNU Make as "\" followed by a space delimiter.
>
> Now Clang will give it "\\\ " which will be handled as "\ " which is
> correct.
>
>
>
> Is this right? GCC and the current patch coincide on the handling of `\ `.
> The only deviation is in the `#` case, no?
>
>
>
> -- Sean Silva
>
>
>
>  (Remember that the string you hand to #include is NOT a normal C string;
> it has no escaping in it.)
>
> --paulr
>
>
>
> *From:* cfe-commits-bounces at cs.uiuc.edu [mailto:
> cfe-commits-bounces at cs.uiuc.edu] *On Behalf Of *Nico Weber
> *Sent:* Friday, May 08, 2015 4:36 PM
> *To:* reviews+D9208+public+0ac91376ffbd3e34 at reviews.llvm.org
> *Cc:* cfe-commits at cs.uiuc.edu
> *Subject:* Re: [PATCH] Fix dependency file escaping
>
>
>
> On Fri, May 8, 2015 at 4:01 PM, Paul Robinson <
> Paul_Robinson at playstation.sony.com> wrote:
>
> In http://reviews.llvm.org/D9208#169446, @thakis wrote:
>
> > Does gcc intend to fix this soon? Isn't being compatible with gcc
> important
> >  than the other things?
>
>
> If gcc emitted an incorrect relocation, would you argue that it's
> important to be compatible with gcc?  Even if you could not point to any
> linker that handled that buggy relocation in a reasonable way?
>
>
>
> 'course not, but that's not the case here. If a program (say, ninja) tries
> to be compatible with gnu make's depfile parsing, it would previously
> convert "\ " to a space from what I understand. Now it's going to get "\\\
> " and think that that's "\ ". So this is breaking backwards compat of clang
> with itself.
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
>
> _______________________________________________
> cfe-commits mailing list
> cfe-commits at cs.uiuc.edu
> http://lists.cs.uiuc.edu/mailman/listinfo/cfe-commits
>
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.llvm.org/pipermail/cfe-commits/attachments/20150513/af72cd52/attachment.html>