Patchwork [4,of,5] setup: replace runhg() with an hgcommand helper class

# HG changeset patch
# User Adam Simpkins <simpkins@fb.com>
# Date 1498501890 25200
#      Mon Jun 26 11:31:30 2017 -0700
# Node ID 116836ddfc564a329200f8540e0a08ae63c40e8d
# Parent  f12d9578f43cc253f46a5dce2c7e288cac0fdada
setup: replace runhg() with an hgcommand helper class

Replace the runhg() function with an hgcommand helper class.  hgcommand has as
run() function similar to runhg(), but no longer requires the caller to pass in
the exact path to python and the hg script, and the environment settings for
invoking hg.

For now this diff contains no behavior changes, but in the future this will
make it easier for the hgcommand helper class to more intelligently figure out
the proper way to invoke hg.

On Mon, Jun 26, 2017 at 11:37:43AM -0700, Adam Simpkins wrote:
> # HG changeset patch
> # User Adam Simpkins <simpkins@fb.com>
> # Date 1498501890 25200
> #      Mon Jun 26 11:31:30 2017 -0700
> # Node ID 116836ddfc564a329200f8540e0a08ae63c40e8d
> # Parent  f12d9578f43cc253f46a5dce2c7e288cac0fdada
> setup: replace runhg() with an hgcommand helper class

This patch doesn't apply, and I can't quite figure out what changed in
setup.py since you started this work. Can you rebase to @ on
hg-committed and resend?

(Sorry for the trouble)

>
> Replace the runhg() function with an hgcommand helper class.  hgcommand has as
> run() function similar to runhg(), but no longer requires the caller to pass in
> the exact path to python and the hg script, and the environment settings for
> invoking hg.
>
> For now this diff contains no behavior changes, but in the future this will
> make it easier for the hgcommand helper class to more intelligently figure out
> the proper way to invoke hg.
>
> diff --git a/setup.py b/setup.py
> --- a/setup.py
> +++ b/setup.py
> @@ -148,23 +148,28 @@
>      out, err = p.communicate()
>      return p.returncode, out, err
>
> -def runhg(cmd, env):
> -    returncode, out, err = runcmd(cmd, env)
> -    # If root is executing setup.py, but the repository is owned by
> -    # another user (as in "sudo python setup.py install") we will get
> -    # trust warnings since the .hg/hgrc file is untrusted. That is
> -    # fine, we don't want to load it anyway.  Python may warn about
> -    # a missing __init__.py in mercurial/locale, we also ignore that.
> -    err = [e for e in err.splitlines()
> -           if not e.startswith(b'not trusting file') \
> -              and not e.startswith(b'warning: Not importing') \
> -              and not e.startswith(b'obsolete feature not enabled')]
> -    if err or returncode != 0:
> -        printf("stderr from '%s':" % (' '.join(cmd)), file=sys.stderr)
> -        printf(b'\n'.join([b'  ' + e for e in err]), file=sys.stderr)
> -        return ''
> -    return out
> +class hgcommand(object):
> +    def __init__(self):
> +        self.cmd = [sys.executable, 'hg']
> +        self.env = gethgenv()
>
> +    def run(self, args):
> +        cmd = self.cmd + args
> +        returncode, out, err = runcmd(cmd, self.env)
> +        # If root is executing setup.py, but the repository is owned by
> +        # another user (as in "sudo python setup.py install") we will get
> +        # trust warnings since the .hg/hgrc file is untrusted. That is
> +        # fine, we don't want to load it anyway.  Python may warn about
> +        # a missing __init__.py in mercurial/locale, we also ignore that.
> +        err = [e for e in err.splitlines()
> +               if not e.startswith(b'not trusting file') \
> +                  and not e.startswith(b'warning: Not importing') \
> +                  and not e.startswith(b'obsolete feature not enabled')]
> +        if err or returncode != 0:
> +            printf("stderr from '%s':" % (' '.join(cmd)), file=sys.stderr)
> +            printf(b'\n'.join([b'  ' + e for e in err]), file=sys.stderr)
> +            return ''
> +        return out
>
>  def gethgenv():
>      # Execute hg out of this directory with a custom environment which takes
> @@ -180,13 +185,13 @@
>          # https://bugs.python.org/issue13524#msg148850
>          env['SystemRoot'] = os.environ['SystemRoot']
>
> -env = gethgenv()
>  version = ''
>
>  if os.path.isdir('.hg'):
> -    cmd = [sys.executable, 'hg', 'log', '-r', '.', '--template', '{tags}\n']
> -    numerictags = [t for t in runhg(cmd, env).split() if t[0].isdigit()]
> -    hgid = runhg([sys.executable, 'hg', 'id', '-i'], env).strip()
> +    hg = hgcommand()
> +    cmd = ['log', '-r', '.', '--template', '{tags}\n']
> +    numerictags = [t for t in hg.run(cmd).split() if t[0].isdigit()]
> +    hgid = hg.run(['id', '-i']).strip()
>      if not hgid:
>          # Bail out if hg is having problems interacting with this repository,
>          # rather than falling through and producing a bogus version number.
> @@ -198,12 +203,10 @@
>          if hgid.endswith('+'): # propagate the dirty status to the tag
>              version += '+'
>      else: # no tag found
> -        ltagcmd = [sys.executable, 'hg', 'parents', '--template',
> -                   '{latesttag}']
> -        ltag = runhg(ltagcmd, env)
> -        changessincecmd = [sys.executable, 'hg', 'log', '-T', 'x\n', '-r',
> -                           "only(.,'%s')" % ltag]
> -        changessince = len(runhg(changessincecmd, env).splitlines())
> +        ltagcmd = ['parents', '--template', '{latesttag}']
> +        ltag = hg.run(ltagcmd)
> +        changessincecmd = ['log', '-T', 'x\n', '-r', "only(.,'%s')" % ltag]
> +        changessince = len(hg.run(changessincecmd).splitlines())
>          version = '%s+%s-%s' % (ltag, changessince, hgid)
>      if version.endswith('+'):
>          version += time.strftime('%Y%m%d')
> @@ -407,7 +410,7 @@
>          # here no extension enabled, disabled() lists up everything
>          code = ('import pprint; from mercurial import extensions; '
>                  'pprint.pprint(extensions.disabled())')
> -        returncode, out, err = runcmd([sys.executable, '-c', code], env)
> +        returncode, out, err = runcmd([sys.executable, '-c', code], gethgenv())
>          if err or returncode != 0:
>              raise DistutilsExecError(err)
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel@mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel

Thanks for applying these!  I just resent rebased versions of patches
4 and 5.  The conflict was with one of the lines changed in commit
db8531c45.