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

Environment variables for remote processes (was: 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:

>> 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.

The problem is, that not all settings of process-environment might be
desired on remote hosts. process-environment keeps *local* variables.

Furthermore, some remote settings might be requested which are not in
process-environment by default. A user shall not be responsible for
those settings; that's why there is tramp-remote-process-environment.
But this is also not satisfactory, because there might be even different
settings required for different hosts.

> `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.

tramp-sh-handle-process-file does not start a new process, it reuses the
existing one. The propagation to the remote sub-process must ensure,
that those settings are not permanent. Via a subshell, or alike.

It's not that easy as it looks at first glance. That's why I would like
a kind of design, before starting to implement.

>         Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

>> 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.
> The problem is, that not all settings of process-environment might be
> desired on remote hosts. process-environment keeps *local* variables.

I'm not so sure.  We're talking here about the settings which are in 
process-environment but not in (default-toplevel-value
'process-environment), so these are all settings added via let-binding
process-environment, and in all the cases I can think of, these seem to
be either useful or harmless to propagate.

But maybe we need to tweak the heuristic by excluding some env-vars
(like PATH).

> Furthermore, some remote settings might be requested which are not in
> process-environment by default.

Not sure what you're referring to here, but it seems like a different
issue than the one at hand (which is to propagate let-bound
process-environment values).

> tramp-sh-handle-process-file does not start a new process, it reuses the
> existing one. The propagation to the remote sub-process must ensure,
> that those settings are not permanent. Via a subshell, or alike.

Of course.


        Stefan

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> The problem is, that not all settings of process-environment might be
>> desired on remote hosts. process-environment keeps *local* variables.
>
> I'm not so sure.  We're talking here about the settings which are in 
> process-environment but not in (default-toplevel-value
> 'process-environment), so these are all settings added via let-binding
> process-environment, and in all the cases I can think of, these seem to
> be either useful or harmless to propagate.

That sounds terrible: two classes of citizens in
process-environment. Some of them being there before Tramp connection
happened, and some of them added later, via let-bind or permanently.

How do you want to explain the difference to a user? It would make a
difference, whether an entry has been added to process-environment
before a Tramp connection, or afterwards.

And how would you deal with deleted entries of process-environment?
Something like

(let ((process-environment process-environment))
  (setenv "DISPLAY")
  (process-file ...))
  
>> Furthermore, some remote settings might be requested which are not in
>> process-environment by default.
>
> Not sure what you're referring to here, but it seems like a different
> issue than the one at hand (which is to propagate let-bound
> process-environment values).

I'm speaking about tramp-remote-process-environment, which uses another
mechanism. But if we have an accepted mechanism for environment
variables on remote hosts, there shall be only The One Way to set them.

>         Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

> That sounds terrible: two classes of citizens in
> process-environment.  Some of them being there before Tramp connection
> happened, and some of them added later, via let-bind or permanently.

Not sure what you mean by "before Tramp connection".  The issue is what
environment to give to the command run via `process-file'.
Whether there's a pre-existing Tramp connection or not is an
implementation detail of Tramp.  But the semantics of `process-file' is
clearly that it should receive in its environment the things that are in
`process-environment', and currently Tramp fails to obey this part of
the semantics of `process-file'.

Of course, it wouldn't be correct to inherit the whole of
`process-environment' either.  And of course, I understand that
implementing this environment handling might take more than a quick
half-hour hack.

> How do you want to explain the difference to a user?  It would make a
> difference, whether an entry has been added to process-environment
> before a Tramp connection, or afterwards.

No, the particular time at which the Tramp connection was made shouldn't
make any appreciable difference.

> (let ((process-environment process-environment))
>   (setenv "DISPLAY")
>   (process-file ...))

In my naive mental model, Tramp's implementation of `process-file' will
run "env <ENVVARS> <COMMAND>" on the remote host, so we could use "-u"
to remove elements from the environment.

But let's start by handling additions, and then we'll see if/when we need to
handle removals.

>>> Furthermore, some remote settings might be requested which are not in
>>> process-environment by default.
>> Not sure what you're referring to here, but it seems like a different
>> issue than the one at hand (which is to propagate let-bound
>> process-environment values).
> I'm speaking about tramp-remote-process-environment, which uses another
> mechanism. But if we have an accepted mechanism for environment
> variables on remote hosts, there shall be only The One Way to set them.

So it does seem like a completely different issue.  What I'm concerned
here is about the environment that is "per subprocess" rather than "per
connection".


        Stefan

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Not sure what you mean by "before Tramp connection".  The issue is what
> environment to give to the command run via `process-file'.
> Whether there's a pre-existing Tramp connection or not is an
> implementation detail of Tramp.  But the semantics of `process-file' is
> clearly that it should receive in its environment the things that are in
> `process-environment', and currently Tramp fails to obey this part of
> the semantics of `process-file'.
>
> Of course, it wouldn't be correct to inherit the whole of
> `process-environment' either.  And of course, I understand that
> implementing this environment handling might take more than a quick
> half-hour hack.

It's not so simple to decide what's appropriate settings. You do NOT
want to propagate all your local environment variables to the remote
process. Your local settings of DISPLAY or DBUS_SESSION_BUS_ADDRESS, to
give prominent examples, would confuse your remote process heavily.

And there might be application specific environment variables we've
never heard about, which could damage the remote process as well.

How to decide what's appropriate for the remote process? I don't know.

> In my naive mental model, Tramp's implementation of `process-file' will
> run "env <ENVVARS> <COMMAND>" on the remote host, so we could use "-u"
> to remove elements from the environment.

No. With a sufficient amount of environment variables, you would exceed
the maximum length of shell command lines. Tramp did it several times
already, and I had to work-around that. Then "env ..." construct is
applicable only, when you know in advance that there aren't too many settings.

> But let's start by handling additions, and then we'll see if/when we need to
> handle removals.
>
>>>> Furthermore, some remote settings might be requested which are not in
>>>> process-environment by default.
>>> Not sure what you're referring to here, but it seems like a different
>>> issue than the one at hand (which is to propagate let-bound
>>> process-environment values).
>> I'm speaking about tramp-remote-process-environment, which uses another
>> mechanism. But if we have an accepted mechanism for environment
>> variables on remote hosts, there shall be only The One Way to set them.
>
> So it does seem like a completely different issue.  What I'm concerned
> here is about the environment that is "per subprocess" rather than "per
> connection".

Yes, but here we must speak about implementation. Tramp opens a
connection (a shell) on the remote host, and sends all its internal
commands via this shell. It sends also the command intended for
process-file to that shell. It does *not* open a new (sub)process, which
could inherit environment variables from Emacs, and alike. The
environment is the same as when that shell has been started. That's why
it is, at least as of today, "per connection".

>         Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

> It's not so simple to decide what's appropriate settings. You do NOT
> want to propagate all your local environment variables to the remote
> process. Your local settings of DISPLAY or DBUS_SESSION_BUS_ADDRESS, to
> give prominent examples, would confuse your remote process heavily.

Check again: these will not be present in "the difference between
process-environment and (default-toplevel-value 'process-environment)".

>> In my naive mental model, Tramp's implementation of `process-file' will
>> run "env <ENVVARS> <COMMAND>" on the remote host, so we could use "-u"
>> to remove elements from the environment.
> No. With a sufficient amount of environment variables, you would exceed
> the maximum length of shell command lines. Tramp did it several times
> already, and I had to work-around that. Then "env ..." construct is
> applicable only, when you know in advance that there aren't too
> many settings.

But whichever other scheme we end up using, removing env-vars shouldn't
be any harder than adding some.

> Yes, but here we must speak about implementation. Tramp opens a
> connection (a shell) on the remote host, and sends all its internal
> commands via this shell. It sends also the command intended for
> process-file to that shell. It does *not* open a new (sub)process, which
> could inherit environment variables from Emacs, and alike. The
> environment is the same as when that shell has been started. That's why
> it is, at least as of today, "per connection".

I know that.  That's why I suggested to send "env <ENV> <CMD>".
Another option might be:

    (export VAR1=VAL1
     unset VAR2
     export VAR3=VAL3
     ...
     <CMD>)


-- Stefan

Re: Environment variables for remote processes

[andres.ramirez]

Hi guys this is  my case. I think it qualifies as as subprocess. I hope it helps to clarify some ideas:

* how I start emacs
It is started from a crontab and using daemon:
@reboot emacs --quick --daemon
** Notice: known condition emacs daemon issue with gtk (this gtk bug have years )
http://bugzilla.gnome.org/show_bug.cgi?id=85715
* What's the problem with emacs started before X?
 no 'DBUS_SESSION_BUS_ADDRESS' and other variables.
Test the condition with an external process notify-send (test the line below on eshell):
notify-send 'wellcome' 'Visit <a href="http://www.kipuamutay.com">kipuamutay</a>'
** enviroment variables from emacs started on cron job (very short)
COLUMNS=80
HOME=/home/aramirez
LANG=en_US.UTF-8
LINES=34
LOGNAME=aramirez
OLDPWD=/home/aramirez/downloads
PATH=/usr/bin:/bin
PWD=/home/aramirez
SHELL=/bin/sh
SHLVL=1
TERM=dumb
USER=aramirez
XAUTHORITY=/home/aramirez/.Xauthority
_=/usr/bin/emacs
** enviroment variables from emacs started after X
ANT_HOME=/usr/share/apache-ant
CLASSPATH=/home/aramirez/GNUstep/Library/Libraries/Java:/usr/lib/GNUstep/Libraries/Java
COLUMNS=80
DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-uxYK2KP5AO,guid=bde80a1c96d99a424bfa48eb546cacf2
GUILE_LOAD_PATH=/home/aramirez/GNUstep/Library/Libraries/Guile:/usr/lib/GNUstep/Libraries/Guile
HOME=/home/aramirez
INFOPATH=/usr/share/info::/home/aramirez/GNUstep/Library/Documentation/info:
LANG=en_US.UTF-8
LC_COLLATE=C
LD_LIBRARY_PATH=/home/aramirez/GNUstep/Library/Libraries:/usr/lib
LIBRARY_COMBO=gnu-gnu-gnu
LINES=34
LOGNAME=aramirez
MAIL=/var/spool/mail/aramirez
MOZ_PLUGIN_PATH=/usr/lib/mozilla/plugins
OLDPWD=/home/aramirez/downloads
PATH=/home/aramirez/GNUstep/Tools:/usr/local/sbin:/usr/local/bin:/usr/bin:/usr/lib/jvm/default/bin:/usr/bin/site_perl:/usr/bin/vendor_perl:/usr/bin/core_perl:/usr/local/bin:/home/aramirez/bin:/usr/local/bin:/home/aramirez/bin
PWD=/home/aramirez
SHELL=/bin/bash
SHLVL=5
TERM=dumb
USER=aramirez
WINDOWID=14680098
WINDOWPATH=1
XAUTHORITY=/home/aramirez/.Xauthority
XDG_RUNTIME_DIR=/run/user/1000
XDG_SEAT=seat0
XDG_SESSION_ID=c1
XDG_VTNR=1
XTERM_LOCALE=en_US.UTF-8
XTERM_SHELL=/bin/bash
XTERM_VERSION=XTerm(312)
_=/usr/bin/emacs
* CONCLUSION: In this particular case. It should be possible to add the necessary environment variables for notify-send to work.
(dbus-init-bus my-dbus-address) is not enough

** This could be unrelated to this thread, but related to this case;when emacs is started from a cron job before X: gnupg validation is not possible
*** issue with this when restarting emacs remotely (GPG developers and the gui-popup 4 requesting the passphrase), when no X then no popup :(
http://permalink.gmane.org/gmane.emacs.devel/169412

Best Regards

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> It's not so simple to decide what's appropriate settings. You do NOT
>> want to propagate all your local environment variables to the remote
>> process. Your local settings of DISPLAY or DBUS_SESSION_BUS_ADDRESS, to
>> give prominent examples, would confuse your remote process heavily.
>
> Check again: these will not be present in "the difference between
> process-environment and (default-toplevel-value
> 'process-environment)".

But then you change the definition of process-environment. The docstring
says "let-binding `process-environment' is an easy way to temporarily
change the value of an environment variable". It does not say it should
be the only way to do that, especially for process-file.

And you still have the distinction between processes on the local host,
and processes on the remote host. The former ones can expect all
settings from process-environment, the latter ones can expect only the
environment vatriables which are different to the toplevel value. Up to
now, nobody had to care about processes running locally or remote;
process-file did arrange it magically.

I do not say that I completely oppose (I start to understand your
proposal), but this might break existing code.

Fortunately, we do not need to care XEmacs compatibility here: there is
no process-file :-)

> That's why I suggested to send "env <ENV> <CMD>".  Another option
> might be:
>
>     (export VAR1=VAL1
>      unset VAR2
>      export VAR3=VAL3
>      ...
>      <CMD>)

Another possibility might be heredoc commands. But that's an
implementation detail.

> -- Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Michael Albinus]

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

> I do not say that I completely oppose (I start to understand your
> proposal), but this might break existing code.

Nitpicking: there is no clean way to remove an environment variable,
which might have been set remotely and which you do not want to have
set while executing process-file.

You can check, whether an environment variable exists on toplevel
process-environment, and does not exist on the let-bound
process-environment. Then you can call "unset ...".

But how do you do it, when this variable does not exists in your
toplevel process-environment?

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

> But then you change the definition of process-environment.  The docstring
> says "let-binding `process-environment' is an easy way to temporarily
> change the value of an environment variable".  It does not say it should
> be the only way to do that, especially for process-file.

What other way do you have in mind?

If you just use straight `setq' this will affect all subsequent
executions of subprocesses, so it only makes sense for env settings
which are not specific for one particular subprocess.

Of course, you could use

     (setq process-environment <foo1>)
     ...
     (setq process-environment <revert>)

but that's quite less convenient and efficient than using `let', so I'm
not worried if we don't handle that case.

> And you still have the distinction between processes on the local host,
> and processes on the remote host.  The former ones can expect all
> settings from process-environment, the latter ones can expect only the
> environment vatriables which are different to the toplevel value.

That's right.

> Up to now, nobody had to care about processes running locally or
> remote; process-file did arrange it magically.

That's not true: vc-hg.el now even has to test file-remote-p to decide
which command to run, specifically because process-file does not arrange
it magically.
Currently, process-file simply ignores process-environment, which works
OK in some cases but is clearly not quite right.

> Nitpicking: there is no clean way to remove an environment variable,
> which might have been set remotely and which you do not want to have
> set while executing process-file.

Actually, C-h v process-environment says:

   irrespective of where it comes from.  To use `process-environment' to
   remove an environment variable, include only its name in the list,
   without "=VALUE".


-- Stefan

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

>> But then you change the definition of process-environment.  The docstring
>> says "let-binding `process-environment' is an easy way to temporarily
>> change the value of an environment variable".  It does not say it should
>> be the only way to do that, especially for process-file.
>
> What other way do you have in mind?
>
> If you just use straight `setq' this will affect all subsequent
> executions of subprocesses, so it only makes sense for env settings
> which are not specific for one particular subprocess.
>
> but that's quite less convenient and efficient than using `let', so I'm
> not worried if we don't handle that case.

There's setenv. And it is the recommended way to manipulate process-environment.

>> Nitpicking: there is no clean way to remove an environment variable,
>> which might have been set remotely and which you do not want to have
>> set while executing process-file.
>
> Actually, C-h v process-environment says:
>
>    irrespective of where it comes from.  To use `process-environment' to
>    remove an environment variable, include only its name in the list,
>    without "=VALUE".

You are right, I forgot this.

> -- Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

>> What other way do you have in mind?
>> If you just use straight `setq' this will affect all subsequent
>> executions of subprocesses, so it only makes sense for env settings
>> which are not specific for one particular subprocess.
>> but that's quite less convenient and efficient than using `let', so I'm
>> not worried if we don't handle that case.
> There's setenv.  And it is the recommended way to manipulate
> process-environment.

I think it's basically the same situation as with setq: either it's done
within a `let' (and we're back to the let-binding case), or it affects
all subsequent subprocesses.


        Stefan

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> I think it's basically the same situation as with setq: either it's done
> within a `let' (and we're back to the let-binding case), or it affects
> all subsequent subprocesses.

Well, a pushed a respective patch to the trunk. Seems to work at least
fine with `eshell' and `shell', which are not trivial examples.

Let's see, whether other people start to rant.

>         Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Stefan Monnier]

>> I think it's basically the same situation as with setq: either it's done
>> within a `let' (and we're back to the let-binding case), or it affects
>> all subsequent subprocesses.
> Well, a pushed a respective patch to the trunk.  Seems to work at least
> fine with `eshell' and `shell', which are not trivial examples.

Does it mean that the file-remote-p hack in vc-hg.el is redundant now?


        Stefan

Re: Environment variables for remote processes

[Michael Albinus]

Stefan Monnier <monnier@IRO.UMontreal.CA> writes:

> Does it mean that the file-remote-p hack in vc-hg.el is redundant now?

Yes. You might have seen, that I have removed it.

>         Stefan

Best regards, Michael.

Re: Environment variables for remote processes

[Michael Albinus]

andres.ramirez <andres.ramirez@kipuamutay.com> writes:

> Hi guys this is  my case. I think it qualifies as as subprocess. I hope it helps to clarify some ideas:

Hi Andres,

> * how I start emacs
> It is started from a crontab and using daemon:
> @reboot emacs --quick --daemon
> ** Notice: known condition emacs daemon issue with gtk (this gtk bug have years )
> http://bugzilla.gnome.org/show_bug.cgi?id=85715

I guess you connect that daemonized Emacs via emacsclient.

> * What's the problem with emacs started before X?
>  no 'DBUS_SESSION_BUS_ADDRESS' and other variables.

For all missing environment variables, you must add them from your
client session to the daemonized Emacs via setenv. But I'm pretty sure
you know that already :-)

> ** enviroment variables from emacs started on cron job (very short)
> ** enviroment variables from emacs started after X
> * CONCLUSION: In this particular case. It should be possible to add the necessary environment variables for notify-send to work.
> (dbus-init-bus my-dbus-address) is not enough

As said above, setenv will be your friend. This extends process-environment.

However, this is not sufficient for remote processes started via
Tramp. Here you have now (after the recent change) two options:

- Add the environment variable to tramp-remote-process-environment
  before opening a remote connection:

  (setq tramp-remote-process-environment
        (cons
          "DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-uxYK2KP5AO,guid=bde80a1c96d99a424bfa48eb546cacf2"
          tramp-remote-process-environment))

  This environment variable will exist permanently on the remote side
  while running Tramp.

- Let-bind the environment variable to process-environment before
  calling process-file or start-file-process:

  (let ((process-environment
          (cons "DBUS_SESSION_BUS_ADDRESS=unix:abstract=/tmp/dbus-uxYK2KP5AO,guid=bde80a1c96d99a424bfa48eb546cacf2"
                process-environment)))
     (process-file ...))

  This environment variable will exist just for the process started on
  the remote side.
  
> Best Regards

Best regards, Michael.