Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Bookmarks section causing TRAMP to open remote connections #408

Open
jdm204 opened this issue Nov 8, 2022 · 22 comments
Open

Bookmarks section causing TRAMP to open remote connections #408

jdm204 opened this issue Nov 8, 2022 · 22 comments

Comments

@jdm204
Copy link

jdm204 commented Nov 8, 2022

My emacs version: GNU Emacs 29.0.50 (build 3, x86_64-pc-linux-gnu, GTK+ Version 3.24.33, cairo version 1.16.0) of 2022-11-05

During startup, enabling the bookmarks section causes TRAMP to open a remote to connection for bookmarks which have a remote path, and so prompts for a password etc.

When I run bookmark-all-names and bookmark-get-filename for a remote bookmark it doesn't seem to open a connection, so it seems that it shouldn't be necessary to do so to start up the dashboard.

@ricardoricho
Copy link
Contributor

Hi @jdm204, I'm not familiar with TRAMP but the only functions that we are using from bookmark are bookmark-all-names and bookmark-get-filename.

Are you sure is a bookmark issue?
I found this issue with TRAMP running at emacs startup, but is related to recentf.el and the solution is setting recentf-auto-cleanup to never

@jeslie0
Copy link

jeslie0 commented Nov 9, 2022

I am having the same issue with remote projects also.

@jdm204
Copy link
Author

jdm204 commented Nov 9, 2022

Thanks for looking into this @ricardoricho, I am confused because I have recentf-auto-cleanup as mode, but it's not an issue with the bookmarks commented out of my dashboard config:

dashboard-items '((recents  . 10)
                       ;;(bookmarks . 10)
                       (agenda . 10)
                       (projects . 10)
                       (registers . 10))

if I uncomment bookmarks, I have the issue.

@jdm204
Copy link
Author

jdm204 commented Nov 9, 2022

I just tried setting recentf-auto-cleanup to never and re-enabling bookmarks on dashboard, but the problem remains.

I'm not proficient at elisp so apologies in advance but is there any chance bookmark-jump could get called without the user doing anything like here?

@ricardoricho
Copy link
Contributor

I'm sorry @jdm204 but I couldn't reproduce the bug. I created two bookmarks, one with sudo and other with ssh, restarted Emacs and everything is good, it didn't ask for any password.

@jeslie0 which Emacs version do you have?

I'm in Emacs 28.2 for mac. Maybe it has something to do with a newer version.

I'm not proficient at elisp so apologies in advance but is there any chance bookmark-jump could get called without the user doing anything like here?

That lambda (bookmark-jump) is only called when user hit enter over the item (bookmark)

@jeslie0
Copy link

jeslie0 commented Nov 10, 2022

I'm using Emacs 29.0.50. I will make a minimal working example and see if it fails there, however I have done the following tests to see when it works:

  • When using a /sudo: based bookmark, I am not prompted for a password when starting Emacs (but I am to start the sudo connection),
  • When using a /ssh:user@localhost, I am not prompted for a password when starting Emacs (but I am on the initial ssh connection),
  • When using /ssh:remote-machine: I am prompted for a password when starting Emacs and when creating initial the ssh connection.

@jeslie0
Copy link

jeslie0 commented Nov 10, 2022

Even with a minimal working example, I get the same results as above. I also have noticed that after the first time I launch emacs and enter my ssh key passphrase, restarting Emacs causes me to not be prompted. Maybe the key is cached somewhere?

@jeslie0
Copy link

jeslie0 commented Nov 10, 2022

One last thing, using my minimal example, I am prompted for my ssh passphrase when using Emacs 28.2 also. I am also asked for passphrase for the /ssh:user@localhost: bookmark. I imagine I wasn't before due to the key being cached.

@jeslie0
Copy link

jeslie0 commented Nov 11, 2022

I noticed I was having the same issue with consult-dir and remote files. It seems like the issue is caused by using file-directory-p. karthink/consult-dir#22

@lckarssen
Copy link

lckarssen commented Oct 8, 2023

I think I've got the same problem as @jeslie0: Dashboard stopped working after I added a remote/Tramp project to my project list. If I remove the Tramp project from my ~/.emacs.d/projects file, the Dashboard is loaded fine again.

Here's a (redacted) trace after running debug-on-error followed by dashboard-open.

