Patchwork [3,of,3,v2] hg: make --binary option for git diffs default to off (BC)

# HG changeset patch
# User Alexander Fomin <afomin@fb.com>
# Date 1491431909 25200
#      Wed Apr 05 15:38:29 2017 -0700
# Node ID f3814cce4dda8e8090736f642b694ce7cd3792bb
# Parent  900f393807023064cb484e86e3ccffa6338b55bb
hg: make --binary option for git diffs default to off (BC)

This patch is a follow-up that makes --binary option default to off in order to
avoid generating binary diffs in Git mode unless explicitly requested by a
user (as discussed in issue5510).

On 4/5/17 11:51 PM, Alexander Fomin wrote:
> # HG changeset patch
> # User Alexander Fomin <afomin@fb.com>
> # Date 1491431909 25200
> #      Wed Apr 05 15:38:29 2017 -0700
> # Node ID f3814cce4dda8e8090736f642b694ce7cd3792bb
> # Parent  900f393807023064cb484e86e3ccffa6338b55bb
> hg: make --binary option for git diffs default to off (BC)

Awesome! This series looks good to me. We have to have a broader 
discussion about the BC break in this last patch (it will probably break 
a lot of --git users, but it seems "more correct" -- is it worth it?), 
but the first two seem like pretty straightforward improvements to me. 
I've marked them as pre-reviewed in patchwork.

