Patchwork [09,of,13,sparse] merge: move sparse config parsing from sparse extension

login
register
mail settings
Submitter via Mercurial-devel
Date July 2, 2017, 4:33 a.m.
Message ID <CAESOdVBXehMRhJRedy1RWwOZ-v=wt-3SBFdqjogizU__GcNeJg@mail.gmail.com>
Download mbox | patch
Permalink /patch/21912/
State Not Applicable
Headers show

Comments

via Mercurial-devel - July 2, 2017, 4:33 a.m.
On Jul 1, 2017 6:55 PM, "Gregory Szorc" <gregory.szorc@gmail.com> wrote:

# HG changeset patch
# User Gregory Szorc <gregory.szorc@gmail.com>
# Date 1498945185 25200
#      Sat Jul 01 14:39:45 2017 -0700
# Node ID a2d07300bdc999403954e965debf53db898ccdc3
# Parent  6636a2dfa31cb84684373bf3a9fdb03c26e42f99
merge: move sparse config parsing from sparse extension

This function is generic and doesn't need to be a repo method.

This patch also marks the beginning of moving code from sparse.py
into core. The goal is for the extension to enable the feature and
provide any UI adjustments.

merge.py may seem like a weird destination. But that's
where code for working directory updating lives. Unless we want
to create a new module for just the sparse bits or something for
working directory code, merge.py is the best current place.

+                ui.warn(_('warning: sparse profile cannot use' +
+                          ' paths starting with /, ignoring %s\n') % line)
+                continue
+            current.add(line)
+
+    return includes, excludes, profiles
Gregory Szorc - July 2, 2017, 5:36 a.m.
On Sat, Jul 1, 2017 at 9:33 PM, Martin von Zweigbergk <martinvonz@google.com
> wrote:

