From: Michael Albinus <michael.albinus@gmx.de>
To: Dmitry Gutov <dgutov@yandex.ru>
Cc: 20637@debbugs.gnu.org
Subject: bug#20637: incompatible, undocumented change to vc-working-revision
Date: Wed, 13 Apr 2016 17:14:19 +0200 [thread overview]
Message-ID: <87oa9dzgl0.fsf@gmx.de> (raw)
In-Reply-To: <08f70cda-44be-0657-e50a-2b2c80d2c21c@yandex.ru> (Dmitry Gutov's message of "Tue, 29 Mar 2016 02:28:22 +0300")
[-- Attachment #1: Type: text/plain, Size: 2201 bytes --]
Dmitry Gutov <dgutov@yandex.ru> writes:
> This has been caused by the commit
> 7f9b037245ddb662ad98685e429a2498ae6b7c62, which made both vc-state and
> vc-working-revision use vc-responsible-backend instead of vc-backend.
Yes.
> As a result, in some backends these functions started return non-nil
> values for unknown files or directories, as long as they lie inside a
> VC repository.
vc-working-revision shall return nil for unregistered files. vc-state
shall return a non-nil value, 'unregistered.
> This change is indeed backward-incompatible, and it breaks the
> previous assumption of some backend functions that if FILE has been
> passed to it, then it's surely registered with the current
> backend.
You cannot guarantee this. Anybody is free to call the functions with
unregistered files. And in the vc-state case, it is even documented that
this could happen.
> In particular, it breaks an assumption I made when fixing #11757, that
> vc-git-state never receives an unregistered file as input. So if you
> evaluate (vc-state "1") now, it'll return `up-to-date'.
This assumption could be kept if vc-state filters such unregistered
files out.
> While reverting the change makes some tests fail, we should fix them
> in different ways.
>
> For some backends, maybe, we should accept that (vc-state
> default-directory) and (vc-working-revision default-directory) will
> return nil. Alternatively, fix that problem inside the respective
> backends, without changing the dispatching functions.
>
> Also, reverting this commit also seems to uncover tests that shouldn't
> pass anyway. Checks like
>
> (should (eq (vc-state default-directory)
> (vc-state default-directory backend)))
>
> don't verify much, and in this case they seem to verify the wrong
> thing. More on that in the respective threads in emacs-devel later.
>
> Michael, thoughts?
I've prepared a patch which just covers the case that a file is
unregistered, in both vc-state and vc-working-revision. It is a very
small change, that I believe it could still go into the emacs-25 branch.
Patch towards emacs-25 branch is appended, including modification of
vc-tests.el. Comments?
Best regards, Michael.
[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: diff --]
[-- Type: text/x-patch, Size: 7217 bytes --]
diff --git a/lisp/vc/vc-hooks.el b/lisp/vc/vc-hooks.el
index 0826744..dc7cc61 100644
--- a/lisp/vc/vc-hooks.el
+++ b/lisp/vc/vc-hooks.el
@@ -475,10 +475,11 @@ vc-state
;; FIXME: New (sub)states needed (?):
;; - `copied' and `moved' (might be handled by `removed' and `added')
(or (vc-file-getprop file 'vc-state)
+ (and (not (vc-registered file)) 'unregistered)
(when (> (length file) 0) ;Why?? --Stef
(setq backend (or backend (vc-responsible-backend file)))
(when backend
- (vc-state-refresh file backend)))))
+ (vc-state-refresh file backend)))))
(defun vc-state-refresh (file backend)
"Quickly recompute the `state' of FILE."
@@ -494,11 +495,12 @@ vc-working-revision
"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-responsible-backend file)))
- (when backend
- (vc-file-setprop file 'vc-working-revision
- (vc-call-backend backend 'working-revision file))))))
+ (and (vc-registered file)
+ (progn
+ (setq backend (or backend (vc-responsible-backend file)))
+ (when backend
+ (vc-file-setprop file 'vc-working-revision
+ (vc-call-backend backend 'working-revision file)))))))
;; Backward compatibility.
(define-obsolete-function-alias
diff --git a/test/automated/vc-tests.el b/test/automated/vc-tests.el
index 2faa143..7f6196b 100644
--- a/test/automated/vc-tests.el
+++ b/test/automated/vc-tests.el
@@ -285,10 +285,9 @@ vc-test--state
(make-directory default-directory)
(vc-test--create-repo-function backend)
- ;; nil: Hg Mtn RCS
- ;; added: Git
- ;; unregistered: CVS SCCS SRC
- ;; up-to-date: Bzr SVN
+ ;; nil: RCS
+ ;; unregistered: Bzr CVS Git Hg Mtn SCCS SRC
+ ;; up-to-date: SVN
(message "vc-state1 %s" (vc-state default-directory))
(should (eq (vc-state default-directory)
(vc-state default-directory backend)))
@@ -298,51 +297,43 @@ vc-test--state
(let ((tmp-name (expand-file-name "foo" default-directory)))
;; Check state of an empty file.
- ;; nil: Hg Mtn SRC SVN
- ;; added: Git
- ;; unregistered: RCS SCCS
- ;; up-to-date: Bzr CVS
+ ;; unregistered: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
(message "vc-state2 %s" (vc-state tmp-name))
(should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
- (should (memq (vc-state tmp-name)
- '(nil added unregistered up-to-date)))
+ (should (eq (vc-state tmp-name) 'unregistered))
;; Write a new file. Check state.
(write-region "foo" nil tmp-name nil 'nomessage)
- ;; nil: Mtn
- ;; added: Git
- ;; unregistered: Hg RCS SCCS SRC SVN
- ;; up-to-date: Bzr CVS
+ ;; unregistered: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
(message "vc-state3 %s" (vc-state tmp-name))
(should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
- (should (memq (vc-state tmp-name)
- '(nil added unregistered up-to-date)))
+ (should (eq (vc-state tmp-name) 'unregistered))
;; Register a file. Check state.
(vc-register
(list backend (list (file-name-nondirectory tmp-name))))
- ;; added: Git Mtn
- ;; unregistered: Hg RCS SCCS SRC SVN
- ;; up-to-date: Bzr CVS
+ ;; nil: SRC
+ ;; added: Bzr CVS Git Hg Mtn
+ ;; unregistered: SVN
+ ;; up-to-date: RCS SCCS
(message "vc-state4 %s" (vc-state tmp-name))
(should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
- (should (memq (vc-state tmp-name) '(added unregistered up-to-date)))
+ (should (memq (vc-state tmp-name)
+ '(nil added unregistered up-to-date)))
;; Unregister the file. Check state.
(condition-case nil
(progn
(vc-test--unregister-function backend tmp-name)
- ;; added: Git
- ;; unregistered: Hg RCS
+ ;; added: Bzr Git Hg
;; unsupported: CVS Mtn SCCS SRC SVN
- ;; up-to-date: Bzr
+ ;; up-to-date: RCS
(message "vc-state5 %s" (vc-state tmp-name))
(should (eq (vc-state tmp-name) (vc-state tmp-name backend)))
- (should (memq (vc-state tmp-name)
- '(added unregistered up-to-date))))
+ (should (memq (vc-state tmp-name) '(added up-to-date))))
(vc-not-supported (message "vc-state5 unsupported")))))
;; Save exit.
@@ -370,8 +361,8 @@ vc-test--working-revision
(make-directory default-directory)
(vc-test--create-repo-function backend)
- ;; nil: CVS Git Mtn RCS SCCS
- ;; "0": Bzr Hg SRC SVN
+ ;; nil: Bzr CVS Git Hg Mtn RCS SCCS SRC
+ ;; "0": SVN
(message
"vc-working-revision1 %s" (vc-working-revision default-directory))
(should (eq (vc-working-revision default-directory)
@@ -382,33 +373,32 @@ vc-test--working-revision
;; Check initial working revision, should be nil until
;; it's registered.
- ;; nil: CVS Git Mtn RCS SCCS SVN
- ;; "0": Bzr Hg SRC
+ ;; nil: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
(message "vc-working-revision2 %s" (vc-working-revision tmp-name))
(should (eq (vc-working-revision tmp-name)
(vc-working-revision tmp-name backend)))
- (should (member (vc-working-revision tmp-name) '(nil "0")))
+ (should-not (vc-working-revision tmp-name))
;; Write a new file. Check working revision.
(write-region "foo" nil tmp-name nil 'nomessage)
- ;; nil: CVS Git Mtn RCS SCCS SVN
- ;; "0": Bzr Hg SRC
+ ;; nil: Bzr CVS Git Hg Mtn RCS SCCS SRC SVN
(message "vc-working-revision3 %s" (vc-working-revision tmp-name))
(should (eq (vc-working-revision tmp-name)
(vc-working-revision tmp-name backend)))
- (should (member (vc-working-revision tmp-name) '(nil "0")))
+ (should-not (vc-working-revision tmp-name))
;; Register a file. Check working revision.
(vc-register
(list backend (list (file-name-nondirectory tmp-name))))
- ;; nil: Mtn Git RCS SCCS
+ ;; nil: Git Mtn
;; "0": Bzr CVS Hg SRC SVN
+ ;; "1.1": RCS SCCS
(message "vc-working-revision4 %s" (vc-working-revision tmp-name))
(should (eq (vc-working-revision tmp-name)
(vc-working-revision tmp-name backend)))
- (should (member (vc-working-revision tmp-name) '(nil "0")))
+ (should (member (vc-working-revision tmp-name) '(nil "0" "1.1")))
;; Unregister the file. Check working revision.
(condition-case nil
@@ -417,12 +407,13 @@ vc-test--working-revision
;; nil: Git RCS
;; "0": Bzr Hg
+ ;; "1.1": RCS
;; unsupported: CVS Mtn SCCS SRC SVN
(message
"vc-working-revision5 %s" (vc-working-revision tmp-name))
(should (eq (vc-working-revision tmp-name)
(vc-working-revision tmp-name backend)))
- (should (member (vc-working-revision tmp-name) '(nil "0"))))
+ (should (member (vc-working-revision tmp-name) '(nil "0" "1.1"))))
(vc-not-supported (message "vc-working-revision5 unsupported")))))
;; Save exit.
next prev parent reply other threads:[~2016-04-13 15:14 UTC|newest]
Thread overview: 41+ messages / expand[flat|nested] mbox.gz Atom feed top
2015-05-23 23:49 bug#20637: incompatible, undocumented change to vc-working-revision Glenn Morris
2016-03-28 23:28 ` Dmitry Gutov
2016-03-29 18:13 ` Michael Albinus
2016-04-01 0:36 ` Dmitry Gutov
2016-04-09 19:34 ` Michael Albinus
2016-04-09 20:42 ` Dmitry Gutov
2016-04-10 8:00 ` Michael Albinus
2016-04-10 16:00 ` Dmitry Gutov
2016-04-10 18:09 ` Michael Albinus
2016-04-10 18:58 ` Dmitry Gutov
2016-04-11 6:55 ` Michael Albinus
2016-04-13 20:55 ` Dmitry Gutov
2016-04-14 7:10 ` Michael Albinus
2016-04-14 13:53 ` Dmitry Gutov
2016-04-14 15:26 ` Michael Albinus
2016-04-15 0:33 ` Dmitry Gutov
2016-04-15 13:13 ` Michael Albinus
2016-04-14 15:23 ` Eli Zaretskii
2016-04-13 15:14 ` Michael Albinus [this message]
2016-04-13 20:49 ` Dmitry Gutov
2016-04-14 7:21 ` Michael Albinus
2016-04-14 14:20 ` Dmitry Gutov
2016-04-14 18:31 ` Michael Albinus
2016-04-15 0:20 ` Dmitry Gutov
2016-04-15 13:11 ` Michael Albinus
2016-04-17 0:44 ` Dmitry Gutov
2016-04-18 12:27 ` Michael Albinus
2016-04-18 12:33 ` Dmitry Gutov
2016-04-18 12:46 ` Michael Albinus
2016-04-18 1:40 ` Dmitry Gutov
2016-04-15 1:01 ` Dmitry Gutov
2016-04-15 1:04 ` Dmitry Gutov
2016-04-15 13:23 ` Michael Albinus
2016-04-17 0:27 ` Dmitry Gutov
2016-04-18 1:33 ` Dmitry Gutov
2016-04-18 12:28 ` Michael Albinus
2016-04-18 12:37 ` Dmitry Gutov
2016-04-18 12:53 ` Michael Albinus
2016-04-18 12:58 ` Dmitry Gutov
2016-04-18 13:06 ` Michael Albinus
2016-04-18 16:34 ` John Wiegley
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=87oa9dzgl0.fsf@gmx.de \
--to=michael.albinus@gmx.de \
--cc=20637@debbugs.gnu.org \
--cc=dgutov@yandex.ru \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).