Rewrite of vc-user-login-name

Rewrite of vc-user-login-name

[Andre Spiegel]

Here's a patch that closes a long standing issue, removing the need for
Tramp to advise vc-user-login-name.  I'm not quite sure how to install
it, though: should I just remove the advice in tramp-vc.el?  Is that
guaranteed not to break anything if people simply update from CVS?
(Since the argument list changed, the tramp advice no longer works with
this patch.)

Apart from that, if anybody sees any issues with the new code, feel free
to suggest improvements.  If I don't hear anything, I'll install it and
simply remove the Tramp advice tomorrow.  (I have already installed a
related patch that makes `vc-user-login-name UID' unnecessary, hence the
change in the argument list.)

--- vc-hooks.el	20 Aug 2005 09:13:19 +0200	1.176
+++ vc-hooks.el	22 Jan 2006 13:51:14 +0100	
@@ -410,14 +410,22 @@
           (vc-file-setprop file 'vc-checkout-model
                            (vc-call checkout-model file)))))
 
-(defun vc-user-login-name (&optional uid)
-  "Return the name under which the user is logged in, as a string.
-\(With optional argument UID, return the name of that user.)
-This function does the same as function `user-login-name', but unlike
-that, it never returns nil.  If a UID cannot be resolved, that
-UID is returned as a string."
-  (or (user-login-name uid)
-      (number-to-string (or uid (user-uid)))))
+(defun vc-user-login-name (file)
+  "Return the name under which the user accesses the given FILE."
+  (or (and (eq (string-match tramp-file-name-regexp file) 0)
+           ;; tramp case: execute "whoami" via tramp
+           (let ((default-directory (file-name-directory file)))
+             (with-temp-buffer
+               (if (not (zerop (process-file "whoami" nil t)))
+                   ;; fall through if "whoami" didn't work
+                   nil
+                 ;; remove trailing newline
+                 (delete-region (1- (point-max)) (point-max))
+                 (buffer-string)))))
+      ;; normal case
+      (user-login-name)
+      ;; if user-login-name is nil, return the UID as a string
+      (number-to-string (user-uid))))
 
 (defun vc-state (file)
   "Return the version control state of FILE.

Re: Rewrite of vc-user-login-name

[Michael Albinus]

Andre Spiegel <spiegel@gnu.org> writes:

> Here's a patch that closes a long standing issue, removing the need for
> Tramp to advise vc-user-login-name.  I'm not quite sure how to install
> it, though: should I just remove the advice in tramp-vc.el?  Is that
> guaranteed not to break anything if people simply update from CVS?
> (Since the argument list changed, the tramp advice no longer works with
> this patch.)

I'll check the needed changes in tramp-vc.el next days. A simple
remove of the defadvice is not desirable, because Tramp is intended to
work with older Emacsen too.

Unfortunately, your patch came too late for the just released Tramp
2.0.52. But so what ...

Best regards, Michael.

Re: Rewrite of vc-user-login-name

[Richard M. Stallman]

    Here's a patch that closes a long standing issue, removing the need for
    Tramp to advise vc-user-login-name.  I'm not quite sure how to install
    it, though: should I just remove the advice in tramp-vc.el?

What is the cause of your doubts?
In general, updating the source is all one needs to do
for a change in the Lisp code.  Occasionally one needs
to update loaddefs.el, but that's not relevant here.

Re: Rewrite of vc-user-login-name

[Andre Spiegel]

On Tue, 2006-01-24 at 11:47 -0500, Richard M. Stallman wrote:

>     Here's a patch that closes a long standing issue, removing the need for
>     Tramp to advise vc-user-login-name.  I'm not quite sure how to install
>     it, though: should I just remove the advice in tramp-vc.el?
> 
> What is the cause of your doubts?
> In general, updating the source is all one needs to do
> for a change in the Lisp code.  Occasionally one needs
> to update loaddefs.el, but that's not relevant here.

I wasn't sure how the advice mechanism actually works, and I thought
that maybe my change could have forced people to recompile Emacs to make
it work.  Also, I'm not the maintainer of tramp-vc.el, so I thought I'd
better ask.

I will install the changes now and comment out the defadvice in
tramp-vc.el.  Michael Albinus can then have a closer look. 

Re: Rewrite of vc-user-login-name

[Michael Albinus]

Andre Spiegel <spiegel@gnu.org> writes:

> I will install the changes now and comment out the defadvice in
> tramp-vc.el.  Michael Albinus can then have a closer look. 

I've added a test for existence of `process-file' before activating
the defadvice. This will ensure it isn't activated for Emacs 22.1 (and
later), but it still cooperates with older Emacsen.

In `vc-user-login-name', I wouldn't recommend to check for
`tramp-file-name-regexp'; that's too specific. A suitable check should
be (file-remote-p file) instead of.

That check is used at other places in vc.el too, it could be replaced
also. And I'm even not sure that the restrictions for Tramp tested
there are still valid - at least for Tramp 2.1. they aren't. But this
discussion we should postpone until after the release, when Tramp
2.1. might find its way into the Emacs CVS.

Best regards, Michael.