Semantic of variable projectile-require-project-root inconsistent with documentation promise.

[aagon]

According to the documentation of variable projectile-require-project-root, we have :

When nil Projectile will consider the current directory the project root.

We interpret : "will work anywhere as if it were in a project by making the current directory the project root if necessary".

So we set :

(setq projectile-require-project-root nil)

And we try projectile-grep in the home directory.

Expected behavior

To work by providing a grep buffer.

Actual behavior

Does not produce anything, produces following error :

abbreviate-file-name: Wrong type argument: stringp, nil

Steps to reproduce the problem

(setq projectile-require-project-root nil)

And (projectile-grep) in the home directory (any directory that is not a project, in fact) (projectile-ag fails as well FYI).

Environment & Version information

Projectile version information

Projectile 20210225.1816

Emacs version

GNU Emacs 27.1 (build 1, x86_64-pc-linux-gnu, X toolkit, Xaw3d scroll bars)
of 2020-11-08, modified by Debian

Operating system

Debian Testing

[aagon]

The culprit has actually been found : it is the function projectile-project-vcs, that is called by both projectile-grep and projectile-ag.

This function is supposed to return the type of vc repo if any, or none.

However, when called without argument and not in a project, it produces the following error :

Debugger entered--Lisp error: (wrong-type-argument stringp nil)
  string-match("^/tmp_mnt/" nil)
  abbreviate-file-name(nil)
  projectile-locate-dominating-file(nil ".git")
  projectile-project-vcs()

This is because, according to its implementation :

(defun projectile-project-vcs (&optional project-root)
 "Determine the VCS used by the project if any.
PROJECT-ROOT is the targeted directory.  If nil, use
`projectile-project-root'."
 (or project-root (setq project-root (projectile-project-root)))
 (cond
  ;; first we check for a VCS marker in the project root itself
  ((projectile-file-exists-p (expand-file-name ".git" project-root)) 'git)
  ((projectile-file-exists-p (expand-file-name ".hg" project-root)) 'hg)
  ((projectile-file-exists-p (expand-file-name ".fslckout" project-root)) 'fossil)
  ((projectile-file-exists-p (expand-file-name "_FOSSIL_" project-root)) 'fossil)
  ((projectile-file-exists-p (expand-file-name ".bzr" project-root)) 'bzr)
  ((projectile-file-exists-p (expand-file-name "_darcs" project-root)) 'darcs)
  ((projectile-file-exists-p (expand-file-name ".svn" project-root)) 'svn)
  ;; then we check if there's a VCS marker up the directory tree
  ;; that covers the case when a project is part of a multi-project repository
  ;; in those cases you can still the VCS to get a list of files for
  ;; the project in question
  ((projectile-locate-dominating-file project-root ".git") 'git)
  ((projectile-locate-dominating-file project-root ".hg") 'hg)
  ((projectile-locate-dominating-file project-root ".fslckout") 'fossil)
  ((projectile-locate-dominating-file project-root "_FOSSIL_") 'fossil)
  ((projectile-locate-dominating-file project-root ".bzr") 'bzr)
  ((projectile-locate-dominating-file project-root "_darcs") 'darcs)
  ((projectile-locate-dominating-file project-root ".svn") 'svn)
  (t 'none)))

If called without argument, it tries to find the root-project by using the function projectile-project-root, but that function is not guarantee to always return a string with a path : it returns nil if the given argument is not a string with a path in a project, or if called without an argument but outside of any project.

For projectile-project-vcs to not cause a Lisp error, we need to be able to guarantee that project-root is always a string before it reaches cond.

I propose to replace projectile-project-root with projectile-acquire-root in the function projectile-project-vcs, which :

• Guarantees exactly that, by calling projectile-ensure-project (which seems to have been implemented exactly for this purpose)
• Correctly sets the project root to the current directory, which in turns guarantees the correct behaviour (at least consistent with the documentation of variable projectile-require-project-root) of (among others) projectile-grep and projectile-ag.

I don't think this replacement breaks anything. If we list all the callers of function projectile-project-vcs :

• projectile-dir-files is called with an argument, doesn't change anything
• idem for projectile-dir-files-alien
• idem for projectile-get-immediate-sub-projects
• Actually repairs projectile-project-info, which would cause a Lisp error otherwise when called outside a project.
• Repairs projectile-grep
• Repairs projectile-ag as well
• Possibly repairs projectile-files-with-strings as well when called from outside a project, and neither rg, nor ag, nor ack, but git is installed (could not be bothered to test, I have rg and ag on my system).
• In projectile-vc, projectile-project-vcs is called with an argument (guaranteed to be a string by projectile-acquire-root BTW)
• In projectile-check-vcs-status-of-known-projects, projectile-project-vcs is called with an argument guaranteed to be a string.

I am therefore convinced that this change would not break anything, but repair 4 functions.

If you agree, I can prepare the trivial PR.

Best,

Aymeric Agon-Rambosson

[bbatsov]

Thanks for looking into this, I'll update it myself now. projectile-project-root used to behave differently in the past and was mostly superceded by projectile-acquire-root eventually. I guess we didn't replace all the relevant usages of it.