[PATCH 1 of 2] match: delay import of fileset module

[Matt Mackall]

On Wed, 2012-04-11 at 21:07 +0200, Angel Ezquerra wrote:
> # HG changeset patch
> # User Angel Ezquerra <angel.ezquerra at gmail.com>
> # Date 1334141104 -7200
> # Node ID 2c32ff4561f6594957596cb711fe853c42f9bd38
> # Parent  063c5b7ded78b03f7b9b765dde2dade81fcfd6ac
> match: delay import of fileset module

You've made a trivial 2-line patch into a difficult to review 50-line
patch by combining your change with unrelated code movement. Don't do
that. When you combine code movement or whitespace changes with
substantive changes, the former completely obscure the latter.

This is probably not the right way to do this, as _expandsets will get
called on basically every operation involving files. It's better to
leave match.py alone and add a delayed import of match in the
infrequently-used predicate in fileset.py. This also more closely
matches the "real" import hierarchy.

> This reduces the impact of creating a circular import between the fileset and
> the match modules.
> 
> diff --git a/mercurial/match.py b/mercurial/match.py
> --- a/mercurial/match.py
> +++ b/mercurial/match.py
> @@ -6,23 +6,9 @@
>  # GNU General Public License version 2 or any later version.
>  
>  import re
> -import scmutil, util, fileset
> +import scmutil, util
>  from i18n import _
>  
> -def _expandsets(pats, ctx):
> -    '''convert set: patterns into a list of files in the given context'''
> -    fset = set()
> -    other = []
> -
> -    for kind, expr in pats:
> -        if kind == 'set':
> -            if not ctx:
> -                raise util.Abort("fileset expression with no context")
> -            s = fileset.getfileset(ctx, expr)
> -            fset.update(s)
> -            continue
> -        other.append((kind, expr))
> -    return fset, other
>  
>  class match(object):
>      def __init__(self, root, cwd, patterns, include=[], exclude=[],
> @@ -335,3 +321,20 @@
>      for kind, name in patterns:
>          if kind in ('glob', 're', 'relglob', 'relre', 'set'):
>              return True
> +
> +def _expandsets(pats, ctx):
> +    '''convert set: patterns into a list of files in the given context'''
> +    import fileset # avoid start-up nasties
> +
> +    fset = set()
> +    other = []
> +
> +    for kind, expr in pats:
> +        if kind == 'set':
> +            if not ctx:
> +                raise util.Abort("fileset expression with no context")
> +            s = fileset.getfileset(ctx, expr)
> +            fset.update(s)
> +            continue
> +        other.append((kind, expr))
> +    return fset, other
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel


-- 
Mathematics is the supreme nostalgia of our time.