Patchwork [STABLE] cmdutil.jsonchangeset: properly compute added and removed files

login
register
mail settings
Submitter Gregory Szorc
Date Jan. 6, 2015, 6:19 a.m.
Message ID <d7b2b6b823ecdbdfc42f.1420525164@3.1.168.192.in-addr.arpa>
Download mbox | patch
Permalink /patch/7325/
State Accepted
Commit f4e6475950f18a08663c2f4ea8f6352a1b0b55fa
Headers show

Comments

Gregory Szorc - Jan. 6, 2015, 6:19 a.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1420525135 28800
#      Mon Jan 05 22:18:55 2015 -0800
# Branch stable
# Node ID d7b2b6b823ecdbdfc42f164a40dd07175343e581
# Parent  aafeaba22826f27cb976e8f0b5ae028e86fd607e
cmdutil.jsonchangeset: properly compute added and removed files

jsonchangeset._show() was computing the reverse status of the current
changeset. As a result, added files were showing up as removed and
removed files were showing up as adds.

There were existing tests for this code and they were flat out wrong.
Sean Farley - Jan. 6, 2015, 6:38 a.m.
Gregory Szorc writes:

> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1420525135 28800
> #      Mon Jan 05 22:18:55 2015 -0800
> # Branch stable
> # Node ID d7b2b6b823ecdbdfc42f164a40dd07175343e581
> # Parent  aafeaba22826f27cb976e8f0b5ae028e86fd607e
> cmdutil.jsonchangeset: properly compute added and removed files
>
> jsonchangeset._show() was computing the reverse status of the current
> changeset. As a result, added files were showing up as removed and
> removed files were showing up as adds.
>
> There were existing tests for this code and they were flat out wrong.
>
> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
> --- a/mercurial/cmdutil.py
> +++ b/mercurial/cmdutil.py
> @@ -1071,9 +1071,9 @@ class jsonchangeset(changeset_printer):
>              self.ui.write(',\n  "extra": {%s}' %
>                            ", ".join('"%s": "%s"' % (j(k), j(v))
>                                      for k, v in ctx.extra().items()))
>  
> -            files = ctx.status(ctx.p1())
> +            files = self.repo.status(ctx.p1().node(), ctx.node())

Calling repo.status is the old way which I worked hard to remove :-) You
should just switch the order:

ctx.p1().status(ctx)
Gregory Szorc - Jan. 6, 2015, 6:43 a.m.
On 1/5/15 10:38 PM, Sean Farley wrote:
>
> Gregory Szorc writes:
>
>> # HG changeset patch
>> # User Gregory Szorc <gregory.szorc@gmail.com>
>> # Date 1420525135 28800
>> #      Mon Jan 05 22:18:55 2015 -0800
>> # Branch stable
>> # Node ID d7b2b6b823ecdbdfc42f164a40dd07175343e581
>> # Parent  aafeaba22826f27cb976e8f0b5ae028e86fd607e
>> cmdutil.jsonchangeset: properly compute added and removed files
>>
>> jsonchangeset._show() was computing the reverse status of the current
>> changeset. As a result, added files were showing up as removed and
>> removed files were showing up as adds.
>>
>> There were existing tests for this code and they were flat out wrong.
>>
>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>> --- a/mercurial/cmdutil.py
>> +++ b/mercurial/cmdutil.py
>> @@ -1071,9 +1071,9 @@ class jsonchangeset(changeset_printer):
>>               self.ui.write(',\n  "extra": {%s}' %
>>                             ", ".join('"%s": "%s"' % (j(k), j(v))
>>                                       for k, v in ctx.extra().items()))
>>
>> -            files = ctx.status(ctx.p1())
>> +            files = self.repo.status(ctx.p1().node(), ctx.node())
>
> Calling repo.status is the old way which I worked hard to remove :-) You
> should just switch the order:
>
> ctx.p1().status(ctx)
>

Sorry about that. In my defense, I did copy the pattern from elsewhere. 
Maybe that's because I was looking at stable when I made the patch?

Anyway, I reckon this can be fixed in flight?
Sean Farley - Jan. 6, 2015, 7:01 a.m.
Gregory Szorc writes:

> On 1/5/15 10:38 PM, Sean Farley wrote:
>>
>> Gregory Szorc writes:
>>
>>> # HG changeset patch
>>> # User Gregory Szorc <gregory.szorc@gmail.com>
>>> # Date 1420525135 28800
>>> #      Mon Jan 05 22:18:55 2015 -0800
>>> # Branch stable
>>> # Node ID d7b2b6b823ecdbdfc42f164a40dd07175343e581
>>> # Parent  aafeaba22826f27cb976e8f0b5ae028e86fd607e
>>> cmdutil.jsonchangeset: properly compute added and removed files
>>>
>>> jsonchangeset._show() was computing the reverse status of the current
>>> changeset. As a result, added files were showing up as removed and
>>> removed files were showing up as adds.
>>>
>>> There were existing tests for this code and they were flat out wrong.
>>>
>>> diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
>>> --- a/mercurial/cmdutil.py
>>> +++ b/mercurial/cmdutil.py
>>> @@ -1071,9 +1071,9 @@ class jsonchangeset(changeset_printer):
>>>               self.ui.write(',\n  "extra": {%s}' %
>>>                             ", ".join('"%s": "%s"' % (j(k), j(v))
>>>                                       for k, v in ctx.extra().items()))
>>>
>>> -            files = ctx.status(ctx.p1())
>>> +            files = self.repo.status(ctx.p1().node(), ctx.node())
>>
>> Calling repo.status is the old way which I worked hard to remove :-) You
>> should just switch the order:
>>
>> ctx.p1().status(ctx)
>>
>
> Sorry about that. In my defense, I did copy the pattern from elsewhere. 
> Maybe that's because I was looking at stable when I made the patch?

