Issue 5644 - Add --binary to git diff options

Add --binary to git diff options

repository: https://hg.adblockplus.org/rietveld/
base revision: 5d4e635b14e3

[Vasily Kuznetsov, 2017-09-08 16:49:01]

This is a change to `upload.py` script that makes it work more consistently with
Git and Mercurial in terms of binary files handling. It's needed to be able to
apply the patches from reviews that contain binary files and were uploaded from
a Git repository.

[mathias, 2017-09-08 19:01:42]

I wonder why the `git show` commands (e.g. within `GetFileContent()` and
`GetBaseFile()`) would not need a `--binary` flag as well?

[Vasily Kuznetsov, 2017-09-08 20:24:50]

On 2017/09/08 19:01:42, mathias wrote:
> I wonder why the `git show` commands (e.g. within `GetFileContent()` and
> `GetBaseFile()`) would not need a `--binary` flag as well?

Those parts seemed to be working fine -- at least the new version of the file
gets correctly uploaded and can be seen in Rietveld UI. It's only the patch that
was semi-broken.

I also tried `git show $filehash` and it shows the binary content of a file, so
that probably explains it.

[mathias, 2017-09-08 20:48:32]

On 2017/09/08 20:24:50, Vasily Kuznetsov wrote:
> On 2017/09/08 19:01:42, mathias wrote:
> > I wonder why the `git show` commands (e.g. within `GetFileContent()` and
> > `GetBaseFile()`) would not need a `--binary` flag as well?
> 
> Those parts seemed to be working fine -- at least the new version of the file
> gets correctly uploaded and can be seen in Rietveld UI. It's only the patch
that
> was semi-broken.
> 
> I also tried `git show $filehash` and it shows the binary content of a file,
so
> that probably explains it.

Have you tried a binary that changes between patch-sets?

[Vasily Kuznetsov, 2017-09-08 23:02:14]

On 2017/09/08 20:48:32, mathias wrote:
> On 2017/09/08 20:24:50, Vasily Kuznetsov wrote:
> > On 2017/09/08 19:01:42, mathias wrote:
> > > I wonder why the `git show` commands (e.g. within `GetFileContent()` and
> > > `GetBaseFile()`) would not need a `--binary` flag as well?
> > 
> > Those parts seemed to be working fine -- at least the new version of the
file
> > gets correctly uploaded and can be seen in Rietveld UI. It's only the patch
> that
> > was semi-broken.
> > 
> > I also tried `git show $filehash` and it shows the binary content of a file,
> so
> > that probably explains it.
> 
> Have you tried a binary that changes between patch-sets?

No, but now I did. It works.

[mathias, 2017-09-08 23:10:24]

LGTM.

[tlucas, 2017-09-11 08:27:21]

LGTM