Debugger entered--Lisp error: (file-error "Couldn't find command to check if file exists")
  signal(file-error ("Couldn't find command to check if file exists"))
  tramp-error((tramp-file-name "sudo" "remote_user" nil "remote.server" nil "/home/remote_user/very_long_path..." nil) file-error "Couldn't find command to check if file exists")
  tramp-find-file-exists-command((tramp-file-name "sudo" "remote_user" nil "remote.server" nil "/home/remote_user/very_long_path..." nil))
  tramp-get-file-exists-command((tramp-file-name "sudo" "remote_user" nil "remote.server" nil "/home/remote_user/very_long_path..." nil))
  tramp-sh-handle-file-exists-p("/sudo:remote_user@remote.server:/home/remote_user/very...")
  tramp-sh-file-name-handler(file-exists-p "/sudo:remote_user@remote.server:/home/remote_user/very...")
  apply(tramp-sh-file-name-handler file-exists-p "/sudo:remote_user@remote.server:/home/remote_user/very...")
  tramp-file-name-handler(file-exists-p "/sudo:remote_user@remote.server:/home/remote_user/very...")
  project-forget-zombie-projects()
  dashboard-funcall-fboundp(project-forget-zombie-projects)
  dashboard-projects-backend-load-projects()
  dashboard-insert-projects(5)
  #f(compiled-function (els) #<bytecode 0x1be9c38791d2de34>)((projects . 5))
  mapc(#f(compiled-function (els) #<bytecode 0x1be9c38791d2de34>) ((recents . 10) (projects . 5) (bookmarks . 5)))
  dashboard-insert-startupify-lists()
  dashboard-open()
  funcall-interactively(dashboard-open)
  command-execute(dashboard-open record)
  execute-extended-command(nil "dashboard-open" nil)
  funcall-interactively(execute-extended-command nil "dashboard-open" nil)
  command-execute(execute-extended-command)

I don't get prompted for a password by Tramp, but that is probably because I wasn't connected to the VPN when I started my Emacs.

From the trace, I see a call to project-forget-zomby-projects, which calls file-exists-p for each of the projects in the project list. Would it be an idea to not call this function?

[edit]

I used advice-add with :override to replace the function dashboard-projects-backend-load-projects with one that doesn't call 'project-forget-zombie-projects and now having remote projects works fine. For future reference, here's my use-package call:

(use-package dashboard
  :after (project all-the-icons)
  :init
  ;; Overwrite the function that checks projects because it tries to
  ;; see if the project still exists, but that doesn't work well for
  ;; remote (Tramp) projects, especially those behind a VPN. See
  ;; https://github.com/emacs-dashboard/emacs-dashboard/issues/408#issuecomment-1752014811 
  (defun lck/dashboard-projects-backend-load-projects ()
  "Depending on `dashboard-projects-backend' load corresponding backend.
Return function that returns a list of projects. LCK: Removed check for zombie projects"
  (cl-case dashboard-projects-backend
    (`projectile
     (require 'projectile)
     (dashboard-mute-apply (projectile-cleanup-known-projects))
     (projectile-load-known-projects))
    (`project-el
     (require 'project)
     (project-known-project-roots))
    (t
     (display-warning '(dashboard)
                      "Invalid value for `dashboard-projects-backend'"
                      :error))))
  (advice-add 'dashboard-projects-backend-load-projects :override 'lck/dashboard-projects-backend-load-projects)
  :config
  (dashboard-setup-startup-hook)
  :custom
  ;; Set the groups and number of items per group
  (dashboard-items '((recents  . 10)
                     (projects . 5)
                     (bookmarks . 5)
                     ))
  ;; Show projects known to Emacs' built-in project.el, not those
  ;; managed by the projectile package.
  (dashboard-projects-backend 'project-el)
  (dashboard-projects-show-base 'align)
  ;; Use icons from all-the-icons theme instead of the default nerd-icons
  (dashboard-icon-type 'all-the-icons)
  (dashboard-set-heading-icons t)
  (dashboard-set-file-icons t)
  ;; Display alternate Emacs logo
  (dashboard-startup-banner 'logo)
  )

@ricardoricho
Copy link
Contributor

Great @lckarssen, thanks for your comment. Would you like to open a PR?
Remove that line and also (dashboard-mute-apply (projectile-cleanup-known-projects)). I think that keeping a clean list of projects is not a dashboard responsibility.

@jcs090218
Copy link
Member

We could probably just wrap with ignore-errors? 🤔 Would that work?

(dashboard-mute-apply
  (ignore-errors
    (dashboard-funcall-fboundp #'project-forget-zombie-projects)))

@ricardoricho
Copy link
Contributor

I don't think it will work. The main issue is that project-forget-zombie-projects uses file-exists-p and as it is a remote project then ask for TRAMP authentication (password or gpg-passphrase). We are trying to avoid that file-exists-p call.
The reason to have a zombie project is to delete the files and not remove the project from the "project-list" (projectile or project). I'm not sure if I want to check the existence of each project every time I open/refresh the dashboard.

@lckarssen
Copy link

lckarssen commented Oct 9, 2023

I tested @jcs090218's suggestion and that works too. So I guess the question is: why is project-forget-zombie-projects actually called by dashboard? I agree with @ricardoricho that keeping a clean list is not dashboard's responsibility and the end result of both my approach and @jcs090218's seems to be the same: a list of projects that can contain entries for projects that don't exist any more or can't be reached any more.

The two approaches could differ in one important way: my approach doesn't change the user's project list file. I'm not sure if ignoring the errors when calling project-forget-zombie-projects will nevertheless edit the user's project list file (which I wouldn't want).

@lckarssen
Copy link

The call to project-forget-zombie-projects was added in PR #363, commit f042f82. There is no reference there about what triggered its addition to the code.

@jcs090218
Copy link
Member

Sorry, I am not very familiar with tramp.

a list of projects that can contain entries for projects that don't exist any more or can't be reached any more.

Can you elaborate on the use case of this? I've added this since I don't think dashboard should show any ghost entries (not only the project list). 🤔

@lckarssen
Copy link

lckarssen commented Oct 10, 2023

I'm using project.el to keep a list of my (mostly Git based) projects. Most of the time I work on the code locally on my PC, but, given that my projects often are (bioinforrmatics) pipelines that take several hours to run, with large data files, it is often most convenient to use Tramp to make code changes on the compute cluster itself and immediately test them there.

In my specific case, Dashboard stopped working because the project I had most recently added was on a remote server that was behind a VPN, to which I wasn't connected at the time.

In general, however, even without VPNs, I'd say it is problematic if Dashboard tries to see if a remote file or project exists by connecting to that remote location. Yes, it would be great if the list of recent files or projects is accurate/up-to-date and without ghost entries. This should work fine locally. Tramp connections, however, are often to remote hosts over SSH, which means the user is asked for a password or SSH key passphrase when e.g. file-exists-p is called. Tramp can also be used to edit a file with sudo (locally or on a remote host), which also requires the user to enter a password. Even if SSH key passphrases are cached by ssh-agent or if credentials are stored in ~/.authinfo.gpg, I'd say it is annoying if this happens whenever Dashboard is opened. If the alternative is to potentially end up with ghost projects in the list, that is more than acceptable to me.

I think @ricardoricho made a good point when he said it isn't Dashboard's responsibility to make sure the lists are up-to-date. It is the user who knows/should know best and who should take care of cleaning up any ghost entries. I never (knowingly) ran into this, but I wouldn't like it if my recentf file, bookmarks file, or projects file were edited without my knowing.

@jcs090218
Copy link
Member

Ah, okay. Thanks for the long explanation! I'll make some changes to fix this. I think it would be great if we had a symbol indicating the ghost entry. 🤔

@jcs090218
Copy link
Member

I have make it into an option by adding a new custom variable dashboard-remove-missing-entry (default is nil), so it should be good now. 🤔

@lckarssen
Copy link

I just tested the current HEAD and this works for me, with the default setting for dashboard-remove-missing-entry. However, when I set it to t, the old behaviour is back again: Dasbhboard doesn't fully load. It only shows the list of recent files and Dashboard mode is not loaded (the buffer remains in Fundamental mode).

Applying ignore-errors as you suggested above still needs to be done to make sure that Dashboard loads correctly if some of the entries in the lists can be reached.

@jcs090218
Copy link
Member

Good catch! I've applied the fix in f034bd5.

@lckarssen
Copy link

Great! That worked. 👍

🙏 Thanks for the quick responses!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

5 participants