Patch to make vc-hg.el work with remote files

[Bernhard Herzog-3]

Hi,

vc-hg.el doesn't work properly with remote files accessed via tramp.  Emacs
doesn't even recognize correctly whether files are managed by hg in those
cases.  The patch below fixes this for me.

Regards,

   Bernhard


Index: lisp/vc-hg.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-hg.el,v
retrieving revision 1.104
diff -c -u -r1.104 vc-hg.el
--- lisp/vc-hg.el 19 Oct 2009 05:04:28 -0000 1.104
+++ lisp/vc-hg.el 25 Oct 2009 17:52:46 -0000
@@ -159,6 +159,7 @@
   "Hg-specific version of `vc-state'."
   (let*
       ((status nil)
+       (default-directory (file-name-directory file))
        (out
         (with-output-to-string
           (with-current-buffer
@@ -166,8 +167,8 @@
             (setq status
                   (condition-case nil
                       ;; Ignore all errors.
-                      (call-process
-                       "hg" nil t nil "--cwd" (file-name-directory file)
+                      (process-file
+                       "hg" nil t nil
                        "status" "-A" (file-name-nondirectory file))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
@@ -190,6 +191,7 @@
   "Hg-specific version of `vc-working-revision'."
   (let*
       ((status nil)
+       (default-directory (file-name-directory file))
        (out
         (with-output-to-string
           (with-current-buffer
@@ -197,8 +199,8 @@
             (setq status
                   (condition-case nil
                       ;; Ignore all errors.
-                      (call-process
-                       "hg" nil t nil "--cwd" (file-name-directory file)
+                      (process-file
+                       "hg" nil t nil
                        "log" "-l1" (file-name-nondirectory file))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
@@ -286,7 +288,6 @@
       (setq oldvers working))
     (apply #'vc-hg-command (or buffer "*vc-diff*") nil
            (mapcar (lambda (file) (file-relative-name file cwd)) files)
-           "--cwd" cwd
            "diff"
            (append
             (vc-switches 'hg 'diff)

[Stefan Monnier]

> +                      (process-file
> +                       "hg" nil t nil
>                         "status" "-A" (file-name-nondirectory file))

The canonical way to use process-file is to pass args of the form
(file-relative-name).


        Stefan

[Bernhard Herzog-3]

On 26.10.2009, Stefan Monnier wrote:
> > +                      (process-file
> > +                       "hg" nil t nil
> >                         "status" "-A" (file-name-nondirectory file))
>
> The canonical way to use process-file is to pass args of the form
> (file-relative-name).

Indeed.  Here's a new patch.

Regards,

    Bernhard


Index: lisp/vc-hg.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-hg.el,v
retrieving revision 1.104
diff -u -r1.104 vc-hg.el
--- lisp/vc-hg.el 19 Oct 2009 05:04:28 -0000 1.104
+++ lisp/vc-hg.el 26 Oct 2009 23:33:04 -0000
@@ -159,6 +159,7 @@
   "Hg-specific version of `vc-state'."
   (let*
       ((status nil)
+       (default-directory (file-name-directory file))
        (out
         (with-output-to-string
           (with-current-buffer
@@ -166,9 +167,9 @@
             (setq status
                   (condition-case nil
                       ;; Ignore all errors.
-                      (call-process
-                       "hg" nil t nil "--cwd" (file-name-directory file)
-                       "status" "-A" (file-name-nondirectory file))
+                      (process-file
+                       "hg" nil t nil
+                       "status" "-A" (file-relative-name file))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
                     (error nil)))))))
@@ -190,6 +191,7 @@
   "Hg-specific version of `vc-working-revision'."
   (let*
       ((status nil)
+       (default-directory (file-name-directory file))
        (out
         (with-output-to-string
           (with-current-buffer
@@ -197,9 +199,9 @@
             (setq status
                   (condition-case nil
                       ;; Ignore all errors.
-                      (call-process
-                       "hg" nil t nil "--cwd" (file-name-directory file)
-                       "log" "-l1" (file-name-nondirectory file))
+                      (process-file
+                       "hg" nil t nil
+                       "log" "-l1" (file-relative-name file))
                     ;; Some problem happened.  E.g. We can't find an `hg'
                     ;; executable.
                     (error nil)))))))
@@ -286,7 +288,6 @@
       (setq oldvers working))
     (apply #'vc-hg-command (or buffer "*vc-diff*") nil
            (mapcar (lambda (file) (file-relative-name file cwd)) files)
-           "--cwd" cwd
            "diff"
            (append
             (vc-switches 'hg 'diff)

[Stefan Monnier]

> Indeed.  Here's a new patch.

Thanks, installed.  But I wonder about the last hunk of your patch:

> @@ -286,7 +288,6 @@
>        (setq oldvers working))
>      (apply #'vc-hg-command (or buffer "*vc-diff*") nil
>             (mapcar (lambda (file) (file-relative-name file cwd)) files)
> -           "--cwd" cwd
>             "diff"
>             (append
>              (vc-switches 'hg 'diff)

What is the reason for it?


        Stefan

[Bernhard Herzog-3]

On 28.10.2009, Stefan Monnier wrote:
> > Indeed.  Here's a new patch.
>
> Thanks, installed.

Great!

> But I wonder about the last hunk of your patch:
> > @@ -286,7 +288,6 @@
> >        (setq oldvers working))
> >      (apply #'vc-hg-command (or buffer "*vc-diff*") nil
> >             (mapcar (lambda (file) (file-relative-name file cwd)) files)
> > -           "--cwd" cwd
> >             "diff"
> >             (append
> >              (vc-switches 'hg 'diff)
>
> What is the reason for it?

The reason I changed vc-hg-diff at all was that it doesn't work correctly with
tramp.  cwd is the name of the directory containing the file to diff and if
file is on a remote system it's a tramp filename which mercurial doesn't
understand, of course.

I've looked a bit closer at it now and AFAICT vc-hg-diff can be made even
simpler and probably more correct by passing files unmodified to
vc-hg-command.  I don't have the time right now to test and produce a better
patch, though. I hope to get around to that tomorrow.

Does anybody know why in vc-hg.el vc-hg-command is sometimes called directly
and sometimes via apply?

Regards

   Bernhard

[Bernhard Herzog-3]

On 28.10.2009, Bernhard Herzog wrote:
> I've looked a bit closer at it now and AFAICT vc-hg-diff can be made even
> simpler and probably more correct by passing files unmodified to
> vc-hg-command.  I don't have the time right now to test and produce a
> better patch, though. I hope to get around to that tomorrow.

It took a while longer, but here's the new patch:

Index: lisp/vc-hg.el
===================================================================
RCS file: /sources/emacs/emacs/lisp/vc-hg.el,v
retrieving revision 1.105
diff -u -r1.105 vc-hg.el
--- lisp/vc-hg.el 28 Oct 2009 02:05:50 -0000 1.105
+++ lisp/vc-hg.el 2 Nov 2009 21:16:15 -0000
@@ -279,16 +279,12 @@
 (defun vc-hg-diff (files &optional oldvers newvers buffer)
   "Get a difference report using hg between two revisions of FILES."
   (let* ((firstfile (car files))
-         (cwd (if firstfile (file-name-directory firstfile)
-                (expand-file-name default-directory)))
          (working (and firstfile (vc-working-revision firstfile))))
     (when (and (equal oldvers working) (not newvers))
       (setq oldvers nil))
     (when (and (not oldvers) newvers)
       (setq oldvers working))
-    (apply #'vc-hg-command (or buffer "*vc-diff*") nil
-           (mapcar (lambda (file) (file-relative-name file cwd)) files)
-           "diff"
+    (apply #'vc-hg-command (or buffer "*vc-diff*") nil files "diff"
            (append
             (vc-switches 'hg 'diff)
             (when oldvers

regards

  Bernhard

[Dan Nicolaescu]

Bernhard Herzog <bernhard.herzog@...> writes:

  > On 28.10.2009, Bernhard Herzog wrote:
  > > I've looked a bit closer at it now and AFAICT vc-hg-diff can be made even
  > > simpler and probably more correct by passing files unmodified to
  > > vc-hg-command.  I don't have the time right now to test and produce a
  > > better patch, though. I hope to get around to that tomorrow.
  >
  > It took a while longer, but here's the new patch:

Was this tested with filesets that contain: multiple files, multiple
directories and a mix of files and directories.  And starting
from a subdirectory of the Hg root?
The previous change caused this regression:
 http://permalink.gmane.org/gmane.emacs.devel/116585
does this patch fix it?

  > Index: lisp/vc-hg.el
  > ===================================================================
  > RCS file: /sources/emacs/emacs/lisp/vc-hg.el,v
  > retrieving revision 1.105
  > diff -u -r1.105 vc-hg.el
  > --- lisp/vc-hg.el 28 Oct 2009 02:05:50 -0000 1.105
  > +++ lisp/vc-hg.el 2 Nov 2009 21:16:15 -0000
  > @@ -279,16 +279,12 @@
  >  (defun vc-hg-diff (files &optional oldvers newvers buffer)
  >    "Get a difference report using hg between two revisions of FILES."
  >    (let* ((firstfile (car files))
  > -         (cwd (if firstfile (file-name-directory firstfile)
  > -                (expand-file-name default-directory)))
  >           (working (and firstfile (vc-working-revision firstfile))))
  >      (when (and (equal oldvers working) (not newvers))
  >        (setq oldvers nil))
  >      (when (and (not oldvers) newvers)
  >        (setq oldvers working))
  > -    (apply #'vc-hg-command (or buffer "*vc-diff*") nil
  > -           (mapcar (lambda (file) (file-relative-name file cwd)) files)
  > -           "diff"
  > +    (apply #'vc-hg-command (or buffer "*vc-diff*") nil files "diff"
  >             (append
  >              (vc-switches 'hg 'diff)
  >              (when oldvers
  >
  > regards
  >
  >   Bernhard

[Sam Steingold]

Stefan Monnier wrote:
>> Indeed.  Here's a new patch.
>
> Thanks, installed.  But I wonder about the last hunk of your patch:

this broke diff:
vc-hg.el revision 1.105 is broken: "=" in *vc-dir* now results in
"<filename>: No such file or directory"
in *vc-diff* (at least when the filename is located in a subdir, not at the top
level directory).

[Bernhard Herzog-2]

Dan Nicolaescu <dann@...> writes:

> Bernhard Herzog <bernhard.herzog@...> writes:
>
>   > On 28.10.2009, Bernhard Herzog wrote:
>   > > I've looked a bit closer at it now and AFAICT vc-hg-diff can be made even
>   > > simpler and probably more correct by passing files unmodified to
>   > > vc-hg-command.  I don't have the time right now to test and produce a
>   > > better patch, though. I hope to get around to that tomorrow.
>   >
>   > It took a while longer, but here's the new patch:
>
> Was this tested with filesets that contain: multiple files, multiple
> directories and a mix of files and directories.  And starting
> from a subdirectory of the Hg root?
> The previous change caused this regression:
>  http://permalink.gmane.org/gmane.emacs.devel/116585
> does this patch fix it?

I've tested various combinations of files and directories from vc-dir
and some commands (e.g. C-c v =) from within a buffer visiting a file
managed by mercurial.  They all worked correctly.  I may have missed
some cases, though.  I was able to reproduce the regression reported in
the mail you referred to, but with my patch it's fixed.

Regards

   Bernhard