Patchwork [on-crew] patch._applydiff: normalize prefix . to empty string

login
register
mail settings
Submitter Siddharth Agarwal
Date March 19, 2015, 5:19 p.m.
Message ID <fbae66bb86e6f4433996.1426785593@devbig136.prn2.facebook.com>
Download mbox | patch
Permalink /patch/8173/
State Accepted
Headers show

Comments

Siddharth Agarwal - March 19, 2015, 5:19 p.m.
# HG changeset patch
# User Siddharth Agarwal <sid0@fb.com>
# Date 1426785485 25200
#      Thu Mar 19 10:18:05 2015 -0700
# Node ID fbae66bb86e6f4433996e5787e276eb79299fc46
# Parent  7262bbef2ba80ce3ccfc726a19c0e33a419aff04
patch._applydiff: normalize prefix . to empty string

This is kind of an edge case, but it'd be nice for tools that use --prefix to
not have to special case . themselves.

(I made sure to put in the (glob) annotations this time!)
Augie Fackler - March 19, 2015, 5:31 p.m.
On Thu, Mar 19, 2015 at 10:19:53AM -0700, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1426785485 25200
> #      Thu Mar 19 10:18:05 2015 -0700
> # Node ID fbae66bb86e6f4433996e5787e276eb79299fc46
> # Parent  7262bbef2ba80ce3ccfc726a19c0e33a419aff04
> patch._applydiff: normalize prefix . to empty string

I'm a little conflicted about this. What if I'm sitting in dir/ and
say --prefix=.? Shouldn't that work out to --prefix=dir?

>
> This is kind of an edge case, but it'd be nice for tools that use --prefix to
> not have to special case . themselves.
>
> (I made sure to put in the (glob) annotations this time!)
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1796,7 +1796,11 @@
>
>      if prefix:
>          # clean up double slashes, lack of trailing slashes, etc
> -        prefix = util.normpath(prefix) + '/'
> +        prefix = util.normpath(prefix)
> +        if prefix == '.':
> +            prefix = ''
> +        else:
> +            prefix += '/'
>      def pstrip(p):
>          return pathtransform(p, strip - 1, prefix)[1]
>
> diff --git a/tests/test-import-git.t b/tests/test-import-git.t
> --- a/tests/test-import-git.t
> +++ b/tests/test-import-git.t
> @@ -626,6 +626,33 @@
>    adding dir/d
>    adding dir/dir2/b
>    adding dir/dir2/c
> +
> +prefix '.' is the same as no prefix
> +  $ hg import --no-commit --prefix . - <<EOF
> +  > diff --git a/dir/a b/dir/a
> +  > --- /dev/null
> +  > +++ b/dir/a
> +  > @@ -0,0 +1 @@
> +  > +aaaa
> +  > diff --git a/dir/d b/dir/d
> +  > --- a/dir/d
> +  > +++ b/dir/d
> +  > @@ -1,1 +1,2 @@
> +  >  d
> +  > +dddd
> +  > EOF
> +  applying patch from stdin
> +  $ cat dir/a
> +  aaaa
> +  $ cat dir/d
> +  d
> +  dddd
> +  $ hg revert -aC
> +  forgetting dir/a (glob)
> +  reverting dir/d (glob)
> +  $ rm dir/a
> +
> +prefix with default strip
>    $ hg import --no-commit --prefix dir/ - <<EOF
>    > diff --git a/a b/a
>    > --- /dev/null
Siddharth Agarwal - March 19, 2015, 5:37 p.m.
On 03/19/2015 10:31 AM, Augie Fackler wrote:
> On Thu, Mar 19, 2015 at 10:19:53AM -0700, Siddharth Agarwal wrote:
>> # HG changeset patch
>> # User Siddharth Agarwal <sid0@fb.com>
>> # Date 1426785485 25200
>> #      Thu Mar 19 10:18:05 2015 -0700
>> # Node ID fbae66bb86e6f4433996e5787e276eb79299fc46
>> # Parent  7262bbef2ba80ce3ccfc726a19c0e33a419aff04
>> patch._applydiff: normalize prefix . to empty string
> I'm a little conflicted about this. What if I'm sitting in dir/ and
> say --prefix=.? Shouldn't that work out to --prefix=dir?

At least in the current implementation, prefix is root-relative, not
cwd-relative. I chose that because (a) import takes no other
path-related arguments and (b) the paths import does take (via filenames
in the patch) are root-relative.
Augie Fackler - March 19, 2015, 5:47 p.m.
On Thu, Mar 19, 2015 at 1:37 PM, Siddharth Agarwal <sid@less-broken.com> wrote:
> On 03/19/2015 10:31 AM, Augie Fackler wrote:
>> On Thu, Mar 19, 2015 at 10:19:53AM -0700, Siddharth Agarwal wrote:
>>> # HG changeset patch
>>> # User Siddharth Agarwal <sid0@fb.com>
>>> # Date 1426785485 25200
>>> #      Thu Mar 19 10:18:05 2015 -0700
>>> # Node ID fbae66bb86e6f4433996e5787e276eb79299fc46
>>> # Parent  7262bbef2ba80ce3ccfc726a19c0e33a419aff04
>>> patch._applydiff: normalize prefix . to empty string
>> I'm a little conflicted about this. What if I'm sitting in dir/ and
>> say --prefix=.? Shouldn't that work out to --prefix=dir?
>
> At least in the current implementation, prefix is root-relative, not
> cwd-relative. I chose that because (a) import takes no other
> path-related arguments and (b) the paths import does take (via filenames
> in the patch) are root-relative.

