Patchwork [4,of,5,relative-diff] patch.trydiff: add support for stripping a relative root

login
register
mail settings
Submitter Siddharth Agarwal
Date March 17, 2015, 10:57 p.m.
Message ID <5f0dfcf33d3c1c06856c.1426633049@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8128/
State Superseded
Headers show

Comments

Siddharth Agarwal - March 17, 2015, 10:57 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1426622381 25200
#      Tue Mar 17 12:59:41 2015 -0700
# Node ID 5f0dfcf33d3c1c06856c9271f8006e9eb7898368
# Parent  194f4ddf99b761fbb2bc883cc93629ecc09c0c84
patch.trydiff: add support for stripping a relative root

This assumes that if relroot is not None, all the files in modified, added and
removed start with it. In upcoming patches we'll follow that.
Martin von Zweigbergk - March 17, 2015, 11:43 p.m.
On Tue, Mar 17, 2015 at 3:58 PM Siddharth Agarwal <sid0@fb.com> wrote:

> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1426622381 25200
> #      Tue Mar 17 12:59:41 2015 -0700
> # Node ID 5f0dfcf33d3c1c06856c9271f8006e9eb7898368
> # Parent  194f4ddf99b761fbb2bc883cc93629ecc09c0c84
> patch.trydiff: add support for stripping a relative root
>
> This assumes that if relroot is not None, all the files in modified, added
> and
> removed start with it. In upcoming patches we'll follow that.
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -2097,7 +2097,7 @@
>
>      def difffn(opts, losedata):
>          return trydiff(repo, revs, ctx1, ctx2, modified, added, removed,
> -                       copy, getfilectx, opts, losedata, prefix)
> +                       copy, getfilectx, opts, losedata, prefix, '')
>      if opts.upgrade and not opts.git:
>          try:
>              def losedata(fn):
> @@ -2205,13 +2205,16 @@
>          yield f1, f2, copyop
>
>  def trydiff(repo, revs, ctx1, ctx2, modified, added, removed,
> -            copy, getfilectx, opts, losedatafn, prefix):
> +            copy, getfilectx, opts, losedatafn, prefix, relroot):
>      '''given input data, generate a diff and yield it in blocks
>
>      If generating a diff would lose data like flags or binary data and
>      losedatafn is not None, it will be called.
>
> -    prefix is added to every path in the diff output.'''
> +    relroot is removed and prefix is added to every path in the diff
> output.
> +
> +    If relroot is not empty, this function expects every path in modified,
> +    added, removed and copy to start with it.'''
>

Too costly to check that they really do? I saw some patches from Augie
about [devel] config. Would that make sense? Or it's just too unlikely to
break so it's not worth it? I would accept that answer. It's just a little
funny that the 'relroot' content is ignored. I'm sure you reflected on this
too.


>
>      def gitindex(text):
>          if not text:
> @@ -2268,8 +2271,10 @@
>                  (f1 and f2 and flag1 != flag2)):
>                  losedatafn(f2 or f1)
>
> -        path1 = posixpath.join(prefix, f1 or f2)
> -        path2 = posixpath.join(prefix, f2 or f1)
> +        path1 = f1 or f2
> +        path2 = f2 or f1
> +        path1 = posixpath.join(prefix, path1[len(relroot):])
> +        path2 = posixpath.join(prefix, path2[len(relroot):])
>          header = []
>          if opts.git:
>              header.append('diff --git %s%s %s%s' %
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
>
Siddharth Agarwal - March 18, 2015, 12:16 a.m.
On 03/17/2015 04:43 PM, Martin von Zweigbergk wrote:
>
> Too costly to check that they really do? I saw some patches from Augie
> about [devel] config. Would that make sense? Or it's just too unlikely
> to break so it's not worth it? I would accept that answer. It's just a
> little funny that the 'relroot' content is ignored. I'm sure you
> reflected on this too.

Reasonably unlikely it'll break -- this function is very low level and I
can't think of anyone that would want to use it. Adding devel checks
(and turning them on in tests) makes sense I think.

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -2097,7 +2097,7 @@ 
 
     def difffn(opts, losedata):
         return trydiff(repo, revs, ctx1, ctx2, modified, added, removed,
-                       copy, getfilectx, opts, losedata, prefix)
+                       copy, getfilectx, opts, losedata, prefix, '')
     if opts.upgrade and not opts.git:
         try:
             def losedata(fn):
@@ -2205,13 +2205,16 @@ 
         yield f1, f2, copyop
 
 def trydiff(repo, revs, ctx1, ctx2, modified, added, removed,
-            copy, getfilectx, opts, losedatafn, prefix):
+            copy, getfilectx, opts, losedatafn, prefix, relroot):
     '''given input data, generate a diff and yield it in blocks
 
     If generating a diff would lose data like flags or binary data and
     losedatafn is not None, it will be called.
 
-    prefix is added to every path in the diff output.'''
+    relroot is removed and prefix is added to every path in the diff output.
+
+    If relroot is not empty, this function expects every path in modified,
+    added, removed and copy to start with it.'''
 
     def gitindex(text):
         if not text:
@@ -2268,8 +2271,10 @@ 
                 (f1 and f2 and flag1 != flag2)):
                 losedatafn(f2 or f1)
 
-        path1 = posixpath.join(prefix, f1 or f2)
-        path2 = posixpath.join(prefix, f2 or f1)
+        path1 = f1 or f2
+        path2 = f2 or f1
+        path1 = posixpath.join(prefix, path1[len(relroot):])
+        path2 = posixpath.join(prefix, path2[len(relroot):])
         header = []
         if opts.git:
             header.append('diff --git %s%s %s%s' %