Add TRAMP support #29

siddharthverma314 · 2021-12-19T01:52:32Z

Hi,

This PR adds tramp support and closes #27 . It has only been tested by me, so I would appreciate it if others could try it out and see if it works for them!

purcell

Thanks for this, looking pretty good! I haven't tested it out yet but I spotted a couple of little cosmetic things.

purcell · 2021-12-20T08:56:59Z

envrc.el

@@ -178,9 +191,10 @@ called `cd'"
  "Update the current buffer's environment if it is managed by direnv.
 All envrc.el-managed buffers with this env will have their
 environments updated."
-  (let ((env-dir (envrc--find-env-dir)))
+  (let ((env-dir (envrc--find-env-dir))
+	(env (envrc--get-process-environment)))


Is there a tab here? The alignment is off.

purcell · 2021-12-20T08:57:23Z

envrc.el

@@ -149,6 +148,20 @@ One of '(none on error).")

 ;;; Internals

+(defun envrc--get-process-environment ()
+  "Returns correct process envirionment for local and remote buffers"


Suggested change

"Returns correct process envirionment for local and remote buffers"

"Returns correct process environment for local and remote buffers."

purcell · 2021-12-20T08:57:52Z

envrc.el

+    process-environment))
+
+(defmacro envrc--with-tramp-vec (vec &rest body)
+  "Binds the tramp vector to VEC and connection buffer to CONN,


This docstring looks wrong (CONN?) and not conformant with checkdoc.

purcell · 2021-12-20T09:00:18Z

envrc.el

@@ -213,16 +227,6 @@ MSG and ARGS are as for that function."
      (insert (apply 'format msg args))
      (newline))))

-(defun envrc--directory-path-deeper-p (a b)


Ah, thanks for deleting this bit

purcell · 2021-12-20T18:00:13Z

I tried this out by adding a .envrc containing export FOO=bar on a remote host, then connecting with dired to the directory via an /ssh: TRAMP path. The *envrc* buffer showed the var was parsed, but I don't see any effect on tramp-remote-process-environment, or the expected output when running M-! echo $FOO. What did you try in order to test this out?

siddharthverma314 · 2021-12-20T19:37:58Z

Hey, thanks for your feedback! In my initial implementation, I wrongly assumed that the variables must be set in the tramp connection buffer. I've updated my PR to reflect this. Most normal operations should now work. I also fixed all of your suggestions.

However, it seems like shell programs still don't work. Using the test you suggested, tramp-remote-process-environment is updated, but M-! echo $FOO does not output anything, and eshell does not have the correct environment variable set. I'm currently working on resolving this.

siddharthverma314 · 2021-12-21T07:21:51Z

It seems like tramp-remote-process-environment must be set on the connection buffer in order for (exec-path) to work. I've updated the PR to set the variables on both the connection buffer and local buffer, although ideally the code should use connection-local-variables to update the environment for all tramp-related buffers. Changing the PR to a draft as it needs more work!

purcell · 2021-12-22T10:40:01Z

Thanks for this, I haven't worked much with the tramp machinery, so I appreciate you tackling the issue.

andir · 2022-12-15T12:20:45Z

Sorry for bumping this almost a year old conversation but as a recent migrant into Emacs I would love this feature :-)

@siddharthverma314 did you continue working the version posted here? Is there something I can do to help testing this?

purcell · 2022-12-15T17:15:04Z

Hey Andi, yeah, I was wondering about this recently too.

siddharthverma314 · 2022-12-16T00:05:42Z

Thanks for your interest! Back when I was writing this, the tramp implementation (to my knowledge) did not support buffer local tramp-remote-process-environment, making it impossible to implement an envrc-style approach. This might have changed recently -- I'll need to look into it more. Meanwhile, I have a PR open on emacs-direnv for TRAMP support that should work.

d-miketa · 2022-12-27T20:18:57Z

Hi, just wanted to say I'd love this feature too! :) Happy to help test it.

sellout · 2023-05-24T18:50:10Z

@siddharthverma314 I haven’t done extensive testing yet, but tramp-remote-process-environment can be set buffer-locally (which I’m guessing you know). However, given your comment:

It seems like tramp-remote-process-environment must be set on the connection buffer in order for (exec-path) to work.

if that’s still the case, then buffer-local doesn’t help, because there’s only one relevant buffer per connection.

I had another thought that might help – do what you’re currently doing, but add advice to the various TRAMP functions like

(advice-add 'some-tramp-function
            :before
            (lambda (orig-fn &rest args)
              ;; get the possibly buffer-local variables from the current (non-connection) buffer
              (let ((local-process-env tramp-remote-process-env)
                    (local-path tramp-remote-path))
                (with-current-buffer (tramp-get-connection-buffer default-directory)
                  ;; assign the non-connection buffer values to the connection buffer
                  (setq-local tramp-remote-process-env local-process-env
                              tramp-remote-path local-path)))))

which would hopefully then use the appropriate environment for each command (and buffers that don’t have buffer-local values for these vars would just end up setting the connection buffer’s values to to defaults. (There are coarser variants, like updating the connection-buffer’s variables whenever the selected window changes, like via window-state-change-hook, but that could maybe lead to race conditions?)

dcunited001 · 2023-10-04T21:40:09Z

👍 if possible, that would be awesome

bbigras · 2023-10-13T21:57:16Z

@andir did you find a workaround to use tramp with a remote nix shell?

siddharthverma314 · 2023-11-15T05:43:18Z

Hey everyone, I've been working on this locally and I think I have an implementation that works ~20% of the time 🤣. I'd love to gather feedback about whether it works or not, but try at your own risk. Or if you find bugs, let me know and I can fix them.

Also, thank you @sellout for the inspiration, I ended up implementing an advice around tramp-get-connection-buffer and tramp-get-remote-path.

Note that in order to enable direnv over tramp, you will need to set envrc-remote to t. This is the config I'm using locally:

(use-package envrc
  :straight (envrc :host github :repo "purcell/envrc"
                   :fork (:host github :repo "siddharthverma314/envrc"))
  :config
  (setq envrc-remote t)
  (envrc-global-mode))

Thanks for your patience!

sellout · 2023-11-15T06:18:25Z

Thanks, @siddharthverma314! I should be able to give this a go tomorrow.

purcell · 2023-11-15T09:12:47Z

Nice! In what way does it fail to work the rest of the time?

hso · 2023-11-15T13:54:15Z

Thanks, @siddharthverma314. I've been looking for something like this for some time. I can't get it working yet, though. When I use your sample config I get envrc/:config: Symbol's value as variable is void: vec when I try to load a file over TRAMP or if I manually call envrc-global-mode. What could I be missing?

siddharthverma314 · 2023-11-15T22:22:42Z

@hso Thanks for the feedback! I'm assuming you tried the obvious, like restarting emacs, running M-x straight-pull-package and reloading envrc.el. Can you provide the following info:

Which version of tramp are you using? For me, M-x tramp-version outputs 2.7.0-pre, so maybe there is minimum tramp version that is required.
Could you provide a stack trace for when this happens? Easy way of doing that is to M-x toggle-debug-on-error and try to enable envrc-global-mode. I'm mainly curious whether this error happens within envrc-global-mode or envrc-get-remote-path.

siddharthverma314 · 2023-11-15T22:25:54Z

@purcell In my testing, things got slow sometimes or the remote path got messed up, but these might have been problems with earlier versions of the code. For now it seems to run fine, but I definitely haven't tested all cases yet.

lakkiy · 2023-11-18T03:22:27Z

Is this ready to merge?

purcell · 2023-11-27T16:12:50Z

Symbol's value as variable is void: vec when I try to load a file over TRAMP or if I manually call envrc-global-mode. What could I be missing?

Perhaps it's the with-parsed-tramp-file-name call — there's no (eval-when-compile (require 'tramp)) present in the changes, so if that macro is unknown, Emacs would produce an error message like that.

Is this ready to merge?

Not yet. I want to understand how it works and what the trade-offs are, and I haven't had the time for a deep dive. For example, the changes apparently rely on copying buffer-local environments into the connection buffer "just in time" for commands to be executed there, so I worry about collisions or unexpected lingering effects. e.g. If you have multiple envrc-mode-enabled buffers on a tramp connection, and some buffers on that same connection that do not have envrc-mode activated, those latter buffers would presumably see the environment left by the last command run in one of the former buffers.

But I think this is promising.

envrc.el

hso · 2023-12-12T19:06:27Z

@siddharthverma314 yes, I tried restarting emacs, running straight-pull-package and reloading envrc.el, but I still get that error.

1. Which version of tramp are you using? For me, `M-x tramp-version` outputs `2.7.0-pre`, so maybe there is minimum tramp version that is required.

I don't have that command available, I suppose I have never installed a package for tramp, I'm just using the one that comes with my emacs version: GNU Emacs 29.1 (build 2, x86_64-suse-linux-gnu, GTK+ Version 3.24.38, cairo version 1.17.8)

2. Could you provide a stack trace for when this happens? Easy way of doing that is to `M-x toggle-debug-on-error` and try to enable `envrc-global-mode`. I'm mainly curious whether this error happens within `envrc-global-mode` or `envrc-get-remote-path`.

Sure:

Debugger entered--Lisp error: (void-variable vec)
  envrc-global-mode(toggle)
  funcall-interactively(envrc-global-mode toggle)
  #<subr call-interactively>(envrc-global-mode record nil)
  call-interactively@ido-cr+-record-current-command(#<subr call-interactively> envrc-global-mode record nil)
  apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> (envrc-global-mode record nil))
  call-interactively(envrc-global-mode record nil)
  command-execute(envrc-global-mode record)
  execute-extended-command(nil "envrc-global-mode" nil)
  funcall-interactively(execute-extended-command nil "envrc-global-mode" nil)
  #<subr call-interactively>(execute-extended-command nil nil)
  call-interactively@ido-cr+-record-current-command(#<subr call-interactively> execute-extended-command nil nil)
  apply(call-interactively@ido-cr+-record-current-command #<subr call-interactively> (execute-extended-command nil nil))
  call-interactively(execute-extended-command nil nil)
  command-execute(execute-extended-command)

bbigras · 2024-04-11T04:29:28Z

Any progress on this? or maybe a workaround.

bbigras · 2024-05-09T15:18:44Z

with 5c1d1eb, I got env: ‘direnv’: No such file or directory.

I tried with /ssh: and /sshx: with (setq envrc-remote t).

sgrb · 2024-10-21T10:58:51Z

envrc.el

+  "Whether or not to enable direnv over TRAMP."
+  :type 'boolean)
+
+(defcustom envrc-supported-tramp-methods '("ssh" "sshx")


I propose adding "scp" "scpx" here, since they differ only in file transfer method, everything else is the same as for ssh[x]. At least for me, it works well for scpx.

sgrb · 2024-10-21T11:01:04Z

Is there any progress on merging this PR? At least for me it works well with remote direnv + nix.

purcell · 2024-10-21T15:59:39Z

Is there any progress on merging this PR?

Unsure of the current status, and I don't use the changes myself. The merge conflict looks trivial, but @bbigras reported issues above, maybe due to not having direnv installed on the target machine?

sgrb · 2024-11-18T14:24:34Z

@bbigras reported issues above, maybe due to not having direnv installed on the target machine?

It looks plausible (or maybe PATH issues on the remote end). At least I use envrc with this PR applied almost every day (both locally and remotely over TRAMP), and didn't have any issues so far.

sellout · 2024-11-18T14:33:59Z

I use envrc with this PR applied almost every day (both locally and remotely over TRAMP), and didn't have any issues so far.

Same. In fact, I recently switched to my own fork to test out some unrelated changes, and was immediately affected by the loss of TRAMP support 😆

purcell · 2024-11-18T17:04:10Z

I've resolved this conflict and merged this PR into the main branch. Couple of questions for you all though:

Do you find that shell-command-to-string etc. work okay from remote buffers that use ? I'm wondering if changes to inheritenv might be necessary too.
Any reason to keep the envrc-remote custom var added by this PR? I feel like tramp support should just always be enabled, or am I missing something?

siddharthverma314 · 2024-11-19T05:27:50Z

Hey! Thanks for merging this PR. Regarding the concerns you raised above:

inheritenv changes may be necessary because I was experiencing odd behavior with magit and couldn't track down why. I think this PR should do it.
We should keep envrc-remote disabled by default because the functionality does not have test coverage 😄. Once I figure out how to do that, it can be removed. As a side note, I have no clue if this works with connection local variables.

Sorry for the delay in pushing this through.

purcell · 2024-11-19T11:25:23Z

We should keep envrc-remote disabled by default because the functionality does not have test coverage 😄. Once I figure out how to do that, it can be removed. As a side note, I have no clue if this works with connection local variables.

Even the existing tests have been a bit broken for a while tbh. :( I'd be inclined to remove the var anyway if that's the only technical reason for it — worst case would be that we get a few helpful bug reports.

purcell · 2024-11-19T11:36:51Z

I'll close this PR to avoid confusion, but feel free to comment further, or open a follow-up issue or PR.

collinarnett · 2025-01-30T00:09:55Z

For Nix home-manager users if you are SSHing into a machine that manages direnv with SSH, direnv will not be found. Use the snippet outlined in this wiki article to solve the problem.

https://nixos.wiki/wiki/Emacs#Cannot_find_all_binaries_on_a_remote_system_with_TRAMP

This solved my problem since I was running into the same issue as @bbigras

purcell reviewed Dec 20, 2021

View reviewed changes

siddharthverma314 marked this pull request as draft December 21, 2021 07:15

siddharthverma314 force-pushed the master branch from 880e6c3 to 4ece907 Compare November 15, 2023 05:26

siddharthverma314 force-pushed the master branch from 4ece907 to cbf7104 Compare November 28, 2023 15:16

purcell reviewed Nov 28, 2023

View reviewed changes

envrc.el Outdated Show resolved Hide resolved

siddharthverma314 force-pushed the master branch from cbf7104 to 1ebacbe Compare November 28, 2023 20:52

siddharthverma314 added 2 commits May 6, 2024 19:44

Add TRAMP support

c6998ec

Add sshx as a supported backend

5c1d1eb

siddharthverma314 force-pushed the master branch from 1ebacbe to 5c1d1eb Compare May 6, 2024 23:45

sgrb reviewed Oct 21, 2024

View reviewed changes

purcell closed this Nov 19, 2024

	"Returns correct process envirionment for local and remote buffers"
	"Returns correct process environment for local and remote buffers."

Add TRAMP support #29

Add TRAMP support #29

Conversation

siddharthverma314 commented Dec 19, 2021

purcell left a comment

Choose a reason for hiding this comment

purcell Dec 20, 2021

Choose a reason for hiding this comment

purcell Dec 20, 2021

Choose a reason for hiding this comment

purcell Dec 20, 2021

Choose a reason for hiding this comment

purcell Dec 20, 2021

Choose a reason for hiding this comment

purcell commented Dec 20, 2021

siddharthverma314 commented Dec 20, 2021

siddharthverma314 commented Dec 21, 2021

purcell commented Dec 22, 2021

andir commented Dec 15, 2022

purcell commented Dec 15, 2022

siddharthverma314 commented Dec 16, 2022

d-miketa commented Dec 27, 2022 • edited Loading

sellout commented May 24, 2023 • edited Loading

dcunited001 commented Oct 4, 2023

bbigras commented Oct 13, 2023

siddharthverma314 commented Nov 15, 2023 • edited Loading

sellout commented Nov 15, 2023

purcell commented Nov 15, 2023

hso commented Nov 15, 2023

siddharthverma314 commented Nov 15, 2023 • edited Loading

siddharthverma314 commented Nov 15, 2023

lakkiy commented Nov 18, 2023

purcell commented Nov 27, 2023

hso commented Dec 12, 2023

bbigras commented Apr 11, 2024

bbigras commented May 9, 2024

sgrb Oct 21, 2024

Choose a reason for hiding this comment

sgrb commented Oct 21, 2024

purcell commented Oct 21, 2024

sgrb commented Nov 18, 2024 • edited Loading

sellout commented Nov 18, 2024

purcell commented Nov 18, 2024

siddharthverma314 commented Nov 19, 2024 • edited Loading

purcell commented Nov 19, 2024

purcell commented Nov 19, 2024

collinarnett commented Jan 30, 2025

d-miketa commented Dec 27, 2022 •

edited

Loading

sellout commented May 24, 2023 •

edited

Loading

siddharthverma314 commented Nov 15, 2023 •

edited

Loading

siddharthverma314 commented Nov 15, 2023 •

edited

Loading

sgrb commented Nov 18, 2024 •

edited

Loading

siddharthverma314 commented Nov 19, 2024 •

edited

Loading