Patchwork [06,of,11,sparse] sparse: move config updating function into core

login
register
mail settings
Submitter Gregory Szorc
Date July 8, 2017, 11:29 p.m.
Message ID <bafbabb23d7c584a0744.1499556541@ubuntu-vm-main>
Download mbox | patch
Permalink /patch/22131/
State Accepted
Headers show

Comments

Gregory Szorc - July 8, 2017, 11:29 p.m.
# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1499548773 25200
#      Sat Jul 08 14:19:33 2017 -0700
# Node ID bafbabb23d7c584a0744e0bf6426d72649f14453
# Parent  bdba7b6ce456921675ebf2dd54efd567709ed446
sparse: move config updating function into core

As part of the move, the code has changed slightly:

* Now using a context manager for wlock
* We always call parseconfig() because parsing an empty string
  will yield empty data structures
* The variable for the old sparse matcher was renamed
* The try..except block was shortened to only cover refreshdir().
  This is what importfromfiles() does and I think it is more
  appropriate.
via Mercurial-devel - July 10, 2017, 10:16 p.m.
On Sat, Jul 8, 2017 at 4:29 PM, Gregory Szorc <gregory.szorc@gmail.com> wrote:
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1499548773 25200
> #      Sat Jul 08 14:19:33 2017 -0700
> # Node ID bafbabb23d7c584a0744e0bf6426d72649f14453
> # Parent  bdba7b6ce456921675ebf2dd54efd567709ed446
> sparse: move config updating function into core
>
> As part of the move, the code has changed slightly:
>
> * Now using a context manager for wlock
> * We always call parseconfig() because parsing an empty string
>   will yield empty data structures
> * The variable for the old sparse matcher was renamed
> * The try..except block was shortened to only cover refreshdir().
>   This is what importfromfiles() does and I think it is more
>   appropriate.

I'm sorry, but do you mind splitting the move and the refactoring up
for this patch too? This one changed a little too much for it to be
easy to spot the differences by quickly switching back and forth.

I'm queuing patch 4 and 5, thanks.

Patch

diff --git a/hgext/sparse.py b/hgext/sparse.py
--- a/hgext/sparse.py
+++ b/hgext/sparse.py
@@ -163,8 +163,8 @@  def _clonesparsecmd(orig, ui, repo, *arg
         raise error.Abort(_("too many flags specified."))
     if include or exclude or enableprofile:
         def clonesparse(orig, self, node, overwrite, *args, **kwargs):
-            _config(self.ui, self.unfiltered(), pat, {}, include=include,
-                    exclude=exclude, enableprofile=enableprofile)
+            sparse.updateconfig(self.unfiltered(), {}, pat, include=include,
+                                exclude=exclude, enableprofile=enableprofile)
             return orig(self, node, overwrite, *args, **kwargs)
         extensions.wrapfunction(hg, 'updaterepo', clonesparse)
     return orig(ui, repo, *args, **opts)
@@ -190,7 +190,7 @@  def _setupadd(ui):
             for pat in pats:
                 dirname, basename = util.split(pat)
                 dirs.add(dirname)
-            _config(ui, repo, list(dirs), opts, include=True)
+            sparse.updateconfig(repo, opts, list(dirs), include=True)
         return orig(ui, repo, *pats, **opts)
 
     extensions.wrapcommand(commands.table, 'add', _add)
@@ -356,9 +356,10 @@  def debugsparse(ui, repo, *pats, **opts)
         return
 
     if include or exclude or delete or reset or enableprofile or disableprofile:
-        _config(ui, repo, pats, opts, include=include, exclude=exclude,
-                reset=reset, delete=delete, enableprofile=enableprofile,
-                disableprofile=disableprofile, force=force)
+        sparse.updateconfig(repo, opts, pats, include=include, exclude=exclude,
+                            reset=reset, delete=delete,
+                            enableprofile=enableprofile,
+                            disableprofile=disableprofile, force=force)
 
     if importrules:
         sparse.importfromfiles(repo, opts, pats, force=force)
@@ -377,70 +378,3 @@  def debugsparse(ui, repo, *pats, **opts)
                                 conflicting=fcounts[2])
         finally:
             wlock.release()
