Patchwork D109: sparse: consolidate common code for writing sparse config

login
register
mail settings
Submitter phabricator
Date July 17, 2017, 7:15 p.m.
Message ID <differential-rev-PHID-DREV-mqfkmb4lraef5ouwhqdu-req@phab.mercurial-scm.org>
Download mbox | patch
Permalink /patch/22452/
State Not Applicable
Headers show

Comments

phabricator - July 17, 2017, 7:15 p.m.
indygreg created this revision.
Herald added a subscriber: mercurial-devel.
Herald added a reviewer: hg-reviewers.

REVISION SUMMARY
  In 3 functions we were writing the sparse config and updating the
  working directory. In two of them we had a transaction-like process
  for restoring the sparse config in case of wdir update fail.
  
  Because the pattern is common, we've already made mistakes, and the
  complexity will increase in the near future, let's consolidate the
  code into a reusable function.
  
  As part of this refactor, we end up reading the "sparse" file twice
  when updating it. This is a bit sub-optimal. But I don't think it
  is worth the code complexity to pass around the variables to avoid
  the redundancy.

REPOSITORY
  rHG Mercurial

REVISION DETAIL
  https://phab.mercurial-scm.org/D109

AFFECTED FILES
  mercurial/sparse.py

CHANGE DETAILS




EMAIL PREFERENCES
  https://phab.mercurial-scm.org/settings/panel/emailpreferences/

To: indygreg, #hg-reviewers
Cc: mercurial-devel

Patch

diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -521,6 +521,31 @@ 
 
     prunetemporaryincludes(repo)
 
+def _updateconfigandrefreshwdir(repo, includes, excludes, profiles,
+                                force=False):
+    """Update the sparse config and working directory state."""
+    raw = repo.vfs.tryread('sparse')
+    oldincludes, oldexcludes, oldprofiles = parseconfig(repo.ui, raw)
+
+    oldstatus = repo.status()
+    oldmatch = matcher(repo)
+
+    # TODO remove this try..except once the matcher integrates better
+    # with dirstate. We currently have to write the updated config
+    # because that will invalidate the matcher cache and force a
+    # re-read. We ideally want to update the cached matcher on the
+    # repo instance then flush the new config to disk once wdir is
+    # updated. But this requires massive rework to matcher() and its
+    # consumers.
+
+    writeconfig(repo, includes, excludes, profiles)
+
+    try:
+        return refreshwdir(repo, oldstatus, oldmatch, force=force)
+    except Exception:
+        writeconfig(repo, oldincludes, oldexcludes, oldprofiles)
+        raise
+
 def clearrules(repo, force=False):
     """Clears include/exclude rules from the sparse config.
 
@@ -534,10 +559,7 @@ 
         if not includes and not excludes:
             return
 
-        oldstatus = repo.status()
-        oldmatch = matcher(repo)
-        writeconfig(repo, set(), set(), profiles)
-        refreshwdir(repo, oldstatus, oldmatch, force=force)
+        _updateconfigandrefreshwdir(repo, set(), set(), profiles, force=force)
 
 def importfromfiles(repo, opts, paths, force=False):
     """Import sparse config rules from files.
@@ -548,10 +570,7 @@ 
     with repo.wlock():
         # read current configuration
         raw = repo.vfs.tryread('sparse')
-        oincludes, oexcludes, oprofiles = parseconfig(repo.ui, raw)
-        includes, excludes, profiles = map(
-                set, (oincludes, oexcludes, oprofiles))
-
+        includes, excludes, profiles = parseconfig(repo.ui, raw)
         aincludes, aexcludes, aprofiles = activeconfig(repo)
 
         # Import rules on top; only take in rules that are not yet
@@ -577,25 +596,8 @@ 
             includecount = len(includes - aincludes)
             excludecount = len(excludes - aexcludes)
 
-            oldstatus = repo.status()
-            oldsparsematch = matcher(repo)
-
-            # TODO remove this try..except once the matcher integrates better
-            # with dirstate. We currently have to write the updated config
-            # because that will invalidate the matcher cache and force a
-            # re-read. We ideally want to update the cached matcher on the
-            # repo instance then flush the new config to disk once wdir is
-            # updated. But this requires massive rework to matcher() and its
-            # consumers.
-            writeconfig(repo, includes, excludes, profiles)
-
-            try:
-                fcounts = map(
-                    len,
-                    refreshwdir(repo, oldstatus, oldsparsematch, force=force))
-            except Exception:
-                writeconfig(repo, oincludes, oexcludes, oprofiles)
-                raise
+            fcounts = map(len, _updateconfigandrefreshwdir(
+                repo, includes, excludes, profiles, force=force))
 
         printchanges(repo.ui, opts, profilecount, includecount, excludecount,
                      *fcounts)
@@ -610,8 +612,6 @@ 
     The new config is written out and a working directory refresh is performed.
     """
     with repo.wlock():
-        oldmatcher = matcher(repo)
-
         raw = repo.vfs.tryread('sparse')
         oldinclude, oldexclude, oldprofiles = parseconfig(repo.ui, raw)
 
@@ -624,8 +624,6 @@ 
             newexclude = set(oldexclude)
             newprofiles = set(oldprofiles)
 
-        oldstatus = repo.status()
-
         if any(pat.startswith('/') for pat in pats):
             repo.ui.warn(_('warning: paths cannot start with /, ignoring: %s\n')
                          % ([pat for pat in pats if pat.startswith('/')]))
@@ -648,20 +646,11 @@ 
         excludecount = (len(newexclude - oldexclude) -
                         len(oldexclude - newexclude))
 
-        # TODO clean up this writeconfig() + try..except pattern once we can.
-        # See comment in importfromfiles() explaining it.
-        writeconfig(repo, newinclude, newexclude, newprofiles)
+        fcounts = map(len, _updateconfigandrefreshwdir(
+            repo, newinclude, newexclude, newprofiles, force=force))
 
-        try:
-            fcounts = map(
-                len,
-                refreshwdir(repo, oldstatus, oldmatcher, force=force))
-
-            printchanges(repo.ui, opts, profilecount, includecount,
-                         excludecount, *fcounts)
-        except Exception:
-            writeconfig(repo, oldinclude, oldexclude, oldprofiles)
-            raise
+        printchanges(repo.ui, opts, profilecount, includecount,
+                     excludecount, *fcounts)
 
 def printchanges(ui, opts, profilecount=0, includecount=0, excludecount=0,
                  added=0, dropped=0, conflicting=0):