Patchwork [3,of,3,STABLE] revert: look for copies information for all local modifications

login
register
mail settings
Submitter Pierre-Yves David
Date Nov. 26, 2014, 11:30 p.m.
Message ID <18bcfb55b93d76ad6a9a.1417044630@marginatus.alto.octopoid.net>
Download mbox | patch
Permalink /patch/6868/
State Superseded
Headers show

Comments

Pierre-Yves David - Nov. 26, 2014, 11:30 p.m.
# HG changeset patch
# User Pierre-Yves David <pierre-yves.david@fb.com>
# Date 1416973254 28800
#      Tue Nov 25 19:40:54 2014 -0800
# Branch stable
# Node ID 18bcfb55b93d76ad6a9a097e203036ac22504e83
# Parent  1866e1771d90ea39c1abc2bf677f7acd48bca701
revert: look for copies information for all local modifications

Renaming a file over an existing one marks the file as modified. So we
track rename source in modified file too.
Martin von Zweigbergk - Nov. 27, 2014, 12:29 a.m.
On Wed Nov 26 2014 at 3:31:37 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

> +Also true for move overwriting existing file
> +
> +  $ hg mv --force a b/b
> +  $ hg revert b/b
> +  $ hg status a b/b
> +
>

Could you remind me what the output would have been before this patch?
Pierre-Yves David - Nov. 27, 2014, 12:32 a.m.
On 11/26/2014 04:29 PM, Martin von Zweigbergk wrote:
>
> On Wed Nov 26 2014 at 3:31:37 PM Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>     +Also true for move overwriting existing file
>     +
>     +  $ hg mv --force a b/b
>     +  $ hg revert b/b
>     +  $ hg status a b/b
>     +
>
>
> Could you remind me what the output would have been before this patch?


B would not have been removed.
Martin von Zweigbergk - Nov. 27, 2014, 12:38 a.m.
On Wed Nov 26 2014 at 4:33:16 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 11/26/2014 04:29 PM, Martin von Zweigbergk wrote:
> >
> > On Wed Nov 26 2014 at 3:31:37 PM Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> > wrote:
> >
> >     +Also true for move overwriting existing file
> >     +
> >     +  $ hg mv --force a b/b
> >     +  $ hg revert b/b
> >     +  $ hg status a b/b
> >     +
> >
> >
> > Could you remind me what the output would have been before this patch?
>
>
> B would not have been removed.
>

It is still not removed... which makes sense, but it's no difference.
Pierre-Yves David - Nov. 27, 2014, 12:43 a.m.
On 11/26/2014 04:38 PM, Martin von Zweigbergk wrote:
>
> On Wed Nov 26 2014 at 4:33:16 PM Pierre-Yves David
> <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> wrote:
>
>
>
>     On 11/26/2014 04:29 PM, Martin von Zweigbergk wrote:
>      >
>      > On Wed Nov 26 2014 at 3:31:37 PM Pierre-Yves David
>      > <pierre-yves.david@ens-lyon.__org
>     <mailto:pierre-yves.david@ens-lyon.org>
>     <mailto:pierre-yves.david@ens-__lyon.org
>     <mailto:pierre-yves.david@ens-lyon.org>>>
>      > wrote:
>      >
>      >     +Also true for move overwriting existing file
>      >     +
>      >     +  $ hg mv --force a b/b
>      >     +  $ hg revert b/b
>      >     +  $ hg status a b/b
>      >     +
>      >
>      >
>      > Could you remind me what the output would have been before this
>     patch?
>
>
>     B would not have been removed.
>
>
> It is still not removed... which makes sense, but it's no difference.

What I wanted to write is:

Before this patch, a (the source of the rename) is left removed. After 
this patch it is properly restored when 'b/b' (the destination of the 
rename) is reverted.

Sorry for the extra round trip.
Martin von Zweigbergk - Nov. 27, 2014, 12:50 a.m.
On Wed Nov 26 2014 at 4:44:00 PM Pierre-Yves David <
pierre-yves.david@ens-lyon.org> wrote:

>
>
> On 11/26/2014 04:38 PM, Martin von Zweigbergk wrote:
> >
> > On Wed Nov 26 2014 at 4:33:16 PM Pierre-Yves David
> > <pierre-yves.david@ens-lyon.org <mailto:pierre-yves.david@ens-lyon.org>>
> > wrote:
> >
> >
> >
> >     On 11/26/2014 04:29 PM, Martin von Zweigbergk wrote:
> >      >
> >      > On Wed Nov 26 2014 at 3:31:37 PM Pierre-Yves David
> >      > <pierre-yves.david@ens-lyon.__org
> >     <mailto:pierre-yves.david@ens-lyon.org>
> >     <mailto:pierre-yves.david@ens-__lyon.org
> >     <mailto:pierre-yves.david@ens-lyon.org>>>
> >      > wrote:
> >      >
> >      >     +Also true for move overwriting existing file
> >      >     +
> >      >     +  $ hg mv --force a b/b
> >      >     +  $ hg revert b/b
> >      >     +  $ hg status a b/b
> >      >     +
> >      >
> >      >
> >      > Could you remind me what the output would have been before this
> >     patch?
> >
> >
> >     B would not have been removed.
> >
> >
> > It is still not removed... which makes sense, but it's no difference.
>
> What I wanted to write is:
>
> Before this patch, a (the source of the rename) is left removed. After
> this patch it is properly restored when 'b/b' (the destination of the
> rename) is reverted.
>
>
Interesting. I would not expect a to be restored since the user explicitly
asked to restore b/b. Did you? The patch is not tagged with issue4458, so I
assume it's not needed for mq.

Patch

diff --git a/mercurial/cmdutil.py b/mercurial/cmdutil.py
--- a/mercurial/cmdutil.py
+++ b/mercurial/cmdutil.py
@@ -2529,16 +2529,20 @@  def revert(ui, repo, ctx, parents, *pats
         # or just forget etc)
         if parent == node:
             dsmodified = modified
             dsadded = added
             dsremoved = removed
+            # store all local modification, useful later for renames detection
+            localchanges = dsmodified | dsadded
             modified, added, removed = set(), set(), set()
         else:
             changes = repo.status(node1=parent, match=m)
             dsmodified = set(changes[0])
             dsadded    = set(changes[1])
             dsremoved  = set(changes[2])
+            # store all local modification, useful later for renames detection
+            localchanges = dsmodified | dsadded
 
             # only take into account for removes between wc and target
             clean |= dsremoved - removed
             dsremoved &= removed
             # distinct between dirstate remove and other
@@ -2568,11 +2572,11 @@  def revert(ui, repo, ctx, parents, *pats
             dsadded |= mergeadd
             dsmodified -= mergeadd
 
         # if f is a rename, update `names` to also revert the source
         cwd = repo.getcwd()
-        for f in dsadded:
+        for f in localchanges:
             src = repo.dirstate.copied(f)
             # XXX should we check for rename down to target node?
             if src and src not in names and repo.dirstate[src] == 'r':
                 dsremoved.add(src)
                 names[src] = (repo.pathto(src, cwd), True)
diff --git a/tests/test-revert.t b/tests/test-revert.t
--- a/tests/test-revert.t
+++ b/tests/test-revert.t
@@ -232,10 +232,16 @@  reverting a rename target should revert 
   $ hg mv a newa
   $ hg revert newa
   $ hg st a newa
   ? newa
 
+Also true for move overwriting existing file
+
+  $ hg mv --force a b/b
+  $ hg revert b/b
+  $ hg status a b/b
+
   $ cd ..
 
   $ hg init ignored
   $ cd ignored
   $ echo '^ignored$' > .hgignore