Patch failed to apply due to line endings mismatch - help :(

[Oren Blasberg]

Hi all,

I'm trying to update a Polymer component with changes generated by the third_party/polymer/reproduce.sh script. My CL is here: https://codereview.chromium.org/913353002/

However, all attempts to submit fail because the git patch is failing to apply. Doing git diff reveals that the generated changes are apparently adding ^M to the end of certain lines in paper-checkbox.html. However, when I open this file in Vim and :set list, I do not see those ^M characters. They don't appear and don't turn up in a find either.

If I try to apply the git patch locally on a detached head from origin/master, it fails to apply as well, just like it does on the Chromium CQ.

It seems like I need to convert the file to unix file endings, but doing so (by :set ff=unix) appears to rewrite the entire file rather than just the modified lines; 'git diff' now shows the entire file as modified.

Can anyone help me figure out how to solve this issue?

Thanks!

Oren

[Michael Moss]

While ugly, converting to ff=unix might be the best option, since http://www.chromium.org/developers/coding-style does say files should be LF-only, and there's no good reason for an HTML file (unlike maybe a .bat file) to not follow that guideline.

 

Can anyone help me figure out how to solve this issue?

Thanks!

Oren

--
--
Chromium Developers mailing list: chromi...@chromium.org
View archives, change email options, or unsubscribe:
http://groups.google.com/a/chromium.org/group/chromium-dev

[Scott Graham]

third_party/polymer/components/paper-checkbox/paper-checkbox.html is CRLF, maybe change that and regenerate.

(Hopefully the tools aren't doing the \r injection?)

[Oren Blasberg]

Running dos2unix on the css and html files (the ones that were apparently the issue), then making the commit, still resulted in a patch failure. It's happening on the very first line now. From the failed trybot log:

  error: patch failed: third_party/polymer/components-chromium/paper-checkbox/paper-checkbox.css:1
  error: third_party/polymer/components-chromium/paper-checkbox/paper-checkbox.css: patch does not apply

Also, the patch still fails locally with `git apply':

error: patch failed: third_party/polymer/components-chromium/paper-checkbox/paper-checkbox.css:1

error: third_party/polymer/components-chromium/paper-checkbox/paper-checkbox.css: patch does not apply

error: patch failed: third_party/polymer/components/paper-checkbox/paper-checkbox.css:1

error: third_party/polymer/components/paper-checkbox/paper-checkbox.css: patch does not apply

error: patch failed: third_party/polymer/components/paper-checkbox/paper-checkbox.html:1

error: third_party/polymer/components/paper-checkbox/paper-checkbox.html: patch does not apply

:( Anyone else have any thoughts here? This isn't making much sense to me, and that error message is decidedly unhelpful.

Thanks,

Oren

[Scott Graham]

The CQ cannot handle CR's in the patch (in either the before or after patch). The CRLF->LF change has to be directly committed with `git cl land`.

[Michael Moss]

I don't know if upload.py manipulates the patch, but it doesn't look like that CL has the line-ending changes, otherwise I think

https://codereview.chromium.org/913353002/patch/60001/70007

would should the whole file changed, like you described before. Does 'git diff' still show that it all changed?

[Michael Giuffrida]

To clarify, the local "git diff" shows no line-ending changes. CRs are preserved as CRs. So the patch on my file system looks like:

   height: 18px;^M
+  somethingNew: 10px;
   width:18px;^M

Upon uploading, the patch file loses all CRs. So Rietveld thinks it's diffing between two blobs, neither of which have CRs:

   height: 18px;
+  somethingNew: 10px;
   width:18px;

So the patch in Rietveld "does not apply" because the context is wrong.

It sounds like "git cl land" will get around this by preserving the CRs in the patch? In that case, we could "git cl land" a patch which strips out all CRs from these files, so the diff would be:

-  height: 18px;^M
+  height: 18px;

-  width:18px;^M
+  width:18px;

Then we'd be able to use the CQ  for the actual content changes. Am I on the right track?

[Scott Hess]

If the file currently has a mixture, and you change anything w/in the
unified diff region, you're doomed.

The general rule is that if you have a file with incorrect
line-endings or encoding (utf8 versus latin1, for instance) is that
you should first manually land a change to fix things, separate from
the change you originally set out to do. You can maybe TBR the first
change, but I don't think there's any downside to attempting to get
someone to review things, even if just "Is there anything I would
worry about if I'm changing line-endings on <this> file?"

DO NOT land changes which mix line-endings between lines! Like adding
a line with Unix line-ending to a file with DOS line ending! If you
ever find a file which is mixed that way, fix it immediately before it
gets worse!

-scott