[PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[Ilya Ostapyshyn]

[-- Attachment #1: Type: text/plain, Size: 729 bytes --]

Hello emacs-devel,

"vc-git-mode-line-string" (which is called automatically) failed
occasionally with an error message (wrong-type-argument arrayp nil) when
the default-directory is outside the repository.

I hunted the error down to "vc-working-revision" function, which
returned nil under those particular circumstances. The returned value
(nil) was then passed to as an argument to "substring", causing a type
error.

If there are better ways to resolve this issue, I would be very happy to
discover them. Nonetheless I've seen such approach of "let"-ing the
default-directory variable a few times across the emacs source code
tree.

Would doing this on a lower level, for example in "vc-call-backend" be a
better approach?


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: PATCH --]
[-- Type: text/x-patch, Size: 1517 bytes --]

From d651b5f38734c8972aa115ccb05b55bbd767d319 Mon Sep 17 00:00:00 2001
From: Ilya Ostapyshyn <ilya.ostapyshyn@gmail.com>
Date: Sun, 17 May 2020 13:52:41 +0300
Subject: [PATCH] * lisp/vc/vc-hooks.el: Set default-directory to be inside
 repository

---
 lisp/vc/vc-hooks.el | 15 ++++++++-------
 1 file changed, 8 insertions(+), 7 deletions(-)

diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 2ca9d3e620..ebe79311fe 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -497,13 +497,14 @@ status of this file.  Otherwise, the value returned is one of:
 (defun vc-working-revision (file &optional backend)
   "Return the repository version from which FILE was checked out.
 If FILE is not registered, this function always returns nil."
-  (or (vc-file-getprop file 'vc-working-revision)
-      (progn
-        (setq backend (or backend (vc-backend file)))
-        (when backend
-          (vc-file-setprop file 'vc-working-revision
-                           (vc-call-backend
-                            backend 'working-revision file))))))
+  (let ((default-directory (file-name-directory file)))
+    (or (vc-file-getprop file 'vc-working-revision)
+        (progn
+          (setq backend (or backend (vc-backend file)))
+          (when backend
+            (vc-file-setprop file 'vc-working-revision
+                             (vc-call-backend
+                              backend 'working-revision file)))))))
 
 ;; Backward compatibility.
 (define-obsolete-function-alias
-- 
2.26.2

Re: [PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[Dmitry Gutov]

Hi Ilya,

On 17.05.2020 14:16, Ilya Ostapyshyn wrote:
> "vc-git-mode-line-string" (which is called automatically) failed
> occasionally with an error message (wrong-type-argument arrayp nil) when
> the default-directory is outside the repository.

Could you describe a full scenario when this can happen?

Re: [PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[Ilya Ostapyshyn]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi Ilya,
>
> On 17.05.2020 14:16, Ilya Ostapyshyn wrote:
>> "vc-git-mode-line-string" (which is called automatically) failed
>> occasionally with an error message (wrong-type-argument arrayp nil) when
>> the default-directory is outside the repository.
>
> Could you describe a full scenario when this can happen?

My initial-buffer-choice is "~/somedir/startup-file.org", and
emacs-startup-hook sets the default-directory to $HOME, so that it's not
"~/somedir/" after starting up emacs.

~/somedir/ is also a git repository, containing the gnus configuration
(.newsrc in particular).

I usually start gnus just after loading up emacs (to remind,
default-directory: "~", current buffer: "~/somedir/startup-file.org")
and that's the only situation when this error has happened to me. Not
every time, but often enough to be annoying. When this happens I am
unable to reach gnus until I explicitly cd into ~/somedir/ directory.

The buffer in which the error has been occuring is .newsrc-dribble

I am unsure why this does not show up consistently, but it's clear that
the cause is the mismatch of the directories. The patch has fixed the
issue for me.

I am running GNU Emacs 27.0.91 (build 2, x86_64-apple-darwin17.7.0, NS
appkit-1561.61 Version 10.13.6 (Build 17G11023)).

The issue has also appeared on Emacs 26.3.

Re: [PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[Ilya Ostapyshyn]

Ilya Ostapyshyn <ilya.ostapyshyn@gmail.com> writes:

> Dmitry Gutov <dgutov@yandex.ru> writes:
>
> Hi Ilya,
>
> On 17.05.2020 14:16, Ilya Ostapyshyn wrote:
>> "vc-git-mode-line-string" (which is called automatically) failed
>> occasionally with an error message (wrong-type-argument arrayp nil) when
>> the default-directory is outside the repository.
>
> Could you describe a full scenario when this can happen?

Despite the complex scenario I described, the issue is not hard to
reproduce.

1. Open a file in a Git repository
2. (vc-working-revision buffer-file-name)
   => working revision hash
3. (cd "/")
4. (vc-file-clearprops buffer-file-name) ;; clear the cached revision in symbol's props
5. (vc-working-revision buffer-file-name)
   => nil

Note, that (vc-working-revision) attempts to use cached value, and if it
is unavailable calls the VC backend to fetch one.

Re: [PATCH] Fix vc-working-revision return nil when default-directory is not inside repo

[Dmitry Gutov]

On 17.05.2020 17:17, Ilya Ostapyshyn wrote:
> Despite the complex scenario I described, the issue is not hard to
> reproduce.
> 
> 1. Open a file in a Git repository
> 2. (vc-working-revision buffer-file-name)
>     => working revision hash
> 3. (cd "/")
> 4. (vc-file-clearprops buffer-file-name) ;; clear the cached revision in symbol's props
> 5. (vc-working-revision buffer-file-name)
>     => nil

OK, thank you for the examples. Indeed, the behavior is not 
well-defined, so let's fix it, especially considering the performance 
difference is negligible.

I've pushed a fix to master, a slightly different version of your patch. 
This will be in Emacs 28.

Regarding doing it in another place, that would be inside 
vc-git-working-revision. Maybe we'll switch to that approach later, 
given that neither Hg not Bzr seem to have exhibited the same problem.

But I went with the "safer" option now.