bug#34350: 27.0.50; ediff-revision broken with SVN backend + non ascii chars both in directory and in filename

[Stefan Monnier]

>> The `find-revision` backend operation must put the revision's bytes into
>> the provided buffer, indeed similarly to when we visit a file.
>> But the difference is that in 99% of the cases, the bytes don't come
>> from a file but from a process's stdout, so the backed can't directly
>> use the "visit a file" trick.
> Isn't that sub-optimal design, then?  Those back-ends that can produce
> a file for a given revision (there are at least 3 of them, AFAIK)
> should be left to their devices; those which cannot and use some kind
> of 'cat' command instead can be invoked with output redirected to a
> file.

FWIW, I find it cleaner to put the result in a buffer than in a file,
since with files we have to choose a file name, which implies things
like race conditions, accessrights/security issues, interaction with
Tramp, and whatnot.

So I think the API seems clean enough.  Doesn't insert-file-contents do
all the decoding dance done in find-file (such as
auto-coding-regexp-alist, ...)?

> Instead, we invoke the VCS with output redirected to a pipe, slowly
> read that output from the pipe into a buffer,

Why should reading from a pipe be slower than from a file?

> then write the resulting buffer to a file.  Why?

The front-end part may want to do that, but not necessarily.
We did that back in the CVS days because it took a long time to extract
a particular revision, so this was a way to cache the result.

With Git, I find that I basically never want to save the result to
a file.

>> - vc-find-revision-save
>> - vc-find-revision-no-save
>> - vc-default-revert
>> 
>> The last one should indeed call the backend directly (as it currently
>> does) and should be changed not to bind coding-system-for-read/write and
>> instead to assume that the backend deals with bytes.
>> 
>> The other two are begging to be unified to reduce code redundancy and
>> are the ones that need the do the file-like decoding (and they indeed do
>> it).
>
> If vc-find-revision and vc-find-revision-no-save need to enforce
> no-conversion, then why do most of the back-ends do that as well?

The front-end functions (vc-find-revision-* and vc-default-revert)
should not bind coding-system-for-read/write and should leave that to
the backend, since it seems the backends need to do this kind of
coding-system control more finely.

> If we decide that back-ends produce undecoded buffers, then vc.el
> shouldn't be bothered with forcing coding-system-for-read/write at
> all, right?

Right.

> In addition, while I could understand binding of
> coding-system-for-read in the backend's find-revision (assuming we
> want the resulting buffer remain undecoded), why should the back-end
> also bind coding-system-for-write?  I see absolutely no reason for
> that.  E.g., look at this example:

I don't see a reason either, other than the coder not being sure which
of read/write influences which of send/receive, maybe.

>   (defun vc-hg-find-revision (file rev buffer)
>     (let ((coding-system-for-read 'binary)
>         (coding-system-for-write 'binary))
>       (if rev
>         (vc-hg-command buffer 0 file "cat" "-r" rev)
>       (vc-hg-command buffer 0 file "cat"))))
>
> Why on earth does this bind coding-system-for-write, when it doesn't
> write anything at all, it only reads?

Plain bug, AFAICT.

> But vc-git.el, for example, uses both in its find-revision
> implementation.  It therefore must use more complicated juggling with
> binding the various coding-system variables.  Once again, this is an
> argument in favor of leaving the encoding/decoding stuff to the
> back-end.

Agreed: it should be the backend's responsibility to control the
encoding/decoding setup in order for the result in the buffer to be
undecoded bytes (it'd be nice to be able to do it at only one spot
instead of doing it "by hand" in each and every backend, but IIUC this
is not an option).


        Stefan