GNU bug report logs - #70526
29.2; package-vc-upgrade failed with error message "File is not under version control"

Previous Next

To reply to this bug, email your comments to 70526 AT debbugs.gnu.org.
There is no need to reopen the bug first.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: "Yi Yue" <include_yy <at> qq.com>
To: "bug-gnu-emacs" <bug-gnu-emacs <at> gnu.org>
Subject: 29.2; package-vc-upgrade failed with error message "File is not under
 version control"
Date: Tue, 23 Apr 2024 10:51:18 +0900

[Message part 1 (text/plain, inline)]

Hi all:


I encountered the following Backtrace information when invoking
`package-vc-upgrade'.


Debugger entered--Lisp error: (error "File is not under version control")
  error("File is not under version control")
  vc-deduce-fileset-1(t nil nil)
  vc-deduce-fileset(t)
  vc-pull()
  ......


After some edebug debugging and reading the source code, I found that
this error occurs if the buffer where `package-vc-upgrade' is called is
not under a git repository. Additionally, if the current buffer of the
command invocation does not correspond to a specific file, then the
command can execute normally.


After reading the implementation of `package-vc-upgrade', I found that it
binds `default-directory' to the package's path. However, the internally
called `vc-pull' function, which uses `vc-deduce-fileset-1', first
utilizes `buffer-file-name' instead of `default-directory' to fetch
version control backend related information, which leads to this error.


In my past usage, I did not encounter this error, probably because I
usually invoked it within a file buffer under a git repository, or used
the command in a buffer provided by `list-packages'. Here is one
solution I used:


(when (eq emacs-major-version 29)
  (defun yy/package-vc-upgrade-advice (upfun pkg-desc)
    (with-temp-buffer
      (funcall upfun pkg-desc)))
  (advice-add 'package-vc-upgrade :around 'yy/package-vc-upgrade-advice))


In a minimal environment with emacs -Q, the following steps can
reproduce my issue:


;; a simple package by purcell
(package-vc-install "https://github.com/purcell/unfill")
;; then try to upgrade it in a buffer that buffer-file-name is not nil
;; and the file is not under Git version control system.
M-x package-vc-upgrade unfill RET
;; now the error may occurs


Regards


YI YUE

[Message part 2 (text/html, inline)]

Message #8 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: "Yi Yue" <include_yy <at> qq.com>
Cc: 70526 <at> debbugs.gnu.org
Subject: Re: bug#70526: 29.2; package-vc-upgrade failed with error message
 "File is not under version control"
Date: Tue, 23 Apr 2024 08:31:49 +0000

[Message part 1 (text/plain, inline)]

"Yi Yue" <include_yy <at> qq.com> writes:

> Hi all:
>
>
> I encountered the following Backtrace information when invoking
> `package-vc-upgrade'.
>
>
> Debugger entered--Lisp error: (error "File is not under version control")
> &nbsp; error("File is not under version control")
> &nbsp; vc-deduce-fileset-1(t nil nil)
> &nbsp; vc-deduce-fileset(t)
> &nbsp; vc-pull()
> &nbsp; ......
>
>
> After some edebug debugging and reading the source code, I found that
> this error occurs if the buffer where `package-vc-upgrade' is called is
> not under a git repository. Additionally, if the current buffer of the
> command invocation does not correspond to a specific file, then the
> command can execute normally.
>
>
> After reading the implementation of `package-vc-upgrade', I found that it
> binds `default-directory' to the package's path. However, the internally
> called `vc-pull' function, which uses `vc-deduce-fileset-1', first
> utilizes `buffer-file-name' instead of `default-directory' to fetch
> version control backend related information, which leads to this error.
>
>
> In my past usage, I did not encounter this error, probably because I
> usually invoked it within a file buffer under a git repository, or used
> the command in a buffer provided by `list-packages'. Here is one
> solution I used:
>
>
> (when (eq emacs-major-version 29)
> &nbsp; (defun yy/package-vc-upgrade-advice (upfun pkg-desc)
> &nbsp; &nbsp; (with-temp-buffer
> &nbsp; &nbsp; &nbsp; (funcall upfun pkg-desc)))
> &nbsp; (advice-add 'package-vc-upgrade :around 'yy/package-vc-upgrade-advice))

Injecting this into package-vc, we get:

[Message part 2 (text/plain, inline)]

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index ef056c7909b..4e88964ecd0 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -810,8 +810,9 @@ package-vc-upgrade
                   (remove-hook 'vc-post-command-functions post-upgrade))))))
     (add-hook 'vc-post-command-functions post-upgrade)
     (with-demoted-errors "Failed to fetch: %S"
-      (let ((default-directory pkg-dir))
-        (vc-pull)))))
+      (with-temp-buffer
+        (let ((default-directory pkg-dir))
+          (vc-pull))))))
 
 (defun package-vc--archives-initialize ()
   "Initialize package.el and fetch package specifications."

[Message part 3 (text/plain, inline)]

(or alternatively just binding buffer-file-name to nil after default-directory).

but it feels like the fix should happen in vc.el, by being able to
communicate that we the file set should be derived from the default
directory, and the current file should be ignored...

> In a minimal environment with emacs -Q, the following steps can
> reproduce my issue:
>
>
> ;; a simple package by purcell
> (package-vc-install "https://github.com/purcell/unfill")
> ;; then try to upgrade it in a buffer that buffer-file-name is not nil
> ;; and the file is not under Git version control system.
> M-x package-vc-upgrade unfill RET
> ;; now the error may occurs
>
>
> Regards
>
>
> YI YUE

-- 
	Philip Kaludercic on peregrine

Message #11 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: "Yi Yue" <include_yy <at> qq.com>
To: "70526" <70526 <at> debbugs.gnu.org>
Cc: philipk <philipk <at> posteo.net>
Subject: Re: bug#70526: 29.2;
 package-vc-upgrade failed with error message "File is not under
 version control"
Date: Tue, 23 Apr 2024 19:11:33 +0900

[Message part 1 (text/plain, inline)]

> (or alternatively just binding buffer-file-name to nil after default-directory).


Yes, this is simpler.


> but it feels like the fix should happen in vc.el, by being able to
> communicate that we the file set should be derived from the default
> directory, and the current file should be ignored...


Maybe it is better, but I don't know what will happen if we
change the implementation of `vc-deduce-fileset-1', I don't have
much knowledge of vc.el.

