Patchwork [stable] largefiles: fix 'deleted' files sometimes persistently appearing with R status

login
register
mail settings
Submitter Mads Kiilerich
Date Oct. 17, 2016, 3:12 p.m.
Message ID <bba9f6bba98f82f2403a.1476717152@madski>
Download mbox | patch
Permalink /patch/17153/
State Accepted
Headers show

Comments

Mads Kiilerich - Oct. 17, 2016, 3:12 p.m.
# HG changeset patch
# User Mads Kiilerich <madski@unity3d.com>
# Date 1476717144 -7200
#      Mon Oct 17 17:12:24 2016 +0200
# Node ID bba9f6bba98f82f2403aac8dc656569562690472
# Parent  92414d57a05de39864aecc625e775c693d9edb51
largefiles: fix 'deleted' files sometimes persistently appearing with R status

A code snippet that has been around since largefiles was introduced was wrong:
Standins no longer found in lfdirstate has *not* been removed -
they have probably just been deleted ... or not created.

This wrong reporting did that 'up -C' didn't undo the change and didn't sync
the two dirstates.

Instead of reporting such files as removed, propagate the deletion to the
standin file and report the file as deleted.
Mads Kiilerich - Oct. 17, 2016, 4:37 p.m.
On 10/17/2016 05:12 PM, Mads Kiilerich wrote:
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1476717144 -7200
> #      Mon Oct 17 17:12:24 2016 +0200
> # Node ID bba9f6bba98f82f2403aac8dc656569562690472
> # Parent  92414d57a05de39864aecc625e775c693d9edb51
> largefiles: fix 'deleted' files sometimes persistently appearing with R status
>
> A code snippet that has been around since largefiles was introduced was wrong:
> Standins no longer found in lfdirstate has *not* been removed -
> they have probably just been deleted ... or not created.
>
> This wrong reporting did that 'up -C' didn't undo the change and didn't sync
> the two dirstates.
>
> Instead of reporting such files as removed, propagate the deletion to the
> standin file and report the file as deleted.
>
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -164,8 +164,8 @@ def reposetup(ui, repo):
>                       # files from lfdirstate
>                       unsure, s = lfdirstate.status(match, [], False, listclean,
>                                                     False)
> -                    (modified, added, removed, clean) = (s.modified, s.added,
> -                                                         s.removed, s.clean)
> +                    (modified, added, removed, deleted, clean) = (
> +                        s.modified, s.added, s.removed, s.deleted, s.clean)
>                       if parentworking:
>                           for lfile in unsure:
>                               standin = lfutil.standin(lfile)
> @@ -206,14 +206,18 @@ def reposetup(ui, repo):
>                           removed = [lfile for lfile in removed
>                                      if lfutil.standin(lfile) in ctx1]
>   
> -                    # Standins no longer found in lfdirstate has been
> -                    # removed
> +                    # Standins no longer found in lfdirstate have been deleted
>                       for standin in ctx1.walk(lfutil.getstandinmatcher(self)):
>                           lfile = lfutil.splitstandin(standin)
>                           if not match(lfile):
>                               continue
>                           if lfile not in lfdirstate:
> -                            removed.append(lfile)
> +                            deleted.append(lfile)
> +                            # Sync "largefile has been removed" back to the
Hmm ... for clarity, this should have been "deleted"

/Mads
Augie Fackler - Oct. 17, 2016, 5:23 p.m.
Queued this, thanks

