GNU bug report logs - #18619
24.3.93; vc-svn-ignore is broken

Previous Next

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 18619 in the body.
You can then email your comments to 18619 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

Message #5 received at submit <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: bug-gnu-emacs <at> gnu.org
Subject: 24.3.93; vc-svn-ignore is broken
Date: Fri, 03 Oct 2014 06:02:55 +0400

Tags: patch

It plainly doesn't work: gives an error when called on an unregistered
file, or (maybe, haven't tested) succeeds and does nothing when called
on a registered file.

The way it's currently written is wrong. I don't think there's a way to
do "add this file to ignores" in one command invocation. SVN has
"propget" and "propset" commands, the value has to be a list of
wildcards to ignore, one per line, and it needs to be set on a parent
directory, not on the file itself, like the current code tries to do.

The attached patch mostly takes care of this. The change to `vc-ignore'
is needed because this property doesn't work with absolute paths. And
passing a relative path to backend `ignore' implementations make sense
anyway.

In GNU Emacs 24.3.93.4 (x86_64-unknown-linux-gnu, GTK+ Version 3.10.8)
 of 2014-09-19 on axl
Repository revision: 117510 juri <at> jurta.org-20140918205955-kwp5ckzrk2l4w1km
Windowing system distributor `The X.Org Foundation', version 11.0.11501000
System Description:	Ubuntu 14.04.1 LTS

=== modified file 'lisp/vc/vc-svn.el'
--- lisp/vc/vc-svn.el	2014-01-01 07:43:34 +0000
+++ lisp/vc/vc-svn.el	2014-10-03 01:51:12 +0000
@@ -354,14 +354,22 @@
 		(concat "-r" rev))
 	   (vc-switches 'SVN 'checkout))))
 
-(defun vc-svn-ignore (file &optional _directory _remove)
+(defun vc-svn-ignore (file &optional directory remove)
   "Ignore FILE under Subversion.
 FILE is a file wildcard, relative to the root directory of DIRECTORY."
-  (vc-svn-command t 0 file "propedit" "svn:ignore"))
+  (let* ((ignores (vc-svn-ignore-completion-table directory))
+         (ignores (if remove
+                      (delete file ignores)
+                    (push file ignores))))
+    (vc-svn-command nil 0 nil nil "propset" "svn:ignore"
+                    (mapconcat #'identity ignores "\n")
+                    (expand-file-name directory))))
 
-(defun vc-svn-ignore-completion-table (_file)
-  "Return the list of ignored files."
-  )
+(defun vc-svn-ignore-completion-table (directory)
+  "Return the list of ignored files in DIRECTORY."
+  (with-temp-buffer
+    (vc-svn-command t t nil "propget" "svn:ignore" (expand-file-name directory))
+    (split-string (buffer-string))))
 
 (defun vc-svn-find-admin-dir (file)
   "Return the administrative directory of FILE."

=== modified file 'lisp/vc/vc.el'
--- lisp/vc/vc.el	2014-01-01 07:43:34 +0000
+++ lisp/vc/vc.el	2014-10-03 01:58:18 +0000
@@ -1370,7 +1370,9 @@
   (let* ((directory (or directory default-directory))
 	 (backend (or (vc-responsible-backend default-directory)
                       (error "Unknown backend"))))
-    (vc-call-backend backend 'ignore file directory remove)))
+    (vc-call-backend backend 'ignore
+                     (file-relative-name file directory)
+                     directory remove)))
 
 (defun vc-default-ignore (backend file &optional directory remove)
   "Ignore FILE under the VCS of DIRECTORY (default is `default-directory').

Message #8 received at 18619 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 18619 <at> debbugs.gnu.org
Subject: Re: bug#18619: 24.3.93; vc-svn-ignore is broken
Date: Thu, 02 Oct 2014 22:46:04 -0400

> The attached patch mostly takes care of this. The change to `vc-ignore'
> is needed because this property doesn't work with absolute paths. And
> passing a relative path to backend `ignore' implementations make sense
> anyway.

The vc-svn.el change is fine for emacs-24 since the existing code is indeed
hopelessly broken (so it won't be worse in any case).
But could you explain why you think the vc.el change is safe (I can agree
it makes sense, but I want to be extra sure the change is safe).


        Stefan

Message #11 received at 18619 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 18619 <at> debbugs.gnu.org
Subject: Re: bug#18619: 24.3.93; vc-svn-ignore is broken
Date: Fri, 03 Oct 2014 07:55:19 +0400

On 10/03/2014 06:46 AM, Stefan Monnier wrote:

> But could you explain why you think the vc.el change is safe (I can agree
> it makes sense, but I want to be extra sure the change is safe).

Actually, I'm not sure that it's safe. Looking at `vc-default-ignore', 
it seems we're expected to obtain the relative path in the backend 
implementation functions.

This works, too, and limits the changes to `vc-svn.el'.

Message #14 received at 18619 <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 18619 <at> debbugs.gnu.org
Subject: Re: bug#18619: 24.3.93; vc-svn-ignore is broken
Date: Fri, 03 Oct 2014 08:08:26 +0400

The new patch:

=== modified file 'lisp/vc/vc-svn.el'
--- lisp/vc/vc-svn.el	2014-01-01 07:43:34 +0000
+++ lisp/vc/vc-svn.el	2014-10-03 03:41:39 +0000
@@ -354,14 +354,23 @@
 		(concat "-r" rev))
 	   (vc-switches 'SVN 'checkout))))

-(defun vc-svn-ignore (file &optional _directory _remove)
+(defun vc-svn-ignore (file &optional directory remove)
   "Ignore FILE under Subversion.
 FILE is a file wildcard, relative to the root directory of DIRECTORY."
-  (vc-svn-command t 0 file "propedit" "svn:ignore"))
+  (let* ((ignores (vc-svn-ignore-completion-table directory))
+         (file (file-relative-name file directory))
+         (ignores (if remove
+                      (delete file ignores)
+                    (push file ignores))))
+    (vc-svn-command nil 0 nil nil "propset" "svn:ignore"
+                    (mapconcat #'identity ignores "\n")
+                    (expand-file-name directory))))

-(defun vc-svn-ignore-completion-table (_file)
-  "Return the list of ignored files."
-  )
+(defun vc-svn-ignore-completion-table (directory)
+  "Return the list of ignored files in DIRECTORY."
+  (with-temp-buffer
+    (vc-svn-command t t nil "propget" "svn:ignore" (expand-file-name 
directory))
+    (split-string (buffer-string))))

 (defun vc-svn-find-admin-dir (file)
   "Return the administrative directory of FILE."

Message #17 received at 18619 <at> debbugs.gnu.org (full text, mbox):

From: Stefan Monnier <monnier <at> iro.umontreal.ca>
To: Dmitry Gutov <dgutov <at> yandex.ru>
Cc: 18619 <at> debbugs.gnu.org
Subject: Re: bug#18619: 24.3.93; vc-svn-ignore is broken
Date: Fri, 03 Oct 2014 08:24:02 -0400

> The new patch:

Please install it into emacs-24, thank you,


        Stefan

Message #22 received at 18619-done <at> debbugs.gnu.org (full text, mbox):

From: Dmitry Gutov <dgutov <at> yandex.ru>
To: Stefan Monnier <monnier <at> iro.umontreal.ca>
Cc: 18619-done <at> debbugs.gnu.org
Subject: Re: bug#18619: 24.3.93; vc-svn-ignore is broken
Date: Fri, 03 Oct 2014 17:25:22 +0400

On 10/03/2014 04:24 PM, Stefan Monnier wrote:
> Please install it into emacs-24, thank you,

Done, thanks.

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.