Patchwork [5,of,7] patch: require text mode (-a) for binary patches in git mode (issue5510) (BC)

login
register
mail settings
Submitter Alexander Fomin
Date March 21, 2017, 5:08 p.m.
Message ID <98301edf86daecef6ef4.1490116118@devvm2125.lla2.facebook.com>
Download mbox | patch
Permalink /patch/19532/
State Changes Requested
Headers show

Comments

Alexander Fomin - March 21, 2017, 5:08 p.m.
# HG changeset patch
# User Alexander Fomin <afomin@fb.com>
# Date 1490105843 25200
#      Tue Mar 21 07:17:23 2017 -0700
# Node ID 98301edf86daecef6ef43357b0934c351f58574d
# Parent  b1eb6801f90a45ef43f17a22d9835ccba4ac190b
patch: require text mode (-a) for binary patches in git mode (issue5510) (BC)

This changeset makes patch explicitly request binary diffs generation by using
-a/--text option even when in Git mode.
Augie Fackler - March 21, 2017, 9:29 p.m.
On Tue, Mar 21, 2017 at 10:08:38AM -0700, Alexander Fomin wrote:
> # HG changeset patch
> # User Alexander Fomin <afomin@fb.com>
> # Date 1490105843 25200
> #      Tue Mar 21 07:17:23 2017 -0700
> # Node ID 98301edf86daecef6ef43357b0934c351f58574d
> # Parent  b1eb6801f90a45ef43f17a22d9835ccba4ac190b
> patch: require text mode (-a) for binary patches in git mode (issue5510) (BC)
>
> This changeset makes patch explicitly request binary diffs generation by using
> -a/--text option even when in Git mode.

Can I request that the test in patch 6 be introduced as patch 5 with
the existing behavior, then this patch would show off the behavior
change as an edit to a test file?

My gut feeling is that this is a wontfix issue[0], but I'd like to see a
before/after case in order to make that determination.

0: Specifically, the -a option to diff in my mind means "treat all
files as text", which means that the bug in git-diff mode (if any) is
that it still vomits base85 crud to your terminal instead of trying to
treat the file as plain text.

>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2358,7 +2358,8 @@ def diffhunks(repo, node1=None, node2=No
>              # Buffer the whole output until we are sure it can be generated
>              return list(difffn(opts.copy(git=False), losedata))
>          except GitDiffRequired:
> -            return difffn(opts.copy(git=True), None)
> +            # Explicitly request binary diffs if Git mode is required
> +            return difffn(opts.copy(git=True, text=True), None)
>      else:
>          return difffn(opts, None)
>
> @@ -2552,7 +2553,7 @@ def trydiff(repo, revs, ctx1, ctx2, modi
>          elif revs and not repo.ui.quiet:
>              header.append(diffline(path1, revs))
>
> -        if binary and opts.git and not opts.nobinary:
> +        if binary and opts.git and opts.text and not opts.nobinary:
>              text = mdiff.b85diff(content1, content2)
>              if text:
>                  header.append('index %s..%s' %
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2358,7 +2358,8 @@  def diffhunks(repo, node1=None, node2=No
             # Buffer the whole output until we are sure it can be generated
             return list(difffn(opts.copy(git=False), losedata))
         except GitDiffRequired:
-            return difffn(opts.copy(git=True), None)
+            # Explicitly request binary diffs if Git mode is required
+            return difffn(opts.copy(git=True, text=True), None)
     else:
         return difffn(opts, None)
 
@@ -2552,7 +2553,7 @@  def trydiff(repo, revs, ctx1, ctx2, modi
         elif revs and not repo.ui.quiet:
             header.append(diffline(path1, revs))
 
-        if binary and opts.git and not opts.nobinary:
+        if binary and opts.git and opts.text and not opts.nobinary:
             text = mdiff.b85diff(content1, content2)
             if text:
                 header.append('index %s..%s' %