Submitter | Alexander Fomin |
---|---|
Date | April 5, 2017, 10:51 p.m. |
Message ID | <f3814cce4dda8e809073.1491432709@devvm2125.lla2.facebook.com> |
Download | mbox | patch |
Permalink | /patch/19975/ |
State | Accepted |
Headers | show |
Comments
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.
Patch
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