upload.py breaks some git diffs

[GoogleCodeExporter]

What steps will reproduce the problem?

1. Create a file 'test' in a git repo containing:

Hello\n\rOops\n\rOh no\r\nOk\r\n

Those are \n newlines and \r carriage returns

2. Upload file with git cl upload

What is the expected output? What do you see instead?

I expect a file named test in the review with a correct diff but instead the 
upload script have broken the diff. I think this is due to python splitlines 
conflicting with how git formats its diffs. While git splits at only newlines 
producing something like:

+Hello\n+\rOops\n+\rOh no\r\n+Ok\r\n

The upload script (using pythons splitlines) splits at both \n and \r, which 
means that it modifies this to be:

+Hello\n+\nOops\n+\nOh no\n+Ok\n

What browser are you using?  What version? On what operating system?

Original issue reported on code.google.com by ti...@opera.com on 11 Sep 2013 at 1:10

[GoogleCodeExporter]

[deleted comment]

[GoogleCodeExporter]

This is on linux, not sure if the same thing will happen on mac and windows.

Original comment by ti...@opera.com on 12 Sep 2013 at 9:49

[GoogleCodeExporter]

I see it.  Should \n\r be in the diff?  So the download result should be 
exactly:
Hello\n\rOops\n\rOh no\r\nOk\r\n

Original comment by chrisp...@chromium.org on 12 Sep 2013 at 10:06

[GoogleCodeExporter]

See my comment on https://codereview.appspot.com/13668043/, upload.py is 
aggressively using universal_newlines=True when it shouldn't have.

Original comment by maruel@chromium.org on 12 Sep 2013 at 10:09

[GoogleCodeExporter]

Original comment by maruel@chromium.org on 12 Sep 2013 at 10:09

[GoogleCodeExporter]

Note that the universal_newlines=True is not the only place where newlines are 
mangled in the upload.py script. There is at least one place where splitlines 
(without the keep line endings flag) are used to split lines which are then 
rejoined with just a '\n'.

There might be more places on the way to the code review where this could fails 
but I haven't looked into that. Also, I'm not sure how svn diffs deals with 
line endings like this.

I'm not sure what the upload.py script should do with the line endings (it 
could normalize them, but give a warning - or it could keep them as is -- but 
this should be clearly defined) but it should at least send valid diffs.

Original comment by ti...@opera.com on 12 Sep 2013 at 11:43

[GoogleCodeExporter]

Looks like git normalize \n\r to \n.

Original comment by chrisp...@chromium.org on 18 Sep 2013 at 11:40

[GoogleCodeExporter]

That is not generally true, it depends on your platform, your settings and what 
the .gitattributes file contains. 
http://timclem.wordpress.com/2012/03/01/mind-the-end-of-your-line/ explains 
this quite well.

Original comment by ti...@opera.com on 19 Sep 2013 at 6:26

[GoogleCodeExporter]

Oh that is LFCR, which isn't really covered by that article. However -- I don't 
see why that should be converted to a single LF. If anything it should be one 
LF for the LF and one for the CR (old unix/mac style). On my machine that 
doesn't get normalized to a single LF as git is by default set up to not 
normalize line endings at all.

Original comment by ti...@opera.com on 19 Sep 2013 at 8:28

[GoogleCodeExporter]

Original comment by chrisp...@chromium.org on 19 Sep 2013 at 11:54