Not sure it's consistent enough on the CLI.  'hg status .' is
$PWD-relative, which is the strongest non-changeset '.' association I
have for hg. I know I'd get surprised by a root-relative . here.
Siddharth Agarwal - March 19, 2015, 6:10 p.m.
On 03/19/2015 10:19 AM, Siddharth Agarwal wrote:
> # HG changeset patch
> # User Siddharth Agarwal <sid0@fb.com>
> # Date 1426785485 25200
> #      Thu Mar 19 10:18:05 2015 -0700
> # Node ID fbae66bb86e6f4433996e5787e276eb79299fc46
> # Parent  7262bbef2ba80ce3ccfc726a19c0e33a419aff04
> patch._applydiff: normalize prefix . to empty string
>
> This is kind of an edge case, but it'd be nice for tools that use --prefix to
> not have to special case . themselves.

Per discussion on IRC, I'm going to change --prefix to be relative to
the cwd. Please drop this patch.

- Siddharth

>
> (I made sure to put in the (glob) annotations this time!)
>
> diff --git a/mercurial/patch.py b/mercurial/patch.py
> --- a/mercurial/patch.py
> +++ b/mercurial/patch.py
> @@ -1796,7 +1796,11 @@
>  
>      if prefix:
>          # clean up double slashes, lack of trailing slashes, etc
> -        prefix = util.normpath(prefix) + '/'
> +        prefix = util.normpath(prefix)
> +        if prefix == '.':
> +            prefix = ''
> +        else:
> +            prefix += '/'
>      def pstrip(p):
>          return pathtransform(p, strip - 1, prefix)[1]
>  
> diff --git a/tests/test-import-git.t b/tests/test-import-git.t
> --- a/tests/test-import-git.t
> +++ b/tests/test-import-git.t
> @@ -626,6 +626,33 @@
>    adding dir/d
>    adding dir/dir2/b
>    adding dir/dir2/c
> +
> +prefix '.' is the same as no prefix
> +  $ hg import --no-commit --prefix . - <<EOF
> +  > diff --git a/dir/a b/dir/a
> +  > --- /dev/null
> +  > +++ b/dir/a
> +  > @@ -0,0 +1 @@
> +  > +aaaa
> +  > diff --git a/dir/d b/dir/d
> +  > --- a/dir/d
> +  > +++ b/dir/d
> +  > @@ -1,1 +1,2 @@
> +  >  d
> +  > +dddd
> +  > EOF
> +  applying patch from stdin
> +  $ cat dir/a
> +  aaaa
> +  $ cat dir/d
> +  d
> +  dddd
> +  $ hg revert -aC
> +  forgetting dir/a (glob)
> +  reverting dir/d (glob)
> +  $ rm dir/a
> +
> +prefix with default strip
>    $ hg import --no-commit --prefix dir/ - <<EOF
>    > diff --git a/a b/a
>    > --- /dev/null
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

Patch

diff --git a/mercurial/patch.py b/mercurial/patch.py
--- a/mercurial/patch.py
+++ b/mercurial/patch.py
@@ -1796,7 +1796,11 @@ 
 
     if prefix:
         # clean up double slashes, lack of trailing slashes, etc
-        prefix = util.normpath(prefix) + '/'
+        prefix = util.normpath(prefix)
+        if prefix == '.':
+            prefix = ''
+        else:
+            prefix += '/'
     def pstrip(p):
         return pathtransform(p, strip - 1, prefix)[1]
 
diff --git a/tests/test-import-git.t b/tests/test-import-git.t
--- a/tests/test-import-git.t
+++ b/tests/test-import-git.t
@@ -626,6 +626,33 @@ 
   adding dir/d
   adding dir/dir2/b
   adding dir/dir2/c
+
+prefix '.' is the same as no prefix
+  $ hg import --no-commit --prefix . - <<EOF
+  > diff --git a/dir/a b/dir/a
+  > --- /dev/null
+  > +++ b/dir/a
+  > @@ -0,0 +1 @@
+  > +aaaa
+  > diff --git a/dir/d b/dir/d
+  > --- a/dir/d
+  > +++ b/dir/d
+  > @@ -1,1 +1,2 @@
+  >  d
+  > +dddd
+  > EOF
+  applying patch from stdin
+  $ cat dir/a
+  aaaa
+  $ cat dir/d
+  d
+  dddd
+  $ hg revert -aC
+  forgetting dir/a (glob)
+  reverting dir/d (glob)
+  $ rm dir/a
+
+prefix with default strip
   $ hg import --no-commit --prefix dir/ - <<EOF
   > diff --git a/a b/a
   > --- /dev/null