No worries :-)

> Anyway, I reckon this can be fixed in flight?

Sure, but it's not my decision to make.
Matt Mackall - Jan. 6, 2015, 8:49 p.m.
On Mon, 2015-01-05 at 22:19 -0800, Gregory Szorc wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1420525135 28800
> #      Mon Jan 05 22:18:55 2015 -0800
> # Branch stable
> # Node ID d7b2b6b823ecdbdfc42f164a40dd07175343e581
> # Parent  aafeaba22826f27cb976e8f0b5ae028e86fd607e
> cmdutil.jsonchangeset: properly compute added and removed files
> 
> jsonchangeset._show() was computing the reverse status of the current
> changeset. As a result, added files were showing up as removed and
> removed files were showing up as adds.
> 
> There were existing tests for this code and they were flat out wrong.

Queued with smf's tweak, thanks.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -1071,9 +1071,9 @@  class jsonchangeset(changeset_printer):
             self.ui.write(',\n  "extra": {%s}' %
                           ", ".join('"%s": "%s"' % (j(k), j(v))
                                     for k, v in ctx.extra().items()))
 
-            files = ctx.status(ctx.p1())
+            files = self.repo.status(ctx.p1().node(), ctx.node())
             self.ui.write(',\n  "modified": [%s]' %
                           ", ".join('"%s"' % j(f) for f in files[0]))
             self.ui.write(',\n  "added": [%s]' %
                           ", ".join('"%s"' % j(f) for f in files[1]))
diff --git a/tests/test-command-template.t b/tests/test-command-template.t
--- a/tests/test-command-template.t
+++ b/tests/test-command-template.t
@@ -695,10 +695,10 @@  Test JSON style:
     "parents": ["29114dbae42b9f078cf2714dbe3a86bba8ec7453"],
     "manifest": "94961b75a2da554b4df6fb599e5bfc7d48de0c64",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": ["second"],
-    "removed": ["fourth", "third"]
+    "added": ["fourth", "third"],
+    "removed": ["second"]
    },
    {
     "rev": 7,
     "node": "29114dbae42b9f078cf2714dbe3a86bba8ec7453",
@@ -712,10 +712,10 @@  Test JSON style:
     "parents": ["0000000000000000000000000000000000000000"],
     "manifest": "f2dbc354b94e5ec0b4f10680ee0cee816101d0bf",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": [],
-    "removed": ["second"]
+    "added": ["second"],
+    "removed": []
    },
    {
     "rev": 6,
     "node": "d41e714fe50d9e4a5f11b4d595d543481b5f980b",
@@ -746,10 +746,10 @@  Test JSON style:
     "parents": ["10e46f2dcbf4823578cf180f33ecf0b957964c47"],
     "manifest": "4dc3def4f9b4c6e8de820f6ee74737f91e96a216",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": [],
-    "removed": ["d"]
+    "added": ["d"],
+    "removed": []
    },
    {
     "rev": 4,
     "node": "bbe44766e73d5f11ed2177f1838de10c53ef3e74",
@@ -797,10 +797,10 @@  Test JSON style:
     "parents": ["b608e9d1a3f0273ccf70fb85fd6866b3482bf965"],
     "manifest": "6e0e82995c35d0d57a52aca8da4e56139e06b4b1",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": [],
-    "removed": ["c"]
+    "added": ["c"],
+    "removed": []
    },
    {
     "rev": 1,
     "node": "b608e9d1a3f0273ccf70fb85fd6866b3482bf965",
@@ -814,10 +814,10 @@  Test JSON style:
     "parents": ["1e4e1b8f71e05681d422154f5421e385fec3454f"],
     "manifest": "4e8d705b1e53e3f9375e0e60dc7b525d8211fe55",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": [],
-    "removed": ["b"]
+    "added": ["b"],
+    "removed": []
    },
    {
     "rev": 0,
     "node": "1e4e1b8f71e05681d422154f5421e385fec3454f",
@@ -831,10 +831,10 @@  Test JSON style:
     "parents": ["0000000000000000000000000000000000000000"],
     "manifest": "a0c8bcbbb45c63b90b70ad007bf38961f64f2af0",
     "extra": {"branch": "default"},
     "modified": [],
-    "added": [],
-    "removed": ["a"]
+    "added": ["a"],
+    "removed": []
    }
   ]
 
 Error if style not readable: