Patchwork [stable] addremove: add back forgotten files

login
register
mail settings
Submitter Martin von Zweigbergk
Date Nov. 9, 2014, 8:43 a.m.
Message ID <abe330fc28e4b612e0ed.1415522638@martinvonz.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/6655/
State Superseded
Headers show

Comments

Martin von Zweigbergk - Nov. 9, 2014, 8:43 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@google.com>
# Date 1415517219 28800
#      Sat Nov 08 23:13:39 2014 -0800
# Node ID abe330fc28e4b612e0ed2be5fc3cdaa378396ddd
# Parent  2d54aa5397cdb1c697673ba10b7618d5ac25c69e
addremove: add back forgotten files

After running "hg forget README && hg addremove", README will still be
reported as removed. This is because scmutil._interestingfiles()
reports the file as removed, so scmutil.addremove() does not add
it.

Teach _interestingfiles() to report forgotten files separately from
removed files and make addremove() add forgotten files back. However,
do not treat forgotten files as sources for rename detection. Note
that since removed and forgotten files are treated the same before
this change, forgotten files were considered sources for rename
detection.

Also update the other caller, marktouched(), in the same way as
addremove().
Augie Fackler - Nov. 11, 2014, 2:24 p.m.
On Sun, Nov 09, 2014 at 12:43:58AM -0800, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@google.com>
> # Date 1415517219 28800
> #      Sat Nov 08 23:13:39 2014 -0800
> # Node ID abe330fc28e4b612e0ed2be5fc3cdaa378396ddd
> # Parent  2d54aa5397cdb1c697673ba10b7618d5ac25c69e
> addremove: add back forgotten files

I've dithered on this, but it also seems reasonable. Does this go
hand-in-hand with your add patch that you sent yesterday?

>
> After running "hg forget README && hg addremove", README will still be
> reported as removed. This is because scmutil._interestingfiles()
> reports the file as removed, so scmutil.addremove() does not add
> it.
>
> Teach _interestingfiles() to report forgotten files separately from
> removed files and make addremove() add forgotten files back. However,
> do not treat forgotten files as sources for rename detection. Note
> that since removed and forgotten files are treated the same before
> this change, forgotten files were considered sources for rename
> detection.
>
> Also update the other caller, marktouched(), in the same way as
> addremove().
>
> diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
> --- a/mercurial/scmutil.py
> +++ b/mercurial/scmutil.py
> @@ -686,9 +686,9 @@
>      rejected = []
>      m.bad = lambda x, y: rejected.append(x)
>
> -    added, unknown, deleted, removed = _interestingfiles(repo, m)
> +    added, unknown, deleted, removed, forgotten = _interestingfiles(repo, m)
>
> -    unknownset = set(unknown)
> +    unknownset = set(unknown + forgotten)
>      toprint = unknownset.copy()
>      toprint.update(deleted)
>      for abs in sorted(toprint):
> @@ -704,7 +704,7 @@
>                             similarity)
>
>      if not dry_run:
> -        _markchanges(repo, unknown, deleted, renames)
> +        _markchanges(repo, unknown + forgotten, deleted, renames)
>
>      for f in rejected:
>          if f in m.files():
> @@ -718,10 +718,10 @@
>      rejected = []
>      m.bad = lambda x, y: rejected.append(x)
>
> -    added, unknown, deleted, removed = _interestingfiles(repo, m)
> +    added, unknown, deleted, removed, forgotten = _interestingfiles(repo, m)
>
>      if repo.ui.verbose:
> -        unknownset = set(unknown)
> +        unknownset = set(unknown + forgotten)
>          toprint = unknownset.copy()
>          toprint.update(deleted)
>          for abs in sorted(toprint):
> @@ -734,7 +734,7 @@
>      renames = _findrenames(repo, m, added + unknown, removed + deleted,
>                             similarity)
>
> -    _markchanges(repo, unknown, deleted, renames)
> +    _markchanges(repo, unknown + forgotten, deleted, renames)
>
>      for f in rejected:
>          if f in m.files():
> @@ -747,7 +747,7 @@
>
>      This is different from dirstate.status because it doesn't care about
>      whether files are modified or clean.'''
> -    added, unknown, deleted, removed = [], [], [], []
> +    added, unknown, deleted, removed, forgotten = [], [], [], [], []
>      audit_path = pathutil.pathauditor(repo.root)
>
>      ctx = repo[None]
> @@ -760,13 +760,15 @@
>              unknown.append(abs)
>          elif dstate != 'r' and not st:
>              deleted.append(abs)
> +        elif dstate == 'r' and st:
> +            forgotten.append(abs)
>          # for finding renames
> -        elif dstate == 'r':
> +        elif dstate == 'r' and not st:
>              removed.append(abs)
>          elif dstate == 'a':
>              added.append(abs)
>
> -    return added, unknown, deleted, removed
> +    return added, unknown, deleted, removed, forgotten
>
>  def _findrenames(repo, matcher, added, removed, similarity):
>      '''Find renames from removed files to added ones.'''
> diff --git a/tests/test-addremove.t b/tests/test-addremove.t
> --- a/tests/test-addremove.t
> +++ b/tests/test-addremove.t
> @@ -18,7 +18,11 @@
>    dir/bar_2
>    foo_2
>    committed changeset 1:e65414bf35c5
> -  $ cd ../..
> +  $ cd ..
> +  $ hg forget foo
> +  $ hg -v addremove
> +  adding foo
> +  $ cd ..
>
>    $ hg init sim
>    $ cd sim
> @@ -45,4 +49,9 @@
>    adding d
>    recording removal of a as rename to b (100% similar)
>    $ hg commit -mb
> +  $ cp b c
> +  $ hg forget b
> +  $ hg addremove -s 50
> +  adding b
> +  adding c
>    $ cd ..
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel
Martin von Zweigbergk - Nov. 11, 2014, 5:16 p.m.
On Tue Nov 11 2014 at 6:24:15 AM Augie Fackler <raf@durin42.com> wrote:

> On Sun, Nov 09, 2014 at 12:43:58AM -0800, Martin von Zweigbergk wrote:
> > # HG changeset patch
> > # User Martin von Zweigbergk <martinvonz@google.com>
> > # Date 1415517219 28800
> > #      Sat Nov 08 23:13:39 2014 -0800
> > # Node ID abe330fc28e4b612e0ed2be5fc3cdaa378396ddd
> > # Parent  2d54aa5397cdb1c697673ba10b7618d5ac25c69e
> > addremove: add back forgotten files
>
> I've dithered on this, but it also seems reasonable. Does this go
> hand-in-hand with your add patch that you sent yesterday?
>

I didn't know I was going to write the other patch when I wrote this one,
but it seems clear to me that the other one is correct. Regardless of that
other patch, it does seem like "hg add $file" and "hg addremove $file"
should behave the same if the file exists on disk. It also seems like the
point of 'hg addremove [.]' is to make "hg commit" not leave files behind.
Should I resend with a bit more of this thinking in the description?
Augie Fackler - Nov. 11, 2014, 5:17 p.m.
On Tue, Nov 11, 2014 at 12:16 PM, Martin von Zweigbergk
<martinvonz@google.com> wrote:
>> > # Node ID abe330fc28e4b612e0ed2be5fc3cdaa378396ddd
>> > # Parent  2d54aa5397cdb1c697673ba10b7618d5ac25c69e
>> > addremove: add back forgotten files
>>
>> I've dithered on this, but it also seems reasonable. Does this go
>> hand-in-hand with your add patch that you sent yesterday?
>
>
> I didn't know I was going to write the other patch when I wrote this one,
> but it seems clear to me that the other one is correct. Regardless of that
> other patch, it does seem like "hg add $file" and "hg addremove $file"
> should behave the same if the file exists on disk. It also seems like the
> point of 'hg addremove [.]' is to make "hg commit" not leave files behind.
> Should I resend with a bit more of this thinking in the description?


Yes, I think that would help your reviewers immensely. Maybe send both
patches as a series of 2?
Martin von Zweigbergk - Nov. 11, 2014, 5:29 p.m.
On Tue Nov 11 2014 at 9:17:55 AM Augie Fackler <raf@durin42.com> wrote:

> On Tue, Nov 11, 2014 at 12:16 PM, Martin von Zweigbergk
> <martinvonz@google.com> wrote:
> >> > # Node ID abe330fc28e4b612e0ed2be5fc3cdaa378396ddd
> >> > # Parent  2d54aa5397cdb1c697673ba10b7618d5ac25c69e
> >> > addremove: add back forgotten files
> >>
> >> I've dithered on this, but it also seems reasonable. Does this go
> >> hand-in-hand with your add patch that you sent yesterday?
> >
> >
> > I didn't know I was going to write the other patch when I wrote this one,
> > but it seems clear to me that the other one is correct. Regardless of
> that
> > other patch, it does seem like "hg add $file" and "hg addremove $file"
> > should behave the same if the file exists on disk. It also seems like the
> > point of 'hg addremove [.]' is to make "hg commit" not leave files
> behind.
> > Should I resend with a bit more of this thinking in the description?
>
>
> Yes, I think that would help your reviewers immensely. Maybe send both
> patches as a series of 2?
>

Sounds good. Will do.

Patch

diff --git a/mercurial/scmutil.py b/mercurial/scmutil.py
--- a/mercurial/scmutil.py
+++ b/mercurial/scmutil.py
@@ -686,9 +686,9 @@ 
     rejected = []
     m.bad = lambda x, y: rejected.append(x)
 
-    added, unknown, deleted, removed = _interestingfiles(repo, m)
+    added, unknown, deleted, removed, forgotten = _interestingfiles(repo, m)
 
-    unknownset = set(unknown)
+    unknownset = set(unknown + forgotten)
     toprint = unknownset.copy()
     toprint.update(deleted)
     for abs in sorted(toprint):
@@ -704,7 +704,7 @@ 
                            similarity)
 
     if not dry_run:
-        _markchanges(repo, unknown, deleted, renames)
+        _markchanges(repo, unknown + forgotten, deleted, renames)
 
     for f in rejected:
         if f in m.files():
@@ -718,10 +718,10 @@ 
     rejected = []
     m.bad = lambda x, y: rejected.append(x)
 
-    added, unknown, deleted, removed = _interestingfiles(repo, m)
+    added, unknown, deleted, removed, forgotten = _interestingfiles(repo, m)
 
     if repo.ui.verbose:
-        unknownset = set(unknown)
+        unknownset = set(unknown + forgotten)
         toprint = unknownset.copy()
         toprint.update(deleted)
         for abs in sorted(toprint):
@@ -734,7 +734,7 @@ 
     renames = _findrenames(repo, m, added + unknown, removed + deleted,
                            similarity)
 
-    _markchanges(repo, unknown, deleted, renames)
+    _markchanges(repo, unknown + forgotten, deleted, renames)
 
     for f in rejected:
         if f in m.files():
@@ -747,7 +747,7 @@ 
 
     This is different from dirstate.status because it doesn't care about
     whether files are modified or clean.'''
