[PATCH 1 of 8] hgk: stop using util.bytesinput() to read a single line from stdin

[Gregory Szorc]

On Fri, Mar 9, 2018 at 4:35 AM, Yuya Nishihara <yuya at tcha.org> wrote:

> # HG changeset patch
> # User Yuya Nishihara <yuya at tcha.org>
> # Date 1520323525 21600
> #      Tue Mar 06 02:05:25 2018 -0600
> # Node ID f7d9876d750e048b4c0e0ec0682928e86a8e8ecb
> # Parent  b434965f984eff168a7caaa239277b15729bd0b1
> hgk: stop using util.bytesinput() to read a single line from stdin
>

Queued this series, thanks.

If you have this stdio stuff paged into your brain, you may want to look at
hook.py and what it is doing with stdio. Essentially, it is doing dup() and
dup2() to temporarily redirect stdout to stderr such that the wire protocol
can intercept output and forward it to the wire protocol client (or the
CLI). See hook.redirect(True) in the wire protocol server code and follow
the trail from there.

In the case of shell hooks, I believe those processes inherit the parent's
file descriptors. Which after hook.py mucks with the file descriptors, is
actually the stderr stream.

I question the appropriateness of the approach. I think it would be better
to e.g. send ui.ferr to shell hooks and to temporarily muck with
sys.stdout/sys.stderr when running Python hooks. But this has implications
and I haven't thought it through. I'd *really* like to see us not have to
do the dup()/dup2() dance in the hooks because that is mucking with global
state and can make it hard to debug server processes.


>
> Appears that the stdio here is an IPC channel between hg and hgk (tk)
> processes, which shouldn't need a fancy readline support.
>
> diff --git a/hgext/hgk.py b/hgext/hgk.py
> --- a/hgext/hgk.py
> +++ b/hgext/hgk.py
> @@ -51,7 +51,6 @@ from mercurial import (
>      pycompat,
>      registrar,
>      scmutil,
> -    util,
>  )
>
>  cmdtable = {}
> @@ -105,15 +104,15 @@ def difftree(ui, repo, node1=None, node2
>
>      while True:
>          if opts[r'stdin']:
> -            try:
> -                line = util.bytesinput(ui.fin, ui.fout).split(' ')
> -                node1 = line[0]
> -                if len(line) > 1:
> -                    node2 = line[1]
> -                else:
> -                    node2 = None
> -            except EOFError:
> +            line = ui.fin.readline()
> +            if not line:
>                  break
> +            line = line.rstrip(pycompat.oslinesep).split(b' ')
> +            node1 = line[0]
> +            if len(line) > 1:
> +                node2 = line[1]
> +            else:
> +                node2 = None
>          node1 = repo.lookup(node1)
>          if node2:
>              node2 = repo.lookup(node2)
> @@ -186,12 +185,11 @@ def catfile(ui, repo, type=None, r=None,
>      #
>      prefix = ""
>      if opts[r'stdin']:
> -        try:
> -            (type, r) = util.bytesinput(ui.fin, ui.fout).split(' ')
> -            prefix = "    "
> -        except EOFError:
> +        line = ui.fin.readline()
> +        if not line:
>              return
> -
> +        (type, r) = line.rstrip(pycompat.oslinesep).split(b' ')
> +        prefix = "    "
>      else:
>          if not type or not r:
>              ui.warn(_("cat-file: type or revision not supplied\n"))
> @@ -204,10 +202,10 @@ def catfile(ui, repo, type=None, r=None,
>          n = repo.lookup(r)
>          catcommit(ui, repo, n, prefix)
>          if opts[r'stdin']:
> -            try:
> -                (type, r) = util.bytesinput(ui.fin, ui.fout).split(' ')
> -            except EOFError:
> +            line = ui.fin.readline()
> +            if not line:
>                  break
> +            (type, r) = line.rstrip(pycompat.oslinesep).split(b' ')
>          else:
>              break
>
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at mercurial-scm.org
> https://www.mercurial-scm.org/mailman/listinfo/mercurial-devel
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://www.mercurial-scm.org/pipermail/mercurial-devel/attachments/20180309/29a0ffab/attachment.html>