Patchwork [01,of,15,v2] dirstate: separate 'lookup' status field from others

login
register
mail settings
Submitter Martin von Zweigbergk
Date Oct. 5, 2014, 6:07 a.m.
Message ID <06cc871f960f4c22a81b.1412489278@handduk2.mtv.corp.google.com>
Download mbox | patch
Permalink /patch/6127/
State Superseded
Commit 509e2cbee679e0f7ef589928ddde4da99fb91135
Headers show

Comments

Martin von Zweigbergk - Oct. 5, 2014, 6:07 a.m.
# HG changeset patch
# User Martin von Zweigbergk <martinvonz@gmail.com>
# Date 1412397850 25200
#      Fri Oct 03 21:44:10 2014 -0700
# Node ID 06cc871f960f4c22a81b2271ef235ff53f26cf4c
# Parent  889789a2ca9f19755bf0d302c8671f850e42a059
dirstate: separate 'lookup' status field from others

The status tuple returned from dirstate.status() has an additional
field compared to the other status tuples: lookup/unsure. This field
is just an optimization and not something most callers care about
(they want the resolved value of 'modified' or 'clean'). To prepare
for a single future status type, let's separate out the 'lookup' field
from the rest by having dirstate.status() return a pair: (lookup,
status).
Mads Kiilerich - Oct. 6, 2014, 5:48 p.m.
On 10/05/2014 08:07 AM, Martin von Zweigbergk wrote:
> # HG changeset patch
> # User Martin von Zweigbergk <martinvonz@gmail.com>
> # Date 1412397850 25200
> #      Fri Oct 03 21:44:10 2014 -0700
> # Node ID 06cc871f960f4c22a81b2271ef235ff53f26cf4c
> # Parent  889789a2ca9f19755bf0d302c8671f850e42a059
> dirstate: separate 'lookup' status field from others
>
> The status tuple returned from dirstate.status() has an additional
> field compared to the other status tuples: lookup/unsure. This field
> is just an optimization and not something most callers care about
> (they want the resolved value of 'modified' or 'clean'). To prepare
> for a single future status type, let's separate out the 'lookup' field
> from the rest by having dirstate.status() return a pair: (lookup,
> status).
>
> diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
> --- a/hgext/largefiles/lfutil.py
> +++ b/hgext/largefiles/lfutil.py
> @@ -136,8 +136,8 @@
>   
>   def lfdirstatestatus(lfdirstate, repo, rev):
>       match = match_.always(repo.root, repo.getcwd())
> -    s = lfdirstate.status(match, [], False, False, False)
> -    unsure, modified, added, removed, missing, unknown, ignored, clean = s
> +    unsure, s = lfdirstate.status(match, [], False, False, False)
> +    modified, added, removed, missing, unknown, ignored, clean = s
>       for lfile in unsure:
>           try:
>               fctx = repo[rev][standin(lfile)]
> diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
> --- a/hgext/largefiles/overrides.py
> +++ b/hgext/largefiles/overrides.py
> @@ -351,9 +351,9 @@
>       wlock = repo.wlock()
>       try:
>           lfdirstate = lfutil.openlfdirstate(ui, repo)
> -        s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
> -            [], False, False, False)
> -        (unsure, modified, added, removed, missing, unknown, ignored, clean) = s
> +        unsure, s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
> +                                      [], False, False, False)
> +        modified = s[0]

Nice rework of the series!

This change must imply that the name bindings of 'modified' and the 
others actually were unused? It just reused an existing name so we 
didn't notice?

That is arguably a separate change that should come first (for instance 
renaming to _ prefix) ... or at least be mentioned in the description. 
That is however just a minor thing and I do not find it necessary to 
rework&resend just for that.

/Mads

Patch

diff --git a/hgext/largefiles/lfutil.py b/hgext/largefiles/lfutil.py
--- a/hgext/largefiles/lfutil.py
+++ b/hgext/largefiles/lfutil.py
@@ -136,8 +136,8 @@ 
 
 def lfdirstatestatus(lfdirstate, repo, rev):
     match = match_.always(repo.root, repo.getcwd())
-    s = lfdirstate.status(match, [], False, False, False)
-    unsure, modified, added, removed, missing, unknown, ignored, clean = s
+    unsure, s = lfdirstate.status(match, [], False, False, False)
+    modified, added, removed, missing, unknown, ignored, clean = s
     for lfile in unsure:
         try:
             fctx = repo[rev][standin(lfile)]
diff --git a/hgext/largefiles/overrides.py b/hgext/largefiles/overrides.py
--- a/hgext/largefiles/overrides.py
+++ b/hgext/largefiles/overrides.py
@@ -351,9 +351,9 @@ 
     wlock = repo.wlock()
     try:
         lfdirstate = lfutil.openlfdirstate(ui, repo)
