bug#20637: incompatible, undocumented change to vc-working-revision

[Michael Albinus]

Dmitry Gutov <dgutov@yandex.ru> writes:

> Hi Michael,

Hi Dmitry,

>> you have written several things I would like to move for later
>> discussion. I believe, we shall start again from the basics.
>
> OK, but the questions seem tangential to this bug report, which is a
> blocker for 25.1 (whereas investigating how various commands should
> work, isn't).

If there is a simple and obvious fix for this problem, let apply it to
the emacs-25 branch. Even if we must change it later in master.

But I doubt that's possible. This bug report was written on 23 May 2015,
it was marked as release blocker on the same day, and still no fix. I
believe that there won't be a fix in emacs-25, and whatever we might
find on our way to stabilize vc, it will be too late and too much to be
applied to the emacs-25 branch.

For all those problems in vc I have the impression, that we must
stabilize vc first. Starting with the very basic functionality - that's
why I've added tests for vc-backend and vc-responsible-backend. Often,
there might be not an error, but missing or wrong documentation. As
Glenn has said also in bug#19548, another release blocker for 25.1.

I don't care whether we discuss it here, in bug#19548 or in the
emacs-devel ML. But at least *I* am not able to handle those issues
differently. And nobody but you and me seem to work on vc problems in
general.

>> I have extended vc-test-*01-register tests by calls to vc-backend and
>> vc-responsible-backend. Mainly in order to understand how they work, but
>> also for covering these functions. One problem I've found is that
>> vc-file-*prop functions do not work well for relative file names; I've
>> fixed this.
>
> The change looks good, but nevertheless seeing the commit 5e1c32e in
> master makes me worried about conflicts when merging the necessary
> future fix for this bug from emacs-25 to master.

As said, I doubt that we will find something which could go into
emacs-25. Of course I would be happy if I'm wrong :-)

>> Several problems I have marked with FIXME in the working horse of those
>> tests, vc-test--register:
>>
>> - For some backends (CVS, RCS and SVN), vc-backend returns the backend
>>   name for the newly created repo directory, and the directory is
>>   registered already. For the other backends, vc-backend returns nil as
>>   expected. Shouldn't this be consistent for all backends?
>
> I'm not quite clear on what you are saying here. If you're calling
> vc-backend on a directory, I believe the result is undefined. As in,
> the function is allowed to return any value. Maybe we could check
> file-directory-p in vc-backend, and signal an error if it is.

The docstring says nothing about directories, so the call I've applied
is legal. If vc-backend is not intended for directories we should
document it first. But ...

> For directories, one has to call vc-responsible-backend.

... I even doubt that directories are out of scope of vc-backend.
Directories can be registered for some VCS, for example for CVS, or for
ClearCase which I use at work (I know, we have no official support for
ClearCase in vc). Therefore we shall precise the docstring of
vc-backend, that it returns the backend also for directories for VCSes
which support rgistration of directories.

A pointer to vc-responsible-backend might also be helpful in the
docstring of vc-backend.

>> - vc-backend accepts also a list of files, vc-responsible-backend
>>   doesn't. Is this right?
>
> I suppose. The function signatures say so. But I don't see any callers
> of vc-backend that actually pass a list to it.

I know what the docstring says :-) My question is, whether we shall
offer the same argument list for both vc-backend and vc-responsible-backend.

>> - There is no common function vc-unregister, just some backend specific
>>   vc-<backend>-unregister.
>
> Those are the implementations of the `unregister' backend
> command. It's only used in vc-transfer-file currently.
>
>> Shouldn't vc-unregister exist?
>
> Maybe it should. Would you ever use it interactively?

Don't know. But I believe "interactively" is not the criterion whether a
general vc function shall exist. I also don't call vc-registered interactively.

>> It should call
>>   common code, like vc-file-clearprops. For the time being, I have
>>   emulated this.
>
> Are you doing that just to test the `unregister' implementations?

Yes. And I'm still in favor of adding vc-unregister in vc.el.

Best regards, Michael.