Re: git-handler.el

[Michael Albinus <michael.albinus@gmx.de>]

Jonas Bernoulli <jonas@bernoul.li> writes:

Hi Jonas,

>> A revisioned filename is something like "/path/to/file@@revision".
>> "revision" could be a revision like "81656add81", a branch like
>> "scratch/kqueue", or a tag like "emacs-19.34". Of course, the syntax
>> could be changed.
>
> I would prefer /path/to/repo/@git:REVISION:path/to/file.  I don't have
> a strong opinion about what the magic cookie should look like, but I
> think it should be inserted at the root of the working tree.

No, we are speaking about file name handlers. So we must give the syntax
a chance to address just one file. As Eli said, think about other VCSes,
like CVS. Your syntax wouldn't match.

I believe we should not use the VCS name, like "git", in the file name
syntax. There is no need for it, because we could offer a variable
`vc-handler-backend' or so, in which a user could tell us the preferred
backend. It could be one of the values of `vc-handled-backends' like
`Git', or it could be something different like the symbol `Magit'.

If this variable is declared directory-local at project root, the user
don't need to think about ever. If that variable is nil, the fallback
would be the vc mechanism via the `vc-responsible-backend' call, as I
have already implemented. Or something else.

I would also recommend to avoid using ":" in the file name. Although we
haven't the case yet, I fear that we would clash some day with Tramp
file name syntax.

So I'm back to the syntax "/path/to/file@@<backend specific>" or
"/path/to/directory@@<backend specific>". Note the /path/to/directory
case I haven't mentioned before, and which isn't implemented yet in the
code. There's no need to use the slash prior the cookie @@, because this
will be detected by ordinary file operations. About the
/path/to/directory@@... semantics more below.

>> vc-handler.el is the common part. There is the alist
>> `vc-file-name-handler-alist', which lists for every magic file name
>> function the responsible handler function. The majority of them is also
>> implemented in vc-handler.el, because they don't need any vcs specific
>> handling.
>>
>> For every different backend, there could be a respective backend
>> package. I've implemented vc-git-handler.el, because I know more about
>> vc-git than magit. But there's no problem to implement vc-magit.el, for
>> example. I plan also to write at least vc-cvs.el.
>
> As I said I haven't looked at the code much yet, so I don't know what
> the implications of having alternative handlers for the same vc would
> be.  But I fear that it would reduce interoperability.  But packages
> other than VC should be able to customize the behavior to some extend
> and that could probably be implemented using a few hooks and *-function
> variables.