>
> This patch is a follow-up that makes --binary option default to off in order to
> avoid generating binary diffs in Git mode unless explicitly requested by a
> user (as discussed in issue5510).
>
> diff --git a/hgext/mq.py b/hgext/mq.py
> --- a/hgext/mq.py
> +++ b/hgext/mq.py
> @@ -513,6 +513,7 @@ class queue(object):
>           diffopts = patchmod.diffopts(self.ui, opts)
>           if self.gitmode == 'auto':
>               diffopts.upgrade = True
> +            diffopts.nobinary = False
>           elif self.gitmode == 'keep':
>               pass
>           elif self.gitmode in ('yes', 'no'):
> diff --git a/hgext/transplant.py b/hgext/transplant.py
> --- a/hgext/transplant.py
> +++ b/hgext/transplant.py
> @@ -143,6 +143,7 @@ class transplanter(object):
>           pulls = []
>           diffopts = patch.difffeatureopts(self.ui, opts)
>           diffopts.git = True
> +        diffopts.nobinary = False
>   
>           lock = tr = None
>           try:
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -162,6 +162,7 @@ def dorecord(ui, repo, commitfunc, cmdsu
>           diffopts = patch.difffeatureopts(ui, opts=opts, whitespace=True)
>           diffopts.nodates = True
>           diffopts.git = True
> +        diffopts.nobinary = False
>           diffopts.showfunc = True
>           originaldiff = patch.diff(repo, changes=status, opts=diffopts)
>           originalchunks = patch.parsepatch(originaldiff)
> diff --git a/mercurial/commands.py b/mercurial/commands.py
> --- a/mercurial/commands.py
> +++ b/mercurial/commands.py
> @@ -160,7 +160,7 @@ logopts = [
>   diffopts = [
>       ('a', 'text', None, _('treat all files as text')),
>       ('g', 'git', None, _('use git extended diff format')),
> -    ('', 'binary', None, _('generate binary diffs in git mode (default)')),
> +    ('', 'binary', None, _('generate binary diffs in git mode')),
>       ('', 'nodates', None, _('omit dates from diff headers'))
>   ]
>   
> diff --git a/mercurial/mdiff.py b/mercurial/mdiff.py
> --- a/mercurial/mdiff.py
> +++ b/mercurial/mdiff.py
> @@ -50,7 +50,7 @@ class diffopts(object):
>           'showfunc': False,
>           'git': False,
>           'nodates': False,
> -        'nobinary': False,
> +        'nobinary': True,
>           'noprefix': False,
>           'index': 0,
>           'ignorews': False,
> diff --git a/tests/autodiff.py b/tests/autodiff.py
> --- a/tests/autodiff.py
> +++ b/tests/autodiff.py
> @@ -26,6 +26,7 @@ def autodiff(ui, repo, *pats, **opts):
>       elif git == 'auto':
>           diffopts.git = False
>           diffopts.upgrade = True
> +        diffopts.nobinary = False
>       elif git == 'warn':
>           diffopts.git = False
>           diffopts.upgrade = True
> diff --git a/tests/test-diff-binary-file.t b/tests/test-diff-binary-file.t
> --- a/tests/test-diff-binary-file.t
> +++ b/tests/test-diff-binary-file.t
> @@ -19,7 +19,7 @@
>   
>     $ hg diff --nodates -r 0 -r 2
>   
> -  $ hg diff --git -r 0 -r 1
> +  $ hg diff --git --binary -r 0 -r 1
>     diff --git a/binfile.bin b/binfile.bin
>     index 37ba3d1c6f17137d9c5f5776fa040caf5fe73ff9..58dc31a9e2f40f74ff3b45903f7d620b8e5b7356
>     GIT binary patch
> @@ -64,7 +64,7 @@
>     
>   
>   
> -  $ hg diff --git -r 2 -r 3
> +  $ hg diff --git --binary -r 2 -r 3
>     diff --git a/binfile.bin b/nonbinfile
>     copy from binfile.bin
>     copy to nonbinfile
> diff --git a/tests/test-git-export.t b/tests/test-git-export.t
> --- a/tests/test-git-export.t
> +++ b/tests/test-git-export.t
> @@ -342,7 +342,7 @@ Binary diff:
>   
>     $ cp "$TESTDIR/binfile.bin" .
>     $ hg add binfile.bin
> -  $ hg diff --git > b.diff
> +  $ hg diff --git --binary > b.diff
>     $ cat b.diff
>     diff --git a/binfile.bin b/binfile.bin
>     new file mode 100644
> diff --git a/tests/test-help.t b/tests/test-help.t
> --- a/tests/test-help.t
> +++ b/tests/test-help.t
> @@ -543,7 +543,7 @@ Test command without options
>      -c --change REV          change made by revision
>      -a --text                treat all files as text
>      -g --git                 use git extended diff format
> -      --binary              generate binary diffs in git mode (default)
> +      --binary              generate binary diffs in git mode
>         --nodates             omit dates from diff headers
>         --noprefix            omit a/ and b/ prefixes from filenames
>      -p --show-function       show which function each change is in
> diff --git a/tests/test-import-eol.t b/tests/test-import-eol.t
> --- a/tests/test-import-eol.t
> +++ b/tests/test-import-eol.t
> @@ -131,7 +131,7 @@ Test --eol and binary patches
>     $ hg ci -Am addb
>     adding b
>     $ $PYTHON -c 'file("b", "wb").write("a\x00\nc\r\nd")'
> -  $ hg diff --git > bin.diff
> +  $ hg diff --git --binary > bin.diff
>     $ hg revert --no-backup b
>   
>   binary patch with --eol

On Thu, 6 Apr 2017 10:09:48 +0100, Ryan McElroy wrote:
> On 4/5/17 11:51 PM, Alexander Fomin wrote:
> > # HG changeset patch
> > # User Alexander Fomin <afomin@fb.com>
> > # Date 1491431909 25200
> > #      Wed Apr 05 15:38:29 2017 -0700
> > # Node ID f3814cce4dda8e8090736f642b694ce7cd3792bb
> > # Parent  900f393807023064cb484e86e3ccffa6338b55bb
> > hg: make --binary option for git diffs default to off (BC)
> 
> Awesome! This series looks good to me. We have to have a broader 
> discussion about the BC break in this last patch (it will probably break 
> a lot of --git users, but it seems "more correct" -- is it worth it?), 
> but the first two seem like pretty straightforward improvements to me. 
> I've marked them as pre-reviewed in patchwork.

I don't think this is trivial enough to change now. I vote against it.

Also, "git format-patch", which I think is equivalent to "hg export", includes
binary diff by default.