-
-def _config(ui, repo, pats, opts, include=False, exclude=False, reset=False,
-            delete=False, enableprofile=False, disableprofile=False,
-            force=False):
-    """
-    Perform a sparse config update. Only one of the kwargs may be specified.
-    """
-    wlock = repo.wlock()
-    try:
-        oldsparsematch = sparse.matcher(repo)
-
-        raw = repo.vfs.tryread('sparse')
-        if raw:
-            oldinclude, oldexclude, oldprofiles = map(
-                set, sparse.parseconfig(ui, raw))
-        else:
-            oldinclude = set()
-            oldexclude = set()
-            oldprofiles = set()
-
-        try:
-            if reset:
-                newinclude = set()
-                newexclude = set()
-                newprofiles = set()
-            else:
-                newinclude = set(oldinclude)
-                newexclude = set(oldexclude)
-                newprofiles = set(oldprofiles)
-
-            oldstatus = repo.status()
-
-            if any(pat.startswith('/') for pat in pats):
-                ui.warn(_('warning: paths cannot start with /, ignoring: %s\n')
-                          % ([pat for pat in pats if pat.startswith('/')]))
-            elif include:
-                newinclude.update(pats)
-            elif exclude:
-                newexclude.update(pats)
-            elif enableprofile:
-                newprofiles.update(pats)
-            elif disableprofile:
-                newprofiles.difference_update(pats)
-            elif delete:
-                newinclude.difference_update(pats)
-                newexclude.difference_update(pats)
-
-            sparse.writeconfig(repo, newinclude, newexclude, newprofiles)
-
-            fcounts = map(
-                len,
-                sparse.refreshwdir(repo, oldstatus, oldsparsematch,
-                                   force=force))
-
-            profilecount = (len(newprofiles - oldprofiles) -
-                            len(oldprofiles - newprofiles))
-            includecount = (len(newinclude - oldinclude) -
-                            len(oldinclude - newinclude))
-            excludecount = (len(newexclude - oldexclude) -
-                            len(oldexclude - newexclude))
-            sparse.printchanges(ui, opts, profilecount, includecount,
-                                excludecount, *fcounts)
-        except Exception:
-            sparse.writeconfig(repo, oldinclude, oldexclude, oldprofiles)
-            raise
-    finally:
-        wlock.release()
diff --git a/mercurial/sparse.py b/mercurial/sparse.py
--- a/mercurial/sparse.py
+++ b/mercurial/sparse.py
@@ -586,6 +586,69 @@  def importfromfiles(repo, opts, paths, f
         printchanges(repo.ui, opts, profilecount, includecount, excludecount,
                      *fcounts)
 
+def updateconfig(repo, opts, pats, include=False, exclude=False, reset=False,
+                 delete=False, enableprofile=False, disableprofile=False,
+                 force=False):
+    """Perform a sparse config update.
+
+    Only one of the actions may be performed.
+
+    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)
+        oldprofiles = set(oldprofiles)
+
+        if reset:
+            newinclude = set()
+            newexclude = set()
+            newprofiles = set()
+        else:
+            newinclude = set(oldinclude)
+            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('/')]))
+        elif include:
+            newinclude.update(pats)
+        elif exclude:
+            newexclude.update(pats)
+        elif enableprofile:
+            newprofiles.update(pats)
+        elif disableprofile:
+            newprofiles.difference_update(pats)
+        elif delete:
+            newinclude.difference_update(pats)
+            newexclude.difference_update(pats)
+
+        profilecount = (len(newprofiles - oldprofiles) -
+                        len(oldprofiles - newprofiles))
+        includecount = (len(newinclude - oldinclude) -
+                        len(oldinclude - newinclude))
+        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)
+
+        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
+
 def printchanges(ui, opts, profilecount=0, includecount=0, excludecount=0,
                  added=0, dropped=0, conflicting=0):
     """Print output summarizing sparse config changes."""