I don't see yet what you have in mind. Implementing all magic file name
operations is sufficient in my understanding. vc-handler.el shall be
the common part for all VCSes, and vc-<backend>-handler.el shall be the
backend specific part. Beside the `vc-responsible-backend' call I've
mentioned above, there is no vc*.el functionality used in
vc-handler.el. Maybe the name is misleading here.

I have established a mechanism, allowing backend specific handlers to
overrule common ones. `vc-git-handle-file-attributes' takes priority
over `vc-handle-file-attributes' in my implementation. So could
(vc-)magit-handle-* functions.

>> You might play a little bit to see how it looks like. Maybe the most
>> simple start is to enter dired, because it uses many of the magic file
>> name operations. Just do "C-x d ~/src/emacs/src/emacs.c@@" (supposed
>> your Emacs git is located at ~/src/emacs, as in my case).
>
> Speaking of Dired, trees (directories) should be supported in addition
> to blobs (files), among other things that would allow visiting them in
> Dired.

Dired itself doesn't matter too much for the file name handler
business. It is just a proper UI in order to explore the possibilities.

In my previous mail I have described how a file oriented view would
look like. Take the Emacs source tree, and file
lisp/display-line-numbers.el as example (I take it, because it has a
short commit history :-) In dired, there exists then

--8<---------------cut here---------------start------------->8---
  /home/albinus/src/emacs/lisp/display-line-numbers.el@@:
  total 12
  lr--r--r--  1 Paul Eggert     UNKNOWN 3922 08-03 04:46 emacs23 -> a8a81df8da
  lr--r--r--  1 Michael Albinus UNKNOWN 3922 08-01 10:13 HEAD -> master
  dr-xr-xr-x  1 Michael Albinus UNKNOWN 3922 08-01 10:13 master

  /home/albinus/src/emacs/lisp/display-line-numbers.el@@/master:
  total 23
  -r--r--r--  1 Michael Albinus   UNKNOWN 3846 07-23 09:28 012487bc41
  -r--r--r--  1 Mark Oteiza       UNKNOWN 3937 07-25 02:17 32daa3cb54
  -r--r--r--  1 Alexander Gramiak UNKNOWN 3831 07-22 11:16 ebb78a7bfa
  -r--r--r--  1 Michael Albinus   UNKNOWN 3922 08-01 10:13 ef7a18a071
  -r--r--r--  1 Mark Oteiza       UNKNOWN 3960 07-23 21:41 f57c710772
  lr--r--r--  1 Michael Albinus   UNKNOWN 3922 08-01 10:13 HEAD -> 81e22163eb
--8<---------------cut here---------------end--------------->8---

The file belongs only to the master branch so far; if it would belong to
other branches, there would be respective subdirectories. And there is
only one label on that file so far, emacs23. (Btw, where does it come from?)

A revision oriented view would start with a directory, and not with a
file. Let's use lisp/, and revision ef7a18a071

--8<---------------cut here---------------start------------->8---
  /home/albinus/src/emacs/lisp@@:
  total 12
  lr--r--r--  1 Paul Eggert     UNKNOWN 3922 08-03 04:46 emacs23 -> a8a81df8da
  lr--r--r--  1 Michael Albinus UNKNOWN 3922 08-01 10:13 HEAD -> master
  dr-xr-xr-x  1 Michael Albinus UNKNOWN 3922 08-01 10:13 master
  [... Other branches] 
  dr-xr-xr-x  1 Michael Albinus   UNKNOWN 3922 08-01 10:13 ef7a18a071
  [... Other revisions] 

  /home/albinus/src/emacs/lisp@@/ef7a18a071:
  total 23
  -r--r--r--  1 Michael Albinus   UNKNOWN   3922 08-01 10:13 display-line-numbers.el
  -r--r--r--  1 Michael Albinus   UNKNOWN 109714 08-01 10:13 menu-bar.el
--8<---------------cut here---------------end--------------->8---

(This example I have written manually, it is not implemented yet).

You see the difference. In the file oriented view, we have
"/home/albinus/src/emacs/lisp/display-line-numbers.el@@/master" being a
directory, containing all revisions and labels as regulary files.

In the revision oriented view, we have
"/home/albinus/src/emacs/lisp@@/ef7a18a071" being a directory,
containing the files which have been modified by this commit.

This is just the basic idea, and it would work for other backends as
well. For CVS, the revision oriented view would just offer one file for
a given commit, and likely several files for a given label (which is a
directory or a symlink to a directory).

>> Both packages are far from being complete. Performance is terrible (a
>> proper cache mechanism is needed),
>
> In what way is performance terrible?  I would have expected that asking
> git for information about one blob would not be that much more expensive
> than asking the file-system for the same information about one file.
> (I.e. the the difference would only become relevant if you needed
> information about many files.)

For one blob it might perform better. But I have implemented the file
oriented view first, which asks information for every file containing to
a blob again and again. `vc-git-handle-file-attributes', for example,
raises two process calls. Much place for improvement, of course.

> What happens when you visit say HEAD:file and then HEAD is updated?  I
> think this should behave much the same as for files that are modified on
> disk.  The user could then use `revert-buffer' to update the buffer.

Yep. See the comment in vc-git-handler.el:

;; TODO: We shall add also functions to expire the caches.  Best would
;; be file notification, which watches respective git files (indexes).

Once the cache expires the information of a file, let's say the
`file-attributes' information, new information is retrieved next time
`file-attributes' is called. Emacs will warn you then that the buffer
contents is not current. You could revert.

> You mentioned in another message that this is read-only.  While that's a
> good default, I think it should be possible for Magit or something else
> to provide a `modified-p' and a `save-file' function by setting some
> `*-function' variables.

I don't understand. What do you need, which is not handled by basic file
operations, like `verify-visited-file-modtime'?

> It would help me and others that are not very familiar with VC's
> internals to understand the file-handler parts if you could create one
> commit that implements those parts without taking advantage of any
> caching provided by VC and then add that caching in a separate commit.

I have no plan yet for committing something, because I don't know where
it shall go. Emacs core? ELPA?

>   Cheers,
>   Jonas
>
> Ps: I'll probably be unavailable for a few days.

No problem, take your time.

Best regards, Michael.