[Message part 2 (text/html, inline)]

Message #14 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: "Yi Yue" <include_yy <at> qq.com>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2; package-vc-upgrade failed with error message
 "File is not under version control"
Date: Tue, 23 Apr 2024 19:35:57 +0000

(unrelated, but why do your messages include HTML entities?)

"Yi Yue" <include_yy <at> qq.com> writes:

> &gt; (or alternatively just binding buffer-file-name to nil after default-directory).
>
>
> Yes, this is simpler.

My main fear is that this might have some unintended consequences.

> &gt; but it feels like the fix should happen in vc.el, by being able to
> &gt; communicate that we the file set should be derived from the default
> &gt; directory, and the current file should be ignored...
>
> Maybe it is better, but I don't know what will happen if we
> change the implementation of `vc-deduce-fileset-1', I don't have
> much knowledge of vc.el.

I wouldn't want to rely on my understanding of vc either.  While it
would be easy to add a dynamic variable to indicate this behaviour, I am
careful not to overburden the abstractions that VC provides.

-- 
	Philip Kaludercic on peregrine

Message #17 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: "Yi Yue" <include_yy <at> qq.com>
To: "Philip Kaludercic" <philipk <at> posteo.net>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2;
 package-vc-upgrade failed with error message "File is not under
 version control"
Date: Wed, 24 Apr 2024 06:07:47 +0900

[Message part 1 (text/plain, inline)]

> (unrelated, but why do your messages include HTML entities?) (Sorry, I forgot to forbid the default formatter of my email client.) > My main fear is that this might have some unintended consequences. Of course. > I wouldn't want to rely on my understanding of vc either.  While it > would be easy to add a dynamic variable to indicate this behaviour, I am > careful not to overburden the abstractions that VC provides. Understand. I read the docstring of `vc-pull', which is used by `package-vc-upgrade'. It says: --------------------------------------------------------------------------   "Update the current fileset or branch. You must be visiting a version controlled file, or in a `vc-dir' buffer. ..." -------------------------------------------------------------------------- But `vc-dir' is a user command other than a function, it will open a  new buffer and return nil. In consideration of the fact that `vc-pull'  is an async function, it is not easy for us to kill the `vc-diff' buffer  right after the pull operation. As you suggested earlier, maybe we need to modify vc.el, making the restriction looser?  Also, I noticed that the maintainer bind `default-directory' in this commit: https://github.com/emacs-mirror/emacs/commit/7ab556b57631cb28db86b89ba296bc0599d9a399 Improve robustness of 'package-vc-update' Regards

[Message part 2 (text/html, inline)]

Message #20 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: "Yi Yue" <include_yy <at> qq.com>
To: "Philip Kaludercic" <philipk <at> posteo.net>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2;
 package-vc-upgrade failed with error message "File is not under
 version control"
Date: Wed, 24 Apr 2024 11:00:32 +0900

[Message part 1 (text/plain, inline)]

(sorry for the format problem again) > (unrelated, but why do your messages include HTML entities?)  (Sorry, I forgot to forbid the default formatter of my email client.) > My main fear is that this might have some unintended consequences.  Of course.  > I wouldn't want to rely on my understanding of vc either. While it  > would be easy to add a dynamic variable to indicate this behaviour, I am  > careful not to overburden the abstractions that VC provides.  Understand.  I read the docstring of `vc-pull', which is used by `package-vc-upgrade'.  It says:  --------------------------------------------------------------------------  "Update the current fileset or branch.  You must be visiting a version controlled file, or in a `vc-dir' buffer.  ..." --------------------------------------------------------------------------  But `vc-dir' is a user command other than a function, it will open a new buffer and return nil.  In consideration of the fact that `vc-pull' is an async function, it is not easy for us to  kill the `vc-diff' buffer right after the pull operation.  As you suggested earlier, maybe we need to modify vc.el, making the restriction looser?  Also, I noticed that the maintainer bind `default-directory' in this commit:  https://github.com/emacs-mirror/emacs/commit/7ab556b57631cb28db86b89ba296bc0599d9a399  Improve robustness of 'package-vc-update' Regards

[Message part 2 (text/html, inline)]

Message #23 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: "Yi Yue" <include_yy <at> qq.com>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2; package-vc-upgrade failed with error message
 "File is not under version control"
Date: Wed, 24 Apr 2024 06:19:19 +0000

[Message part 1 (text/plain, inline)]

"Yi Yue" <include_yy <at> qq.com> writes:

>> (unrelated, but why do your messages include HTML entities?) 
>
> (Sorry, I forgot to forbid the default formatter of my email client.)

it appears more or less correctly when I view the HTML message, so don't
worry.

>> My main fear is that this might have some unintended consequences. 
>
> Of course. 
>
>> I wouldn't want to rely on my understanding of vc either. While it 
>> would be easy to add a dynamic variable to indicate this behaviour, I am 
>> careful not to overburden the abstractions that VC provides. 
>
> Understand. 
>
> I read the docstring of `vc-pull', which is used by `package-vc-upgrade'. 
> It says: 
>
> -------------------------------------------------------------------------- 
> "Update the current fileset or branch. 
> You must be visiting a version controlled file, or in a `vc-dir' buffer. 
> ..."
> -------------------------------------------------------------------------- 
>
> But `vc-dir' is a user command other than a function, it will open a new buffer and return nil. 
> In consideration of the fact that `vc-pull' is an async function, it is not easy for us to 
> kill the `vc-diff' buffer right after the pull operation. 

