GNU bug report logs - #20637
incompatible, undocumented change to vc-working-revision

Previous Next

Full log

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

From: Michael Albinus <michael.albinus <at> gmx.de>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: Glenn Morris <rgm <at> gnu.org>, 20637 <at> debbugs.gnu.org
Subject: Re: bug#20637: incompatible,
 undocumented change to vc-working-revision
Date: Wed, 13 Apr 2016 17:14:19 +0200

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

Dmitry Gutov <dgutov <at> 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.

[diff (text/x-patch, attachment)]

Previous Next

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