-        s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
-            [], False, False, False)
-        (unsure, modified, added, removed, missing, unknown, ignored, clean) = s
+        unsure, s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
+                                      [], False, False, False)
+        modified = s[0]
 
         if opts['check']:
             mod = len(modified) > 0
@@ -1110,9 +1110,9 @@ 
         return orig(repo, pats, opts, dry_run, similarity)
     # Get the list of missing largefiles so we can remove them
     lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
-    s = lfdirstate.status(match_.always(repo.root, repo.getcwd()), [], False,
-        False, False)
-    (unsure, modified, added, removed, missing, unknown, ignored, clean) = s
+    unsure, s = lfdirstate.status(match_.always(repo.root, repo.getcwd()), [],
+                                  False, False, False)
+    missing = s[3]
 
     # Call into the normal remove code, but the removing of the standin, we want
     # to have handled by original addremove.  Monkey patching here makes sure
@@ -1288,9 +1288,10 @@ 
             # update standins for linear-merge or force-branch-merge,
             # because largefiles in the working directory may be modified
             lfdirstate = lfutil.openlfdirstate(repo.ui, repo)
-            s = lfdirstate.status(match_.always(repo.root, repo.getcwd()),
-                                  [], False, False, False)
-            unsure, modified, added = s[:3]
+            unsure, s = lfdirstate.status(match_.always(repo.root,
+                                                        repo.getcwd()),
+                                          [], False, False, False)
+            modified, added = s[:2]
             for lfile in unsure + modified + added:
                 lfutil.updatestandin(repo, lfutil.standin(lfile))
 
diff --git a/hgext/largefiles/reposetup.py b/hgext/largefiles/reposetup.py
--- a/hgext/largefiles/reposetup.py
+++ b/hgext/largefiles/reposetup.py
@@ -159,10 +159,10 @@ 
                                     if sfindirstate(f)]
                     # Don't waste time getting the ignored and unknown
                     # files from lfdirstate
-                    s = lfdirstate.status(match, [], False,
-                            listclean, False)
-                    (unsure, modified, added, removed, missing, _unknown,
-                            _ignored, clean) = s
+                    unsure, s = lfdirstate.status(match, [], False, listclean,
+                                                  False)
+                    (modified, added, removed, missing, _unknown, _ignored,
+                     clean) = s
                     if parentworking:
                         for lfile in unsure:
                             standin = lfutil.standin(lfile)
@@ -296,9 +296,9 @@ 
                     # large.
                     lfdirstate = lfutil.openlfdirstate(ui, self)
                     dirtymatch = match_.always(self.root, self.getcwd())
-                    s = lfdirstate.status(dirtymatch, [], False, False, False)
-                    (unsure, modified, added, removed, _missing, _unknown,
-                            _ignored, _clean) = s
+                    unsure, s = lfdirstate.status(dirtymatch, [], False, False,
+                                                  False)
+                    modified, added, removed = s[:3]
                     modifiedfiles = unsure + modified + added + removed
                     lfiles = lfutil.listlfiles(self)
                     # this only loops through largefiles that exist (not
diff --git a/mercurial/context.py b/mercurial/context.py
--- a/mercurial/context.py
+++ b/mercurial/context.py
@@ -1418,9 +1418,9 @@ 
         subrepos = []
         if '.hgsub' in self:
             subrepos = sorted(self.substate)
-        s = self._repo.dirstate.status(match, subrepos, listignored,
-                                       listclean, listunknown)
-        cmp, modified, added, removed, deleted, unknown, ignored, clean = s
+        cmp, s = self._repo.dirstate.status(match, subrepos, listignored,
+                                            listclean, listunknown)
+        modified, added, removed, deleted, unknown, ignored, clean = s
 
         # check for any possibly clean files
         if cmp:
diff --git a/mercurial/dirstate.py b/mercurial/dirstate.py
--- a/mercurial/dirstate.py
+++ b/mercurial/dirstate.py
@@ -801,8 +801,8 @@ 
 
     def status(self, match, subrepos, ignored, clean, unknown):
         '''Determine the status of the working copy relative to the
-        dirstate and return a tuple of lists (unsure, modified, added,
-        removed, deleted, unknown, ignored, clean), where:
+        dirstate and return a nested tuple of lists (unsure, (modified, added,
+        removed, deleted, unknown, ignored, clean)), where:
 
           unsure:
             files that might have been modified since the dirstate was
@@ -900,8 +900,8 @@ 
             elif state == 'r':
                 radd(fn)
 
-        return (lookup, modified, added, removed, deleted, unknown, ignored,
-                clean)
+        return (lookup, (modified, added, removed, deleted, unknown, ignored,
+                         clean))
 
     def matches(self, match):
         '''