This actually gives us another possibility how to tackle the issue:

[Message part 2 (text/plain, inline)]

diff --git a/lisp/emacs-lisp/package-vc.el b/lisp/emacs-lisp/package-vc.el
index ef056c7909b..c29e8b5d738 100644
--- a/lisp/emacs-lisp/package-vc.el
+++ b/lisp/emacs-lisp/package-vc.el
@@ -774,6 +774,9 @@ package-vc-upgrade-all
         (package-vc-upgrade pkg-desc))))
   (message "Done upgrading packages."))
 
+(declare-function vc-dir-prepare-status-buffer "vc-dir"
+                  (bname dir backend &optional create-new))
+
 ;;;###autoload
 (defun package-vc-upgrade (pkg-desc)
   "Upgrade the package described by PKG-DESC from package's VC repository.
@@ -810,7 +813,10 @@ package-vc-upgrade
                   (remove-hook 'vc-post-command-functions post-upgrade))))))
     (add-hook 'vc-post-command-functions post-upgrade)
     (with-demoted-errors "Failed to fetch: %S"
-      (let ((default-directory pkg-dir))
+      (require 'vc-dir)
+      (with-current-buffer (vc-dir-prepare-status-buffer
+                            (format " *vc-dir: %s*" pkg-dir)
+                            pkg-dir (vc-responsible-backend pkg-dir))
         (vc-pull)))))
 
 (defun package-vc--archives-initialize ()

[Message part 3 (text/plain, inline)]

I am actually satisfied with this approach, and it seems reliable.

>
> As you suggested earlier, maybe we need to modify vc.el, making the restriction looser? 

If you can, try out the above patch and tell me if you end up having any
issues, otherwise I don't think we need to adjust vc.

> Also, I noticed that the maintainer bind `default-directory' in this commit: 
>
> https://github.com/emacs-mirror/emacs/commit/7ab556b57631cb28db86b89ba296bc0599d9a399 
> Improve robustness of 'package-vc-update' Regards

FYI that was my change, GitHub just doesn't display commit names by
default:

https://github.com/emacs-mirror/emacs/commit/7ab556b57631cb28db86b89ba296bc0599d9a399.patch

-- 
	Philip Kaludercic on peregrine

Message #26 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: "Yi Yue" <include_yy <at> qq.com>
To: "Philip Kaludercic" <philipk <at> posteo.net>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2;
 package-vc-upgrade failed with error message "File is not under
 version control"
Date: Wed, 24 Apr 2024 16:02:42 +0900

[Message part 1 (text/plain, inline)]

> I am actually satisfied with this approach, and it seems reliable. > > > > As you suggested earlier, maybe we need to modify vc.el, making the restriction looser? > If you can, try out the above patch and tell me if you end up having any > issues, otherwise I don't think we need to adjust vc. Yes, it works as expected, thanks. But now it will create an additional empty buffer named "{pkg}*", {pkg} is the name of pacakge. So I suggest the following code: ------------------------------------------------------------------ (with-demoted-errors "Failed to fetch: %S"   (require 'vc-dir)   (let ((vcbuf (vc-dir-prepare-status-buffer                 (format " *vc-dir: %s*" pkg-dir)                 pkg-dir (vc-responsible-backend pkg-dir))))     (unwind-protect 	(with-current-buffer vcbuf           (vc-pull))       (kill-buffer vcbuf)))) ------------------------------------------------------------------- Anyway, I think this buffer doesn't matter. Both are OK. Regards Yi Yue

[Message part 2 (text/html, inline)]

Message #29 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: "Yi Yue" <include_yy <at> qq.com>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2; package-vc-upgrade failed with error message
 "File is not under version control"
Date: Wed, 24 Apr 2024 07:51:54 +0000

"Yi Yue" <include_yy <at> qq.com> writes:

>> >
>> > As you suggested earlier, maybe we need to modify vc.el, making the restriction looser?
>
>> If you can, try out the above patch and tell me if you end up having any
>> issues, otherwise I don't think we need to adjust vc.
>
> Yes, it works as expected, thanks. But now it will create an
> additional empty buffer named "{pkg}*", {pkg} is the name of pacakge.
> So I suggest the following code:
>
> ------------------------------------------------------------------
> (with-demoted-errors "Failed to fetch: %S"
>   (require 'vc-dir)
>   (let ((vcbuf (vc-dir-prepare-status-buffer
>                 (format " *vc-dir: %s*" pkg-dir)
>                 pkg-dir (vc-responsible-backend pkg-dir))))
>     (unwind-protect
> 	(with-current-buffer vcbuf
>           (vc-pull))
>       (kill-buffer vcbuf))))
> -------------------------------------------------------------------
>
> Anyway, I think this buffer doesn't matter. Both are OK.

As the buffer is hidden, I didn't see the need to clean it up.  It can
be re-used later on and might help with debugging.

> Regards
>
> Yi Yue

-- 
	Philip Kaludercic on peregrine

Message #32 received at 70526 <at> debbugs.gnu.org (full text, mbox):

From: "o0o0o����" <include_yy <at> qq.com>
To: "Philip Kaludercic" <philipk <at> posteo.net>
Cc: 70526 <70526 <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2;
 package-vc-upgrade failed with error message "File is not under
 version control"
Date: Wed, 24 Apr 2024 17:09:18 +0900

[Message part 1 (text/plain, inline)]

> As the buffer is hidden, I didn't see the need to clean it up.  It can > be re-used later on and might help with debugging. I think you are right. Now we fix this bug. Regards

[Message part 2 (text/html, inline)]

Message #37 received at 70526-done <at> debbugs.gnu.org (full text, mbox):

From: Philip Kaludercic <philipk <at> posteo.net>
To: o0o0o耗子 <include_yy <at> qq.com>
Cc: 70526 <70526-done <at> debbugs.gnu.org>
Subject: Re: bug#70526: 29.2; package-vc-upgrade failed with error message
 "File is not under version control"
Date: Sat, 27 Apr 2024 07:28:19 +0000

"o0o0o耗子" <include_yy <at> qq.com> writes:

> &gt; As the buffer is hidden, I didn't see the need to clean it up.
> It can &gt; be re-used later on and might help with debugging. I think
> you are right. Now we fix this bug. Regards

I have pushed the changes, and will therefore close this bug report.
Thank you for your help!

-- 
	Philip Kaludercic on peregrine

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.