bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Daniel Pittman]

C-x C-f /sshx:dpittman@remotehost.local:/path/to/file/in/hg/repo.txt

…and Emacs hangs.

I had problems with the Hg backend hanging via tramp; checking showed that it
was hung waiting on `less`, which was sitting there telling me that the
terminal wasn't fully featured and could it please, kindly, have some human
input to let it know that it was OK to continue.

Fixing this wasn't too dire: we can just ask hg to not use a pager, which
solves the problem.  It might be a good idea to ask it to be entirely
non-interactive, but I didn't test that, and don't require it for this to work
on my system.

After these changes are applied, the problem is resolved:

(require 'vc-hg)
(defun dp/vc-hg-state-interactive-pager-workaround (args)
  "Potentially modify process-file arguments to work around a vc-hg bug"
  (if (and (equal (first args) vc-hg-program)
           (not (cl-search vc-hg-global-switches args :test 'equal)))
      (concatenate 'list args vc-hg-global-switches)
    args))
(advice-add 'process-file :filter-args 'dp/vc-hg-state-interactive-pager-workaround)
(setq vc-hg-global-switches '("--pager" "never"))


The `vc-hg-global-switches` setting *should* be enough, but unfortunately the
`vc-hg-state` function doesn't *use* that setting, which means that it out and
out needs awful hacks to make it work.

(I could have redefined the function, but this felt better, and should not
cause trouble if we actually fix the bug.)


Anyway, options that probably make sense to set to make this smoother:

1. `HGPLAIN` exists in the environment.

This disables things that might change output, and is recommended for
non-interactive calls to try and discourage random breakage.

2. `-y` or `--noninteractive`

These cause it to pick the first choice if a prompt would have been displayed,
instead of asking interactively.  Probably good on any of the read-only
operations, at the very least.

3. --color never

...because if you have a tty, and less/hg think it is an interactive enough
call to invoke the pager and complain to a human, you might well get color
displayed as well.

4. --pager never

...because you just don't want to page the output for non-interactive use.





In GNU Emacs 24.4.1 (x86_64-apple-darwin14.0.0, NS apple-appkit-1343.14)
 of 2014-10-22 on dpittman-mbp.local
Windowing system distributor `Apple', version 10.3.1343
Configured using:
 `configure --prefix=/opt/local --with-ns --without-x --without-dbus
 'CFLAGS=-pipe -Os -arch x86_64' CPPFLAGS=-I/opt/local/include
 'LDFLAGS=-L/opt/local/lib -Wl,-headerpad_max_install_names -arch x86_64''

Important settings:
  locale-coding-system: utf-8-unix

Major mode: Emacs-Lisp

Minor modes in effect:
  eldoc-mode: t
  shell-dirtrack-mode: t
  flyspell-mode: t
  show-paren-mode: t
  global-auto-revert-mode: t
  display-battery-mode: t
  desktop-save-mode: t
  auto-insert-mode: t
  tooltip-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  menu-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  size-indication-mode: t
  column-number-mode: t
  line-number-mode: t
  auto-fill-function: do-auto-fill
  transient-mark-mode: t

Recent input:
<elided for privacy>

Recent messages:
Tramp: Decoding local file `/var/folders/29/dht2zzt17xnff164_5vrs41r95p38b/T/tramp.43368aIF.cinc' with `base64-decode-region'...done
Tramp: Inserting `/sshx:dpittman.sb.facebook.com:/home/dpittman/elided/for/privacy.txt'...done
privacy.txt has auto save data; consider M-x recover-this-file
Can't guess python-indent-offset, using defaults: 4

Tramp: Checking `vc-registered' for /sshx:dpittman.sb.facebook.com:/home/dpittman/elided/for/privacy.txt...done
Can't guess python-indent-offset, using defaults: 4
Running hg --pager never parent --template {rev} privacy.txt in foreground...
Running hg --pager never parent --template {rev} privacy.txt...OK = 0
Mark set

Load-path shadows:
~/.emacs.d/fb-emacs/git hides /opt/local/share/emacs/site-lisp/git
~/.emacs.d/fb-emacs/git-blame hides /opt/local/share/emacs/site-lisp/git-blame

Features:
(shadow sort mail-extr gnus-msg gnus-art mm-uu mml2015 mm-view mml-smime smime
dig mailcap gnus-sum nnoo emacsbug sendmail mule-util misearch multi-isearch
eieio-opt speedbar sb-image ezimage dframe find-func apropos sh-script smie
executable info jka-compr vc-dispatcher tramp-cache python json debug vc-git
eldoc gnus-topic gnus-group gnus-undo nnmail mail-source gnus-start gnus-spec
gnus-int gnus-range message idna rfc822 mml mml-sec mm-decode mm-bodies
mm-encode mail-parse rfc2231 rfc2047 rfc2045 ietf-drums mailabbrev gmm-utils
mailheader gnus-win footnote skeleton vc-hg tramp-sh tramp tramp-compat
auth-source eieio eieio-core password-cache tramp-loaddefs trampver shell
pcomplete format-spec server emacs-lock flyspell ispell filladapt align
hippie-exp edmacro kmacro fb-coding-style cc-styles cc-align cc-engine cc-vars
cc-defs byte-opt advice .loaddefs el-get el-get-autoloads el-get-list-packages
el-get-dependencies el-get-build el-get-status pp el-get-methods el-get-fossil
el-get-svn el-get-pacman el-get-github-zip el-get-github-tar el-get-http-zip
el-get-http-tar el-get-hg el-get-go el-get-git-svn el-get-fink
el-get-emacswiki el-get-http el-get-notify help-mode easymenu
el-get-emacsmirror el-get-github el-get-git el-get-elpa package epg-config
el-get-darcs el-get-cvs el-get-bzr el-get-brew el-get-builtin el-get-apt-get
el-get-recipes el-get-byte-compile el-get-custom el-get-core autoload lisp-mnt
bytecomp byte-compile cconv dired cl-macs saveplace midnight paren icomplete
grep compile comint ansi-color ring gnus gnus-ems nnheader gnus-util
mail-utils mm-util help-fns mail-prsvr wid-edit autorevert filenotify battery
desktop frameset autoinsert cus-start cus-load assoc cl gv cl-loaddefs cl-lib
time-date tooltip electric uniquify ediff-hook vc-hooks lisp-float-type mwheel
ns-win tool-bar dnd fontset image regexp-opt fringe tabulated-list newcomment
lisp-mode prog-mode register page menu-bar rfn-eshadow timer select scroll-bar
mouse jit-lock font-lock syntax facemenu font-core frame cham georgian
utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao korean japanese
hebrew greek romanian slovak czech european ethiopic indian cyrillic chinese
case-table epa-hook jka-cmpr-hook help simple abbrev minibuffer nadvice
loaddefs button faces cus-face macroexp files text-properties overlay sha1 md5
base64 format env code-pages mule custom widget hashtable-print-readable
backquote make-network-process cocoa ns multi-tty emacs)

Memory information:
((conses 16 266820 141934)
 (symbols 48 34433 2)
 (miscs 40 105 808)
 (strings 32 72690 51434)
 (string-bytes 1 2447073)
 (vectors 16 33402)
 (vector-slots 8 1327221 186720)
 (floats 8 259 880)
 (intervals 56 4074 314)
 (buffers 960 22))

bug#18940: Acknowledgement (24.4; vc-hg does not disable pager, leading to hangs (at least with tramp) )

[Daniel Pittman]

Additional notes:

Ideally, we should set `HGPLAIN=1` in the environment, but we can’t actually do that successfully if this is run with tramp: tramp doesn’t carry across `process-environment` to the remote system in `process-file` or similar commands.

Setting it unconditionally in tramp-remote-process-environment does fix the problem with no other changes applied, for files accessed via tramp.  Given `process-connection-type` of `t`, I suspect that a local instance will still have problems unless that is set in the local envirnoment.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Michael Albinus]

Daniel Pittman <dpittman@fb.com> writes:

> C-x C-f /sshx:dpittman@remotehost.local:/path/to/file/in/hg/repo.txt
>
> …and Emacs hangs.
>
> I had problems with the Hg backend hanging via tramp; checking showed that it
> was hung waiting on `less`, which was sitting there telling me that the
> terminal wasn't fully featured and could it please, kindly, have some human
> input to let it know that it was OK to continue.

<http://mercurial.selenic.com/wiki/PagerExtension> discusses the
problem, and proposes even a solution for "Tramp in Emacs" :-)

> Anyway, options that probably make sense to set to make this smoother:
>
> 1. `HGPLAIN` exists in the environment.
>
> This disables things that might change output, and is recommended for
> non-interactive calls to try and discourage random breakage.

I'm a little bit reluctant to add this per default to
`tramp-remote-process-environment'. There shouldn't be such default
application specific settings.

But the following patch might be sufficient (could you, please, test?):

--8<---------------cut here---------------start------------->8---
*** /home/albinus/src/emacs/lisp/vc/vc-hg.el.~118313~	2014-11-09 11:19:05.851785605 +0100
--- /home/albinus/src/emacs/lisp/vc/vc-hg.el	2014-11-09 11:13:53.255025363 +0100
***************
*** 210,220 ****
  			     ;; can parse the output.
  			     (append (list "TERM=dumb" "LANGUAGE=C")
  				     process-environment)))
! 			(process-file
! 			 vc-hg-program nil t nil
! 			 "--config" "alias.status=status"
! 			 "--config" "defaults.status="
! 			 "status" "-A" (file-relative-name file)))
                      ;; Some problem happened.  E.g. We can't find an `hg'
                      ;; executable.
                      (error nil)))))))
--- 210,227 ----
  			     ;; can parse the output.
  			     (append (list "TERM=dumb" "LANGUAGE=C")
  				     process-environment)))
! 			(if (file-remote-p file)
! 			    (process-file
! 			     "env" nil t nil
! 			     "HGPLAIN=1" vc-hg-program
! 			     "--config" "alias.status=status"
! 			     "--config" "defaults.status="
! 			     "status" "-A" (file-relative-name file))
! 			  (process-file
! 			   vc-hg-program nil t nil
! 			   "--config" "alias.status=status"
! 			   "--config" "defaults.status="
! 			   "status" "-A" (file-relative-name file))))
                      ;; Some problem happened.  E.g. We can't find an `hg'
                      ;; executable.
                      (error nil)))))))
--8<---------------cut here---------------end--------------->8---

> 3. --color never
>
> ...because if you have a tty, and less/hg think it is an interactive enough
> call to invoke the pager and complain to a human, you might well get color
> displayed as well.
>
> 4. --pager never
>
> ...because you just don't want to page the output for non-interactive use.

It looks, like you need to enable the respective extensions in your .hgrc.
Therefore, they cannot be added in vg-hg.el by default, I fear.

Best regards, Michael.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Michael Albinus]

Michael Albinus <michael.albinus@gmx.de> writes:

> I'm a little bit reluctant to add this per default to
> `tramp-remote-process-environment'. There shouldn't be such default
> application specific settings.
>
> But the following patch might be sufficient (could you, please, test?):

I've committed the patch to the Emacs repository. Closing the bug.

Best regards, Michael.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Stefan Monnier]

> I've committed the patch to the Emacs repository. Closing the bug.

I think it's an OK workaround for now, but we should implement a more
reliable solution.

I think "the right way" is for Tramp to propagate some env-vars in its
implementation of process-file (and start-file-process).  Of course,
it's not completely clear which envvars to propagate, but maybe we could
simply compare the current process-environment from
"global/initial" (either using default-toplevel-value or stashing the
initial value somewhere).


        Stefan

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> I think "the right way" is for Tramp to propagate some env-vars in its
> implementation of process-file (and start-file-process).  Of course,
> it's not completely clear which envvars to propagate, but maybe we could
> simply compare the current process-environment from
> "global/initial" (either using default-toplevel-value or stashing the
> initial value somewhere).

There is a proposal in the TODO list (see tramp.el):

;; * Implement a general server-local-variable mechanism, as there are
;;   probably other variables that need different values for different
;;   servers too.  The user could then configure a variable (such as
;;   tramp-server-local-variable-alist) to define any such variables
;;   that they need to, which would then be let bound as appropriate
;;   in tramp functions.  (Jason Rumney)

process-environment would be one of the candidates.

>         Stefan

Best regards, Michael.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Stefan Monnier]

> There is a proposal in the TODO list (see tramp.el):

> ;; * Implement a general server-local-variable mechanism, as there are
> ;;   probably other variables that need different values for different
> ;;   servers too.  The user could then configure a variable (such as
> ;;   tramp-server-local-variable-alist) to define any such variables
> ;;   that they need to, which would then be let bound as appropriate
> ;;   in tramp functions.  (Jason Rumney)

> process-environment would be one of the candidates.

It seems related, indeed, but in the case at hand, this
process-environment value should be specific to the process started with
process-file, not specific to a particular server.


        Stefan

bug#18940: Enabling hg extensions

[Jordi Gutiérrez Hermoso]

Michael Albinus <michael.albinus@gmx.de> writes:
> Daniel Pittman <dpittman@fb.com> writes:
> > 3. --color never
> >
> > ...because if you have a tty, and less/hg think it is an interactive
> > enough call to invoke the pager and complain to a human, you might
> > well get color displayed as well.
> >
> > 4. --pager never
> >
> > ...because you just don't want to page the output for
> > non-interactive use.

> It looks, like you need to enable the respective extensions in your .hgrc.
> Therefore, they cannot be added in vg-hg.el by default, I fear.

You can pass `--config extensions.color=` and `--config
extenions.pager=` as extra arguments to hg in order to enable these
particular extensions for a single invocation... but enabling them
just to disable their effects again seems rather silly. At any rate,
HGPLAIN=1 takes care of disabling both of these, unless I'm much
mistaken.

Still, I think it's important to spread the knowledge that hg
extensions can be temporarily enabled from the command-line. I feel
like Mercurial is much misunderstood.

bug#18940: Enabling hg extensions

[Michael Albinus]

Jordi Gutiérrez Hermoso <jordigh@octave.org> writes:

> At any rate, HGPLAIN=1 takes care of disabling both of these, unless
> I'm much mistaken.

That's what I did in vc-hg.el.

Best regards, Michael.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Michael Albinus]

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> It seems related, indeed, but in the case at hand, this
> process-environment value should be specific to the process started with
> process-file, not specific to a particular server.

We need a mechanism which shall allow kind of let-bind the process
environment variables when starting a new (remote) process. But there
shall be also server-specific defaults to be used.

I haven't started yet a design, how this could look like. Proposals welcome!

>         Stefan

Best regards, Michael.

PS: Maybe we could shift this dicussion to emacs-devel?

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Eli Zaretskii]

> From: Michael Albinus <michael.albinus@gmx.de>
> Date: Sun, 16 Nov 2014 11:55:23 +0100
> Cc: 18940@debbugs.gnu.org, dpittman@fb.com
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > It seems related, indeed, but in the case at hand, this
> > process-environment value should be specific to the process started with
> > process-file, not specific to a particular server.
> 
> We need a mechanism which shall allow kind of let-bind the process
> environment variables when starting a new (remote) process. But there
> shall be also server-specific defaults to be used.
> 
> I haven't started yet a design, how this could look like. Proposals welcome!

Maybe I'm missing something, but what's wrong with let-binding
process-environment?

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Michael Albinus]

Eli Zaretskii <eliz@gnu.org> writes:

>> We need a mechanism which shall allow kind of let-bind the process
>> environment variables when starting a new (remote) process. But there
>> shall be also server-specific defaults to be used.
>> 
>> I haven't started yet a design, how this could look like. Proposals welcome!
>
> Maybe I'm missing something, but what's wrong with let-binding
> process-environment?

process-environment handles local variables. Variables on the remote
host might require different settings.

Usually, let-binding of process-environment keeps its value, and
prepends some settings. That's not what could be applied for remote hosts.

Tramp has tramp-remote-process-environment, but this is not a
comprehensive solution. It is evaluated when a new connection is
establishes. So it could be useful for asynchronous processes, but it
doesn't play sufficiently for synchronous processes.

Best regards, Michael.

bug#18940: 24.4; vc-hg does not disable pager, leading to hangs (at least with tramp)

[Stefan Monnier]

> Maybe I'm missing something, but what's wrong with let-binding
> process-environment?

Nothing wrong with it.  The problem is that Tramp ignores those
let-bindings because it fails to propagate this environment to its
remote sub-processed.

`tramp-sh-handle-process-file' really needs to compare
process-environment with (default-toplevel-value 'process-environment)
to infer the env-vars that have been added via let-binding and then
propagate those to the remote sub-process.


        Stefan