-    added, unknown, deleted, removed = [], [], [], []
+    added, unknown, deleted, removed, forgotten = [], [], [], [], []
     audit_path = pathutil.pathauditor(repo.root)
 
     ctx = repo[None]
@@ -760,13 +760,15 @@ 
             unknown.append(abs)
         elif dstate != 'r' and not st:
             deleted.append(abs)
+        elif dstate == 'r' and st:
+            forgotten.append(abs)
         # for finding renames
-        elif dstate == 'r':
+        elif dstate == 'r' and not st:
             removed.append(abs)
         elif dstate == 'a':
             added.append(abs)
 
-    return added, unknown, deleted, removed
+    return added, unknown, deleted, removed, forgotten
 
 def _findrenames(repo, matcher, added, removed, similarity):
     '''Find renames from removed files to added ones.'''
diff --git a/tests/test-addremove.t b/tests/test-addremove.t
--- a/tests/test-addremove.t
+++ b/tests/test-addremove.t
@@ -18,7 +18,11 @@ 
   dir/bar_2
   foo_2
   committed changeset 1:e65414bf35c5
-  $ cd ../..
+  $ cd ..
+  $ hg forget foo
+  $ hg -v addremove
+  adding foo
+  $ cd ..
 
   $ hg init sim
   $ cd sim
@@ -45,4 +49,9 @@ 
   adding d
   recording removal of a as rename to b (100% similar)
   $ hg commit -mb
+  $ cp b c
+  $ hg forget b
+  $ hg addremove -s 50
+  adding b
+  adding c
   $ cd ..