search

bbatsov / projectile

Project Interaction Library for Emacs
https://docs.projectile.mx
GNU General Public License v3.0
4k stars 584 forks source link

Projectile not remembering remote projects #1649

Open jingxuanlim opened 3 years ago

jingxuanlim commented 3 years ago

This is essentially a repost of: https://emacs.stackexchange.com/questions/37583/projectile-not-remembering-remote-projects

Expected behavior

I expect projectile to remember remote projects every time I leave and relaunch emacs.

Actual behavior

Project only remember local projects.

Steps to reproduce the problem

(projectile already contains some local projects)

  1. ssh into a remote, password protected server M-x find-file /ssh:server:
  2. cd into remote project of interest M-x find-file /ssh:server:/path/to/remote/project
  3. M-x projectile-add-known-project
  4. Find remote project in my list when running M-x counsel-projectile-switch-project
  5. M-x kill-emacs
  6. Launch emacs
  7. Run M-x counsel-projectile-switch-project and only see my local projects

Environment & Version information

Projectile version information

Projectile 2.4.0snapshot

Emacs version

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, GTK+ Version 3.24.20, cairo version 1.16.0)

Operating system

Windows 10 running Ubuntu 20.04 on WSL 1

bbatsov commented 3 years ago

I think the actual problem is that those get deleted on startup by the code that checks if the remembered projects exist or not. Remote project don't exist until you connect with the TRAMP to the remote endpoint. Potentially we can just ignore those in the filter or something like this. Seems to however this is supposedly handled here:

(defun projectile-switch-project-by-name (project-to-switch &optional arg)
  "Switch to project by project name PROJECT-TO-SWITCH.
Invokes the command referenced by `projectile-switch-project-action' on switch.
With a prefix ARG invokes `projectile-commander' instead of
`projectile-switch-project-action.'"
  ;; let's make sure that the target directory exists and is actually a project
  ;; we ignore remote folders, as the check breaks for TRAMP unless already connected
  (unless (or (file-remote-p project-to-switch) (projectile-project-p project-to-switch))
    (projectile-remove-known-project project-to-switch)
    (error "Directory %s is not a project" project-to-switch))
  (let ((switch-project-action (if arg
                                   'projectile-commander
                                 projectile-switch-project-action)))
    (run-hooks 'projectile-before-switch-project-hook)
jingxuanlim commented 3 years ago

Hi bbatsov. Thanks for writing back!

How do I check if that snippet is actually run?

seanfarley commented 3 years ago

I'm also seeing this; odd that the code is supposed to handle this already 🤔

jingxuanlim commented 3 years ago

@seanfarley, this issue disappeared for me after I uninstalled wsl 1, installed wsl 2 and re-installed emacs.

seanfarley commented 3 years ago

It might be something related to the remote project name being the same as a local project name, hmm

132ikl commented 2 years ago

Still an issue for me on Arch Linux, Doom Emacs v21.12.0-alpha

TransshipmentEnvoy commented 2 years ago

I have similar issue on Doom Emacs. Output of doom version:

GNU Emacs     v28.1            nil
Doom core     v3.0.0-dev       HEAD -> master c2f8476c8 2022-06-29 18:12:43 +0200
Doom modules  v22.06.0-dev     HEAD -> master c2f8476c8 2022-06-29 18:12:43 +0200
stites commented 2 years ago

I have this problem using a vanilla emacs config.

stites commented 2 years ago

Here is a somewhat minimal chemacs profile I came up with to reproduce this issue (I kept some evil bindings to speed up iterating on this bug):

;;; init.el -*- lexical-binding: t; -*-

(straight-use-package 'use-package)

(use-package straight
         :custom (straight-use-package-by-default t))

(eval-when-compile
  (require 'use-package))

(use-package which-key
  :init
  (which-key-mode)
  (which-key-setup-minibuffer)
  :config
  (setq which-key-show-early-on-C-h t)
  (setq which-key-idle-secondary-delay 0.3)
  )

(use-package general
  :ensure projectile
  :config
  (general-evil-setup)

  (general-create-definer my-leader-def
    :prefix "SPC")

  (my-leader-def
   :states '(normal motion visual)
   :keymaps 'override
   ":"   '(execute-extended-command :which-key "M-x")
  )
)

(use-package projectile
  :ensure t
  :general

  (my-leader-def
   :states '(normal motion visual)
   :keymaps 'override
   "p" '(nil :which-key "projectile")
   "p p" '(projectile-switch-project :which-key "switch")
   "p a" '(projectile-add-known-project :which-key "add")
  )
  :init (projectile-mode +1))

(use-package evil
  :ensure t
  :init
  (setq evil-search-module 'evil-search)
  (setq evil-ex-complete-emacs-commands nil)
  :config
  (evil-mode 1))

(require 'tramp)

Steps to reproduce:

Observations:

What I expect:

What I see

I would love to figure out how to fix this bug, but I don't exactly know where to start (I am pretty new to emacs-lisp). @bbatsov there any other place where you filter the projects list? If not, I can just start there.

stites commented 2 years ago

I just forked projectile, blew away my straight cache, queued up my fork via the use-package :load-path, ran cp ~/.emacs.slim/projectile-bookmarks.eld{.bk,} && emacs --with-profile slim.

I changed the above code listing that @bbatsov pointed out to the following: I don't see anything in my *Messages* buffer, so I suspect the tramp project is getting deleted elsewhere.

defun projectile-switch-project-by-name (project-to-switch &optional arg)
  "...'"
  ;; let's make sure that the target directory exists and is actually a project
  ;; we ignore remote folders, as the check breaks for TRAMP unless already connected
  (unless (or (file-remote-p project-to-switch) (projectile-project-p project-to-switch))
    (message "(projectile-remove-known-project project-to-switch)") ;; <<<< note!
    (error "Directory %s is not a project" project-to-switch))
  ...

Update: Curiously, I do not need to run any projectile functions for the cache to be deleted.

it looks like the bug is run on load of projectile-mode via the ~projectile-auto-discover conditional (which runs projectile-discover-projects-in-search-path). A quick fix might be disabling this conditional via a custom property?~

Update: looks like I was too tired last night and commented out more than I needed, the above fix wont work. The culprit is in the definition of projectile-keep-project-p, which is run by projectile--cleanup-known-projects.

This is defined as:

(defun projectile-keep-project-p (project)
  "Determine whether we should cleanup (remove) PROJECT or not.

It handles the case of remote projects as well.
See `projectile--cleanup-known-projects'."
  ;; Taken from from `recentf-keep-default-predicate'
  (cond
   ((file-remote-p project nil t) (file-readable-p project))
   ((file-remote-p project) )
   ((file-readable-p project))))

My remote project is getting filtered out in the projects-removed list, which I observe to mean that all conditions return nil even though I would expect project to pass one of the first two conditions.

file-remote-p tests the location on a remote system without opening a new remote connection. Breaking down the conditions:

  1. condition 1: (file-remote-p project nil t) implies that:
    • IDENTIFICATION is nil: "the returned string is a complete remote identifier: with components method, user, and host. The components are those present in FILE, with defaults filled in for any that are missing."
    • CONNECTED is non-nil, so the connection must already be established. This is not the case as I am observing removal on startup, when TRAMP hasn't really run yet.
  2. condition 2: (file-remote-p project), our only other option. Both IDENTIFICATION and CONNECTED are nil, so I want this to be true. Unfortunately this is not the case! Why? evaluating this code after startup gives me "/ssh:host:" as I expect!

It turns out that file-remote-p has an implicit dependency on tramp being in scope -- it will silently fail, regardless of the validity of the file name!

Final validation of this, in my init.el

(message "pre-tramp: %s" (file-remote-p "/ssh:noether:~/git/hasktorch"))
(require 'tramp)
(message "pre-tramp: %s" (file-remote-p "/ssh:noether:~/git/hasktorch"))

returns what I hypothesize in the *Messages* buffer:

pre-tramp: nil
pre-tramp: /ssh:noether:
stites commented 2 years ago

Recapping on the above comment (which was treating as a rubber-duck): it turns out file-remote-p implicitly requires tramp to be loaded or it will silently fail. This seems weird since it should only be doing string parsing unless CONNECTED is non-nil.

It seems like the correct fix would be upstream (but I don't have time to report it). For everyone else: just make sure you (require 'tramp) before loading projectile-mode: 

(use-package projectile
  :ensure t
  :init
  (require 'tramp)
  (projectile-mode +1))
seanfarley commented 2 years ago

Yep, that fixed it for me as well; can confirm

seanfarley commented 2 years ago

Loading tramp is quite the penalty in start-up time (and tramp needs to be loaded so that the handlers' table is there). Would a cheaper string-match (e.g. string starts with /foo:) solution be acceptable to PR?

stites commented 2 years ago

I would hope a PR here is acceptable as I also don't want to load tramp on startup! I think the main corner case is Windows support where, if I recall correctly, absolute paths include : (I have no idea what that looks like in emacs+tramp). I think we can assume that the projectile-bookmarks.eld file was made in good faith.

To hit parity with the current functionality of projectile-keep-project-p, you could also just check if tramp is enabled before running file-remote-p.

bbatsov commented 2 years ago

Well, as this has to happen on startup there's really no way to avoid loading tramp - after all you need to check whether the known projects are remote. I'm a bit puzzled that file-remote-p depends on it being loaded, though - the function is not defined there, but I guess it depends on some state that gets updated by tramp.

Might be easiest to just document this better in the docs, as a ton of people never use tramp and probably don't care about it (me included).

seanfarley commented 2 years ago

I wonder if we can just trust the cache? Or defer cleanup until exiting (or some other time)?

seanfarley commented 2 years ago

After some discussion in the doom discord, there were a few suggestions. I'd like to ask here first before spending time on a PR since bbatsov could very well decide none of the options.