> On Oct 17, 2016, at 11:12, Mads Kiilerich <mads@kiilerich.com> wrote:
> 
> # HG changeset patch
> # User Mads Kiilerich <madski@unity3d.com>
> # Date 1476717144 -7200
> #      Mon Oct 17 17:12:24 2016 +0200
> # Node ID bba9f6bba98f82f2403aac8dc656569562690472
> # Parent  92414d57a05de39864aecc625e775c693d9edb51
> largefiles: fix 'deleted' files sometimes persistently appearing with R status
> 
> A code snippet that has been around since largefiles was introduced was wrong:
> Standins no longer found in lfdirstate has *not* been removed -
> they have probably just been deleted ... or not created.
> 
> This wrong reporting did that 'up -C' didn't undo the change and didn't sync
> the two dirstates.
> 
> Instead of reporting such files as removed, propagate the deletion to the
> standin file and report the file as deleted.
> 
> diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
> --- a/hgext/largefiles/reposetup.py
> +++ b/hgext/largefiles/reposetup.py
> @@ -164,8 +164,8 @@ def reposetup(ui, repo):
>                     # files from lfdirstate
>                     unsure, s = lfdirstate.status(match, [], False, listclean,
>                                                   False)
> -                    (modified, added, removed, clean) = (s.modified, s.added,
> -                                                         s.removed, s.clean)
> +                    (modified, added, removed, deleted, clean) = (
> +                        s.modified, s.added, s.removed, s.deleted, s.clean)
>                     if parentworking:
>                         for lfile in unsure:
>                             standin = lfutil.standin(lfile)
> @@ -206,14 +206,18 @@ def reposetup(ui, repo):
>                         removed = [lfile for lfile in removed
>                                    if lfutil.standin(lfile) in ctx1]
> 
> -                    # Standins no longer found in lfdirstate has been
> -                    # removed
> +                    # Standins no longer found in lfdirstate have been deleted
>                     for standin in ctx1.walk(lfutil.getstandinmatcher(self)):
>                         lfile = lfutil.splitstandin(standin)
>                         if not match(lfile):
>                             continue
>                         if lfile not in lfdirstate:
> -                            removed.append(lfile)
> +                            deleted.append(lfile)
> +                            # Sync "largefile has been removed" back to the
> +                            # standin. Removing a file as a side effect of
> +                            # running status is gross, but the alternatives (if
> +                            # any) are worse.
> +                            self.wvfs.unlink(standin)
> 
>                     # Filter result lists
>                     result = list(result)
> @@ -237,7 +241,7 @@ def reposetup(ui, repo):
>                     normals = [[fn for fn in filelist
>                                 if not lfutil.isstandin(fn)]
>                                for filelist in result]
> -                    lfstatus = (modified, added, removed, s.deleted, [], [],
> +                    lfstatus = (modified, added, removed, deleted, [], [],
>                                 clean)
>                     result = [sorted(list1 + list2)
>                               for (list1, list2) in zip(normals, lfstatus)]
> diff --git a/tests/test-largefiles-update.t b/tests/test-largefiles-update.t
> --- a/tests/test-largefiles-update.t
> +++ b/tests/test-largefiles-update.t
> @@ -751,30 +751,22 @@ added:
>   $ hg up -Cr "8" --config extensions.crashupdatelfiles=../crashupdatelfiles.py
>   [7]
> Check large1 content and status ... and that update will undo modifications:
> -BUG: large is R
>   $ cat large1
>   large1 in #3
>   $ hg st
>   M large1
> -  R largeX
> +  ! largeX
>   $ hg up -Cr .
>   getting changed largefiles
> -  1 largefiles updated, 0 removed
> -  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
> +  2 largefiles updated, 0 removed
> +  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
>   $ cat large1
>   manually modified before 'hg transplant --continue'
>   $ hg st
> -  R largeX
> -Force largefiles rehashing and check again - which makes it realize that largeX
> -not has been removed but just doesn't exist:
> +Force largefiles rehashing and check that all changes have been caught by
> +status and update:
>   $ rm .hg/largefiles/dirstate
>   $ hg st
> -  ! largeX
> -  $ hg up -Cr .
> -  getting changed largefiles
> -  1 largefiles updated, 0 removed
> -  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
> -  $ hg st
> 
>   $ cd ..
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Patch

diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -164,8 +164,8 @@  def reposetup(ui, repo):
                     # files from lfdirstate
                     unsure, s = lfdirstate.status(match, [], False, listclean,
                                                   False)
-                    (modified, added, removed, clean) = (s.modified, s.added,
-                                                         s.removed, s.clean)
+                    (modified, added, removed, deleted, clean) = (
+                        s.modified, s.added, s.removed, s.deleted, s.clean)
                     if parentworking:
                         for lfile in unsure:
                             standin = lfutil.standin(lfile)
@@ -206,14 +206,18 @@  def reposetup(ui, repo):
                         removed = [lfile for lfile in removed
                                    if lfutil.standin(lfile) in ctx1]
 
-                    # Standins no longer found in lfdirstate has been
-                    # removed
+                    # Standins no longer found in lfdirstate have been deleted
                     for standin in ctx1.walk(lfutil.getstandinmatcher(self)):
                         lfile = lfutil.splitstandin(standin)
                         if not match(lfile):
                             continue
                         if lfile not in lfdirstate:
-                            removed.append(lfile)
+                            deleted.append(lfile)
+                            # Sync "largefile has been removed" back to the
+                            # standin. Removing a file as a side effect of
+                            # running status is gross, but the alternatives (if
+                            # any) are worse.
+                            self.wvfs.unlink(standin)
 
                     # Filter result lists
                     result = list(result)
@@ -237,7 +241,7 @@  def reposetup(ui, repo):
                     normals = [[fn for fn in filelist
                                 if not lfutil.isstandin(fn)]
                                for filelist in result]
-                    lfstatus = (modified, added, removed, s.deleted, [], [],
+                    lfstatus = (modified, added, removed, deleted, [], [],
                                 clean)
                     result = [sorted(list1 + list2)
                               for (list1, list2) in zip(normals, lfstatus)]
diff --git a/tests/test-largefiles-update.t b/tests/test-largefiles-update.t
--- a/tests/test-largefiles-update.t
+++ b/tests/test-largefiles-update.t
@@ -751,30 +751,22 @@  added:
   $ hg up -Cr "8" --config extensions.crashupdatelfiles=../crashupdatelfiles.py
   [7]
 Check large1 content and status ... and that update will undo modifications:
-BUG: large is R
   $ cat large1
   large1 in #3
   $ hg st
   M large1
-  R largeX
+  ! largeX
   $ hg up -Cr .
   getting changed largefiles
-  1 largefiles updated, 0 removed
-  1 files updated, 0 files merged, 0 files removed, 0 files unresolved
+  2 largefiles updated, 0 removed
+  2 files updated, 0 files merged, 0 files removed, 0 files unresolved
   $ cat large1
   manually modified before 'hg transplant --continue'
   $ hg st
-  R largeX
-Force largefiles rehashing and check again - which makes it realize that largeX
-not has been removed but just doesn't exist:
+Force largefiles rehashing and check that all changes have been caught by
+status and update:
   $ rm .hg/largefiles/dirstate
   $ hg st
-  ! largeX
-  $ hg up -Cr .
-  getting changed largefiles
-  1 largefiles updated, 0 removed
-  0 files updated, 0 files merged, 0 files removed, 0 files unresolved
-  $ hg st
 
   $ cd ..