threads / patch / 6056

patchfix vc git

Subject: [PATCH] fix vc git

## tl;dr

10 messages between Dec 21, 2006 and Dec 22, 2006. Diffs are folded; open one to read it.

replies: 9people: 4as markdown or json

Duncan Mak· Dec 21, 2006, 11:25 UTC · lore
Fix vc-git.el to not cd into a non-existent directory.
---
 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base 54851157acf707eb953eada2a84830897dde5c1d last 9172c26ece749fc4289eeb89ef45c35936c5a869

Show changes to contrib/emacs/vc-git.el +1 −1
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..d3ba93325255aea3dc52187dd703d7a8
63cd6e5d 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (if (file-exists-p dir) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil "ls-files"
 "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))

-- 
1.4.4.1
Jakub Narebski· Dec 21, 2006, 11:32 UTC · re: Duncan Mak · lore

Re: [PATCH] fix vc git

Duncan Mak wrote:
> -      (when dir (cd dir))
> +      (if (file-exists-p dir) (cd dir))

Adding `file-exist-p' is good, but there is no need to change `when' to `if'.

-- 
Jakub Narebski
Warsaw, Poland
ShadeHawk on #git
Duncan Mak· Dec 21, 2006, 11:59 UTC · re: Jakub Narebski · lore

Re: [PATCH] fix vc git

Fix vc-git.el to not cd into a non-existent directory.
---
 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base 54851157acf707eb953eada2a84830897dde5c1d last c2af9380cd8ec0170b9b251c0ee8a5efa63bac87

Show changes to contrib/emacs/vc-git.el +1 −1
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..b3c8040dd766c9b15e1d0a9247b6343fdf0a8caf
100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (when (file-exists-p dir) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
"ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))

-- 
1.4.4.1
Junio C Hamano· Dec 22, 2006, 00:52 UTC · re: Duncan Mak · lore

Re: [PATCH] fix vc git

"Duncan Mak" <duncan@a-chinaman.com> writes:
Show 12 quoted lines
> --- a/contrib/emacs/vc-git.el
> +++ b/contrib/emacs/vc-git.el
> @@ -58,7 +58,7 @@
>   (with-temp-buffer
>     (let* ((dir (file-name-directory file))
>            (name (file-relative-name file dir)))
> -      (when dir (cd dir))
> +      (when (file-exists-p dir) (cd dir))
>       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
> "ls-files" "-c" "-z" "--" name)))
>            (let ((str (buffer-string)))
>              (and (> (length str) (length name))

I wonder what would happen when (file-name-directory file) returns nil with this change...

Duncan Mak· Dec 22, 2006, 04:11 UTC · re: Junio C Hamano · lore

Re: [PATCH] fix vc git

I don't think vc-git-registered-file will ever be called with a filename without a directory, as it is used as a hook on vc-next-action, which works on a real file.

Either way, it's a modified patch:
Fix vc-git.el to not cd into a non-existent directory.
---
 contrib/emacs/vc-git.el |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

base c902c9a608c1b727160d6fbb04fa363d9418cd00 last 96e9944b714094ab81f8091711a92506886b8004

Show changes to contrib/emacs/vc-git.el +1 −1
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b6361922fd6e6a2fcd9acb20fd54f5b645b36f0..668bd4a98653046aadf91e7a91af6cd76ba044d4
100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,7 +58,7 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
+      (when (and dir (file-exists-p dir)) (cd dir))
       (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil
"ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))

-- 
1.4.4.1
Junio C Hamano· Dec 22, 2006, 04:54 UTC · re: Duncan Mak · lore

Re: [PATCH] fix vc git

"Duncan Mak" <duncan@a-chinaman.com> writes:
> I don't think vc-git-registered-file will ever be called with a
> filename without a directory, as it is used as a hook on
> vc-next-action, which works on a real file.
I am showing my ignorance, but
 (1) if it works on a real file then can it ever give you a file
     that is in nonexisting directory?
 (2) when it works on a file at the root level of the project
     tree, say, just "Makefile" wouldn't dir which is the result
     of (file-name-directory file) be sometimes nil?  I suspect
     that is the reason of the original (when dir ...)

In either case, your change would not break anything. I am just being curious.

Alexandre Julliard· Dec 22, 2006, 12:39 UTC · re: Duncan Mak · lore

Re: [PATCH] fix vc git

"Duncan Mak" <duncan@a-chinaman.com> writes:
> I don't think vc-git-registered-file will ever be called with a
> filename without a directory, as it is used as a hook on
> vc-next-action, which works on a real file.

Well, if it's a real file I'd expect the directory to exist, and if it doesn't, I'm not sure there's much point in calling git-ls-files at all. In which case do you get a non-existent directory here?

-- 
Alexandre Julliard
julliard@winehq.org
Duncan Mak· Dec 22, 2006, 12:51 UTC · re: Alexandre Julliard · lore

Re: [PATCH] fix vc git

On 12/22/06, Alexandre Julliard <julliard@winehq.org> wrote:
Show 5 quoted lines
>
> Well, if it's a real file I'd expect the directory to exist, and if it
> doesn't, I'm not sure there's much point in calling git-ls-files at
> all. In which case do you get a non-existent directory here?
>

Yeah, but the issue is that, as you know, to create a new file in emacs, you give find-file a non-existent file and emacs will open up a buffer for you and let you save it when you're done.

Once I installed the git vc backend, that feature stopped working for me - that's the reason why I made this patch.

Duncan.
Alexandre Julliard· Dec 22, 2006, 13:56 UTC · re: Duncan Mak · lore

Re: [PATCH] fix vc git

"Duncan Mak" <duncan@a-chinaman.com> writes:
> Yeah, but the issue is that, as you know, to create a new file in
> emacs, you give find-file  a non-existent file and emacs will open up
> a buffer for you and let you save it when you're done.

OK, but in that case vc-git-registered needs to return failure, you cannot call git-ls-files as it may find the file in the wrong directory. I'd suggest something like this:

>From abf4311add221102957145255d5418a7ec06fe1d Mon Sep 17 00:00:00 2001
From: Alexandre Julliard <julliard@winehq.org>
Date: Fri, 22 Dec 2006 14:51:23 +0100
Subject: [PATCH] vc-git: Ignore errors caused by a non-existent directory in vc-git-registered.
Signed-off-by: Alexandre Julliard <julliard@winehq.org>
---
 contrib/emacs/vc-git.el |    5 +++--
 1 files changed, 3 insertions(+), 2 deletions(-)
Show changes to contrib/emacs/vc-git.el +3 −2
diff --git a/contrib/emacs/vc-git.el b/contrib/emacs/vc-git.el
index 8b63619..3eb4bd1 100644
--- a/contrib/emacs/vc-git.el
+++ b/contrib/emacs/vc-git.el
@@ -58,8 +58,9 @@
   (with-temp-buffer
     (let* ((dir (file-name-directory file))
            (name (file-relative-name file dir)))
-      (when dir (cd dir))
-      (and (ignore-errors (eq 0 (call-process "git" nil '(t nil) nil "ls-files" "-c" "-z" "--" name)))
+      (and (ignore-errors
+             (when dir (cd dir))
+             (eq 0 (call-process "git" nil '(t nil) nil "ls-files" "-c" "-z" "--" name)))
            (let ((str (buffer-string)))
              (and (> (length str) (length name))
                   (string= (substring str 0 (1+ (length name))) (concat name "\0"))))))))

-- 
1.4.4.2.g28ce

-- 
Alexandre Julliard
julliard@winehq.org
Duncan Mak· Dec 22, 2006, 15:17 UTC · re: Alexandre Julliard · lore

Re: [PATCH] fix vc git

On 12/22/06, Alexandre Julliard <julliard@winehq.org> wrote:
Show 5 quoted lines
>
> OK, but in that case vc-git-registered needs to return failure, you
> cannot call git-ls-files as it may find the file in the wrong
> directory. I'd suggest something like this:
>
Ah, okay. I agree, that is a better patch.
Would you be committing it  soon?
Thanks,
Duncan.

← back to recent threads