>
>
> On Jul 1, 2017 6:55 PM, "Gregory Szorc" <gregory.szorc@gmail.com> wrote:
>
> # HG changeset patch
> # User Gregory Szorc <gregory.szorc@gmail.com>
> # Date 1498945185 25200
> #      Sat Jul 01 14:39:45 2017 -0700
> # Node ID a2d07300bdc999403954e965debf53db898ccdc3
> # Parent  6636a2dfa31cb84684373bf3a9fdb03c26e42f99
> merge: move sparse config parsing from sparse extension
>
> This function is generic and doesn't need to be a repo method.
>
> This patch also marks the beginning of moving code from sparse.py
> into core. The goal is for the extension to enable the feature and
> provide any UI adjustments.
>
> merge.py may seem like a weird destination. But that's
> where code for working directory updating lives. Unless we want
> to create a new module for just the sparse bits or something for
> working directory code, merge.py is the best current place.
>
> diff --git a/hgext/sparse.py b/hgext/sparse.py
> --- a/hgext/sparse.py
> +++ b/hgext/sparse.py
> @@ -407,40 +407,6 @@ def _setupdirstate(ui):
>
>  def _wraprepo(ui, repo):
>      class SparseRepo(repo.__class__):
> -        def readsparseconfig(self, raw):
> -            """Takes a string sparse config and returns the includes,
> -            excludes, and profiles it specified.
> -            """
> -            includes = set()
> -            excludes = set()
> -            current = includes
> -            profiles = []
> -            for line in raw.split('\n'):
> -                line = line.strip()
> -                if not line or line.startswith('#'):
> -                    # empty or comment line, skip
> -                    continue
> -                elif line.startswith('%include '):
> -                    line = line[9:].strip()
> -                    if line:
> -                        profiles.append(line)
> -                elif line == '[include]':
> -                    if current != includes:
> -                        raise error.Abort(_('.hg/sparse cannot have
> includes ' +
> -                            'after excludes'))
> -                    continue
> -                elif line == '[exclude]':
> -                    current = excludes
> -                elif line:
> -                    if line.strip().startswith('/'):
> -                        self.ui.warn(_('warning: sparse profile cannot
> use' +
> -                                       ' paths starting with /, ignoring
> %s\n')
> -                                       % line)
> -                        continue
> -                    current.add(line)
> -
> -            return includes, excludes, profiles
> -
>          def getsparsepatterns(self, rev):
>              """Returns the include/exclude patterns specified by the
>              given rev.
> @@ -452,7 +418,8 @@ def _wraprepo(ui, repo):
>                  raise error.Abort(_("cannot parse sparse patterns from " +
>                      "working copy"))
>
> -            includes, excludes, profiles = self.readsparseconfig(raw)
> +            includes, excludes, profiles = mergemod.readsparseconfig(
> +                self.ui, raw)
>
>              ctx = self[rev]
>              if profiles:
> @@ -475,8 +442,8 @@ def _wraprepo(ui, repo):
>                          else:
>                              self.ui.debug(msg)
>                          continue
> -                    pincludes, pexcludes, subprofs = \
> -                        self.readsparseconfig(raw)
> +                    pincludes, pexcludes, subprofs =
> mergemod.readsparseconfig(
> +                        self.ui, raw)
>                      includes.update(pincludes)
>                      excludes.update(pexcludes)
>                      for subprofile in subprofs:
> @@ -785,7 +752,7 @@ def _config(ui, repo, pats, opts, includ
>          raw = repo.vfs.tryread('sparse')
>          if raw:
>              oldinclude, oldexclude, oldprofiles = map(
> -                set, repo.readsparseconfig(raw))
> +                set, mergemod.readsparseconfig(ui, raw))
>          else:
>              oldinclude = set()
>              oldexclude = set()
> @@ -844,7 +811,7 @@ def _import(ui, repo, files, opts, force
>
>          # read current configuration
>          raw = repo.vfs.tryread('sparse')
> -        oincludes, oexcludes, oprofiles = repo.readsparseconfig(raw)
> +        oincludes, oexcludes, oprofiles = mergemod.readsparseconfig(ui,
> raw)
>          includes, excludes, profiles = map(
>                  set, (oincludes, oexcludes, oprofiles))
>
> @@ -861,8 +828,8 @@ def _import(ui, repo, files, opts, force
>          changed = False
>          for file in files:
>              with util.posixfile(util.expandpath(file)) as importfile:
> -                iincludes, iexcludes, iprofiles = repo.readsparseconfig(
> -                    importfile.read())
> +                iincludes, iexcludes, iprofiles =
> mergemod.readsparseconfig(
> +                    ui, importfile.read())
>                  oldsize = len(includes) + len(excludes) + len(profiles)
>                  includes.update(iincludes - aincludes)
>                  excludes.update(iexcludes - aexcludes)
> @@ -895,7 +862,7 @@ def _import(ui, repo, files, opts, force
>  def _clear(ui, repo, files, force=False):
>      with repo.wlock():
>          raw = repo.vfs.tryread('sparse')
> -        includes, excludes, profiles = repo.readsparseconfig(raw)
> +        includes, excludes, profiles = mergemod.readsparseconfig(ui, raw)
>
>          if includes or excludes:
>              oldstatus = repo.status()
> diff --git a/mercurial/merge.py b/mercurial/merge.py
> --- a/mercurial/merge.py
> +++ b/mercurial/merge.py
> @@ -1747,3 +1747,37 @@ def graft(repo, ctx, pctx, labels, keepp
>          # fix up dirstate for copies and renames
>          copies.duplicatecopies(repo, ctx.rev(), pctx.rev())
>      return stats
> +
> +def readsparseconfig(ui, raw):
> +    """Parse sparse config file content.
> +
> +    Returns a tuple of includes, excludes, and profiles.
> +    """
> +    includes = set()
> +    excludes = set()
> +    current = includes
> +    profiles = []
> +    for line in raw.split('\n'):
> +        line = line.strip()
> +        if not line or line.startswith('#'):
> +            # empty or comment line, skip
> +            continue
> +        elif line.startswith('%include '):
> +            line = line[9:].strip()
> +            if line:
> +                profiles.append(line)
> +        elif line == '[include]':
> +            if current != includes:
> +                raise error.Abort(_('.hg/sparse cannot have includes ' +
> +                                    'after excludes'))
>
>
> Since this function already knows it's parsing .hg/sparse, does it make
> sense to pass on the repo and do the read here too?
>

Hah - this error message is wrong since it assumes you are parsing
.hg/sparse. This was likely written before sparse supported multiple
profile files. We should probably refactor this to pass a repo instance and
a path name so it can print a proper error message. Good catch.

(Moving code to core is a good way to flush out these bugs since finding
bugs in the big vendor patch will be difficult given its size.)


>
>
> +            continue
> +        elif line == '[exclude]':
> +            current = excludes
> +        elif line:
> +            if line.strip().startswith('/'):
> +                ui.warn(_('warning: sparse profile cannot use' +
> +                          ' paths starting with /, ignoring %s\n') % line)
> +                continue
> +            current.add(line)
> +
> +    return includes, excludes, profiles
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
>
>

Patch

diff --git a/hgext/sparse.py b/hgext/sparse.py
--- a/hgext/sparse.py
+++ b/hgext/sparse.py
@@ -407,40 +407,6 @@  def _setupdirstate(ui):

 def _wraprepo(ui, repo):
     class SparseRepo(repo.__class__):
-        def readsparseconfig(self, raw):
-            """Takes a string sparse config and returns the includes,
-            excludes, and profiles it specified.
-            """
-            includes = set()
-            excludes = set()
-            current = includes
-            profiles = []
-            for line in raw.split('\n'):
-                line = line.strip()
-                if not line or line.startswith('#'):
-                    # empty or comment line, skip
-                    continue
-                elif line.startswith('%include '):
-                    line = line[9:].strip()
-                    if line:
-                        profiles.append(line)
-                elif line == '[include]':
-                    if current != includes:
-                        raise error.Abort(_('.hg/sparse cannot have
includes ' +
-                            'after excludes'))
-                    continue
-                elif line == '[exclude]':
-                    current = excludes
-                elif line:
-                    if line.strip().startswith('/'):
-                        self.ui.warn(_('warning: sparse profile cannot
use' +
-                                       ' paths starting with /, ignoring
%s\n')
-                                       % line)
-                        continue
-                    current.add(line)
-
-            return includes, excludes, profiles
-
         def getsparsepatterns(self, rev):
             """Returns the include/exclude patterns specified by the
             given rev.
@@ -452,7 +418,8 @@  def _wraprepo(ui, repo):
                 raise error.Abort(_("cannot parse sparse patterns from " +
                     "working copy"))

-            includes, excludes, profiles = self.readsparseconfig(raw)
+            includes, excludes, profiles = mergemod.readsparseconfig(
+                self.ui, raw)

             ctx = self[rev]
             if profiles:
@@ -475,8 +442,8 @@  def _wraprepo(ui, repo):
                         else:
                             self.ui.debug(msg)
                         continue
-                    pincludes, pexcludes, subprofs = \
-                        self.readsparseconfig(raw)
+                    pincludes, pexcludes, subprofs =
mergemod.readsparseconfig(
+                        self.ui, raw)
                     includes.update(pincludes)
                     excludes.update(pexcludes)
                     for subprofile in subprofs:
@@ -785,7 +752,7 @@  def _config(ui, repo, pats, opts, includ
         raw = repo.vfs.tryread('sparse')
         if raw:
             oldinclude, oldexclude, oldprofiles = map(
-                set, repo.readsparseconfig(raw))
+                set, mergemod.readsparseconfig(ui, raw))
         else:
             oldinclude = set()
             oldexclude = set()
@@ -844,7 +811,7 @@  def _import(ui, repo, files, opts, force

         # read current configuration
         raw = repo.vfs.tryread('sparse')
-        oincludes, oexcludes, oprofiles = repo.readsparseconfig(raw)
+        oincludes, oexcludes, oprofiles = mergemod.readsparseconfig(ui,
raw)
         includes, excludes, profiles = map(
                 set, (oincludes, oexcludes, oprofiles))

@@ -861,8 +828,8 @@  def _import(ui, repo, files, opts, force
         changed = False
         for file in files:
             with util.posixfile(util.expandpath(file)) as importfile:
-                iincludes, iexcludes, iprofiles = repo.readsparseconfig(
-                    importfile.read())
+                iincludes, iexcludes, iprofiles =
mergemod.readsparseconfig(
+                    ui, importfile.read())
                 oldsize = len(includes) + len(excludes) + len(profiles)
                 includes.update(iincludes - aincludes)
                 excludes.update(iexcludes - aexcludes)
@@ -895,7 +862,7 @@  def _import(ui, repo, files, opts, force
 def _clear(ui, repo, files, force=False):
     with repo.wlock():
         raw = repo.vfs.tryread('sparse')
-        includes, excludes, profiles = repo.readsparseconfig(raw)
+        includes, excludes, profiles = mergemod.readsparseconfig(ui, raw)

         if includes or excludes:
             oldstatus = repo.status()
diff --git a/mercurial/merge.py b/mercurial/merge.py
--- a/mercurial/merge.py
+++ b/mercurial/merge.py
@@ -1747,3 +1747,37 @@  def graft(repo, ctx, pctx, labels, keepp
         # fix up dirstate for copies and renames
         copies.duplicatecopies(repo, ctx.rev(), pctx.rev())
     return stats
+
+def readsparseconfig(ui, raw):
+    """Parse sparse config file content.
+
+    Returns a tuple of includes, excludes, and profiles.
+    """
+    includes = set()
+    excludes = set()
+    current = includes
+    profiles = []
+    for line in raw.split('\n'):
+        line = line.strip()
+        if not line or line.startswith('#'):
+            # empty or comment line, skip
+            continue
+        elif line.startswith('%include '):
+            line = line[9:].strip()
+            if line:
+                profiles.append(line)
+        elif line == '[include]':
+            if current != includes:
+                raise error.Abort(_('.hg/sparse cannot have includes ' +
+                                    'after excludes'))


Since this function already knows it's parsing .hg/sparse, does it make
sense to pass on the repo and do the read here too?


+            continue
+        elif line == '[exclude]':
+            current = excludes
+        elif line:
+            if line.strip().startswith('/'):