Patchwork [STABLE] largefiles: revert to lfilesrepo.status() being an unfiltered method

login
register
mail settings
Submitter Matt Harbison
Date Jan. 27, 2015, 2:30 a.m.
Message ID <00a94f751ca9e81f79e2.1422325819@Envy>
Download mbox | patch
Permalink /patch/7559/
State Accepted
Commit df463ca0adefbf6ffa084cfd8031dcfc6c944f8e
Headers show

Comments

Matt Harbison - Jan. 27, 2015, 2:30 a.m.
# HG changeset patch
# User Matt Harbison <matt_harbison@yahoo.com>
# Date 1422244510 18000
#      Sun Jan 25 22:55:10 2015 -0500
# Branch stable
# Node ID 00a94f751ca9e81f79e2c35a1eb9e1019aa89af9
# Parent  0c4419faacbcca691737b5e25820dcbf4c7150ac
largefiles: revert to lfilesrepo.status() being an unfiltered method

This effectively reverts 67d63ec85eb7, which caused some normal file copies to
not be displayed as copies.  Other normal file copies could be displayed- the
exact reason isn't clear.  This also adds two tests that were failing prior to
this backout, so that this can be sorted out next cycle.

The difference between copy cases that worked and those that didn't seemed to be
in copies.pathcopies().  When largefiles isn't enabled for the changed test, or
lfstatus is not set in the commands.status() override, 'y.ancestor(x) == x'.
That wasn't true otherwise, which fell through to the _chain() method.  In this
case, the copy is removed in the criss cross loop.

'y.ancestor(x)' returns a context.changectx type, while 'x' is a lfilesctx type
in the failing case.  I tried adding the ancestor method to the lfilesctx class
to change the type of the ancestor context, however the context when printed as
a string then gains a '+'.  This points to it being a context.committablectx,
which clearly isn't correct for an ancestor.  Possibly the problem is the
lfilesctx needs to subclass context.committablectx in some cases, but
context.changectx in others, within the same invocation?  I'm not sure how to
pull that off, and backing out this change is safer during the freeze.

As to the status changing when a path is specified, I haven't looked into it
yet.
Matt Mackall - Jan. 27, 2015, 10:30 p.m.
On Mon, 2015-01-26 at 21:30 -0500, Matt Harbison wrote:
> # HG changeset patch
> # User Matt Harbison <matt_harbison@yahoo.com>
> # Date 1422244510 18000
> #      Sun Jan 25 22:55:10 2015 -0500
> # Branch stable
> # Node ID 00a94f751ca9e81f79e2c35a1eb9e1019aa89af9
> # Parent  0c4419faacbcca691737b5e25820dcbf4c7150ac
> largefiles: revert to lfilesrepo.status() being an unfiltered method

Queued for stable, thanks.

Patch

diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -12,7 +12,7 @@ 
 
 from mercurial import error, manifest, match as match_, util
 from mercurial.i18n import _
-from mercurial import scmutil
+from mercurial import scmutil, localrepo
 
 import lfcommands
 import lfutil
@@ -34,7 +34,7 @@ 
         # their actual contents.
         def __getitem__(self, changeid):
             ctx = super(lfilesrepo, self).__getitem__(changeid)
-            if self.unfiltered().lfstatus:
+            if self.lfstatus:
                 class lfilesmanifestdict(manifest.manifestdict):
                     def __contains__(self, filename):
                         orig = super(lfilesmanifestdict, self).__contains__
@@ -72,20 +72,19 @@ 
         # appropriate list in the result. Also removes standin files
         # from the listing. Revert to the original status if
         # self.lfstatus is False.
+        # XXX large file status is buggy when used on repo proxy.
+        # XXX this needs to be investigated.
+        @localrepo.unfilteredmethod
         def status(self, node1='.', node2=None, match=None, ignored=False,
                 clean=False, unknown=False, listsubrepos=False):
             listignored, listclean, listunknown = ignored, clean, unknown
             orig = super(lfilesrepo, self).status
-
-            # When various overrides set repo.lfstatus, the change is redirected
-            # to the unfiltered repo, and self.lfstatus is always false when
-            # this repo is filtered.
-            if not self.unfiltered().lfstatus:
+            if not self.lfstatus:
                 return orig(node1, node2, match, listignored, listclean,
                             listunknown, listsubrepos)
 
             # some calls in this function rely on the old version of status
-            self.unfiltered().lfstatus = False
+            self.lfstatus = False
             ctx1 = self[node1]
             ctx2 = self[node2]
             working = ctx2.rev() is None
@@ -241,7 +240,7 @@ 
                 if wlock:
                     wlock.release()
 
-            self.unfiltered().lfstatus = True
+            self.lfstatus = True
             return scmutil.status(*result)
 
         def commitctx(self, ctx, *args, **kwargs):
diff --git a/tests/test-copy.t b/tests/test-copy.t
--- a/tests/test-copy.t
+++ b/tests/test-copy.t
@@ -180,6 +180,13 @@ 
   $ hg st -C
   M bar
     foo
+
+XXX: filtering lfilesrepo.status() in 3.3-rc causes the copy source to not be
+displayed.
+  $ hg st -C --config extensions.largefiles=
+  M bar
+    foo
+
   $ hg commit -m3
 
 should show no parents for tip
diff --git a/tests/test-subrepo-recursion.t b/tests/test-subrepo-recursion.t
--- a/tests/test-subrepo-recursion.t
+++ b/tests/test-subrepo-recursion.t
@@ -162,6 +162,14 @@ 
   M ../foo/bar/z.txt
   M ../foo/y.txt
   ? a.txt
+
+XXX: filtering lfilesrepo.status() in 3.3-rc causes these files to be listed as
+added instead of modified.
+  $ hg status -S .. --config extensions.largefiles=
+  M ../foo/bar/z.txt
+  M ../foo/y.txt
+  ? a.txt
+
   $ hg diff --nodates -S ..
   diff -r d254738c5f5e foo/y.txt
   --- a/foo/y.txt