From mboxrd@z Thu Jan 1 00:00:00 1970 Path: news.gmane.org!not-for-mail From: Michael Albinus Newsgroups: gmane.emacs.bugs Subject: bug#20637: incompatible, undocumented change to vc-working-revision Date: Wed, 13 Apr 2016 17:14:19 +0200 Message-ID: <87oa9dzgl0.fsf@gmx.de> References: <6ok2vyzwf9.fsf@fencepost.gnu.org> <08f70cda-44be-0657-e50a-2b2c80d2c21c@yandex.ru> NNTP-Posting-Host: plane.gmane.org Mime-Version: 1.0 Content-Type: multipart/mixed; boundary="=-=-=" X-Trace: ger.gmane.org 1460560530 4238 80.91.229.3 (13 Apr 2016 15:15:30 GMT) X-Complaints-To: usenet@ger.gmane.org NNTP-Posting-Date: Wed, 13 Apr 2016 15:15:30 +0000 (UTC) Cc: 20637@debbugs.gnu.org To: Dmitry Gutov Original-X-From: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Wed Apr 13 17:15:19 2016 Return-path: Envelope-to: geb-bug-gnu-emacs@m.gmane.org Original-Received: from lists.gnu.org ([208.118.235.17]) by plane.gmane.org with esmtp (Exim 4.69) (envelope-from ) id 1aqMVi-0006HZ-3N for geb-bug-gnu-emacs@m.gmane.org; Wed, 13 Apr 2016 17:15:18 +0200 Original-Received: from localhost ([::1]:45622 helo=lists.gnu.org) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqMVh-0000mj-C3 for geb-bug-gnu-emacs@m.gmane.org; Wed, 13 Apr 2016 11:15:17 -0400 Original-Received: from eggs.gnu.org ([2001:4830:134:3::10]:52100) by lists.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqMVX-0000ZL-B6 for bug-gnu-emacs@gnu.org; Wed, 13 Apr 2016 11:15:14 -0400 Original-Received: from Debian-exim by eggs.gnu.org with spam-scanned (Exim 4.71) (envelope-from ) id 1aqMVS-00046E-QG for bug-gnu-emacs@gnu.org; Wed, 13 Apr 2016 11:15:07 -0400 Original-Received: from debbugs.gnu.org ([208.118.235.43]:49728) by eggs.gnu.org with esmtp (Exim 4.71) (envelope-from ) id 1aqMVS-000468-N1 for bug-gnu-emacs@gnu.org; Wed, 13 Apr 2016 11:15:02 -0400 Original-Received: from Debian-debbugs by debbugs.gnu.org with local (Exim 4.84_2) (envelope-from ) id 1aqMVS-0003zY-6W; Wed, 13 Apr 2016 11:15:02 -0400 X-Loop: help-debbugs@gnu.org Resent-From: Michael Albinus Original-Sender: "Debbugs-submit" Resent-CC: bug-gnu-emacs@gnu.org, Dmitry Gutov Resent-Date: Wed, 13 Apr 2016 15:15:02 +0000 Resent-Message-ID: Resent-Sender: help-debbugs@gnu.org X-GNU-PR-Message: followup 20637 X-GNU-PR-Package: emacs X-GNU-PR-Keywords: Original-Received: via spool by 20637-submit@debbugs.gnu.org id=B20637.146056047215288 (code B ref 20637); Wed, 13 Apr 2016 15:15:02 +0000 Original-Received: (at 20637) by debbugs.gnu.org; 13 Apr 2016 15:14:32 +0000 Original-Received: from localhost ([127.0.0.1]:33832 helo=debbugs.gnu.org) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aqMUx-0003yW-SA for submit@debbugs.gnu.org; Wed, 13 Apr 2016 11:14:32 -0400 Original-Received: from mout.gmx.net ([212.227.15.19]:55879) by debbugs.gnu.org with esmtp (Exim 4.84_2) (envelope-from ) id 1aqMUv-0003yG-7X for 20637@debbugs.gnu.org; Wed, 13 Apr 2016 11:14:30 -0400 Original-Received: from detlef.gmx.de ([87.146.56.228]) by mail.gmx.com (mrgmx002) with ESMTPSA (Nemesis) id 0MC4VE-1az7wD1FSa-008qbF; Wed, 13 Apr 2016 17:14:21 +0200 In-Reply-To: <08f70cda-44be-0657-e50a-2b2c80d2c21c@yandex.ru> (Dmitry Gutov's message of "Tue, 29 Mar 2016 02:28:22 +0300") User-Agent: Gnus/5.13 (Gnus v5.13) Emacs/25.1.50 (gnu/linux) X-Provags-ID: V03:K0:6MsITz5fvEVp3sl7N/DPB58SEg1E/nDbfjT19V/NQpXUfGK7mVH Oo32X7uqDzyI33gIGaNwmv9jnbtcIDQe6yUavImQ4DsCK++0SNaesSlJzAJrDTFA+xYIG5m D6irjRbUKomeF0CJKb8SQ601KJw9ztEkF/Yfizxa7v4QsLVFyFg4Q4oURFjNZD4Ue1xozJT mkazVzwn3xYEhq3AIa9lg== X-UI-Out-Filterresults: notjunk:1;V01:K0:czWP7Q8vm6k=:5S6J71tRvJawk1Ta48WLg4 deJKxUD5IRhdb0BWkSCMo6ss4shMINx7dT5a+K092xWtCtsVA9cTuVYd9ymwONpOQhOBvGuyX ps+ijoAvwFOqjfVJQPgJ1N9yq3iUVNGW7lFnRbf3LkqzQdD3OaqED0tGxqLYVzU4b2HDxyEA3 cMzdQBh452JpMQH8LUQPo0qJ4RKQ5JEaPOSZjw78QAns/6GVBY0SEWaRj8J7m56WGPahVjaM1 z7ZZTptRws4cXr78+z0PpJ+08gkyuZdBBpEz0l2IGeldU8vwjmpKQYqV7U6JXc/YfdhxAtk9J 0GNTGdUHZt06pFOcoYIabeJ79oSI7bxUtGejYL94Db2qhJgGIK7wJxfli12jVTfHuE4Qp2ED4 +0iWMWTINWM+B4udF+W0pYlnDFCIvA2ypUZ/9ZtWradKdFlkJ6251nbbzCJsaHKIcW9OjJJhF nL4rdBX4RsdkOFHwlvuwDBdwEAr7HjoW++7cObEB61gcTe/oTIBbXb47NEVlkA1H9z0Goxq3w uah7DusxUfHhwwpGxYKvUYGRiFKm15JivuvGRjBg5PMgr21PopQR9X//GvqhE2jG4QiUobsFp 5tKqA/iFx0dNInHn/bYxsdl2f9wExllYwKi+DNX0HciNO+yta/ft+iX33Ka+gdXjd3Ctk7Lh8 HJ/6O3rKroF3fV5a3D9CvWi69TaH6F4y3TCyk4Dav7EkHFDPDlWpHtFpkalLbrApke1pbGkoi Doix8XPviG+rwVcS6+zlbke2+Rxx+eJU9+S9a1extzEzUOVsBF8bnJBCHXw= X-BeenThere: debbugs-submit@debbugs.gnu.org X-Mailman-Version: 2.1.18 Precedence: list X-detected-operating-system: by eggs.gnu.org: GNU/Linux 2.2.x-3.x [generic] X-Received-From: 208.118.235.43 X-BeenThere: bug-gnu-emacs@gnu.org List-Id: "Bug reports for GNU Emacs, the Swiss army knife of text editors" List-Unsubscribe: , List-Archive: List-Post: List-Help: List-Subscribe: , Errors-To: bug-gnu-emacs-bounces+geb-bug-gnu-emacs=m.gmane.org@gnu.org Original-Sender: "bug-gnu-emacs" Xref: news.gmane.org gmane.emacs.bugs:116436 Archived-At: --=-=-= Content-Type: text/plain Dmitry Gutov 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. --=-=-= Content-Type: text/x-patch Content-Disposition: attachment; filename=diff 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. --=-=-=--