bug#37215: [PATCH] vc-cvs-ignore writes absolute filenames and duplicate strings

[Wolfgang Scherer <Wolfgang.Scherer@gmx.de>]

[-- Attachment #1: Type: text/plain, Size: 1716 bytes --]


Am 21.02.20 um 10:31 schrieb Eli Zaretskii:
>> Cc: larsi@gnus.org, 37215@debbugs.gnu.org, dgutov@yandex.ru
>> From: Wolfgang Scherer <Wolfgang.Scherer@gmx.de>
>> Date: Thu, 20 Feb 2020 00:06:11 +0100
>> (lisp/vc/pcvs.el) Call ‘vc-cvs-append-to-ignore’ with SORT argument.
> Please quote 'like this' in log messages, and try to avoid non-ASCII
> characters there (they are generally only necessary in people's
> names).
OK
>> +Patterns follow glob(7) syntax.  Special characters \"?*[\\\" are
>> +escaped with a backslash."
> I'd say "should be escaped" here, since this is a requirement for the
> argument passed to this function.
Here is that amgbuity again ;-). It is only required, if the user
wants a special character to match literally. It's perfectly fine to
specify  *.pyc  as a pattern. I have phrased it like that.
> Also, I'd mention that FILE can be a pattern, otherwise the reference
> to patterns might come as a surprise to the reader.
I emphasized more, that the basename of the FILE argument (not the
entire FILE) is in fact a CVS ignore pattern.\x15\x04
>> +to hear about anymore.  If SORT is non-nil, sort the ines of the
>> +ignore file."                                        ^^^^
> Typo: should be "lines".
Right.
>> +    (goto-char (point-min))
>> +    (save-match-data
>> +      (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil t)
>> +        (goto-char (point-max))
> You could use non-nil, non-t 3rd argument of re-search-forward, in
> which case the following goto-char would be redundant, right?
Right, I just left the  goto-char  in there, because it makes it
obvious what is going on. Switching to the side-effect optimization ...

[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Do-not-write-absolute-filenames-and-duplicate-string.patch --]
[-- Type: text/x-patch; name="0001-Do-not-write-absolute-filenames-and-duplicate-string.patch", Size: 4541 bytes --]

From edea70777b81f745b86f812c7ca2e0021a9ecb83 Mon Sep 17 00:00:00 2001
From: Wolfgang Scherer <wolfgang.scherer@gmx.de>
Date: Fri, 21 Feb 2020 21:28:11 +0100
Subject: [PATCH] Do not write absolute filenames and duplicate strings into
 CVS ignore files

* lisp/vc/vc-cvs.el: (vc-cvs-ignore) Expand filename correctly
and pass on basename as pattern only.
(vc-cvs-append-to-ignore) Do not write duplicate strings to
.cvsignore.  Addtional optional parameter SORT.
(lisp/vc/pcvs.el) Call 'vc-cvs-append-to-ignore' with SORT argument.
---
 lisp/vc/pcvs.el   |  4 ++--
 lisp/vc/vc-cvs.el | 45 ++++++++++++++++++++++++++++++++-------------
 2 files changed, 34 insertions(+), 15 deletions(-)

diff --git a/lisp/vc/pcvs.el b/lisp/vc/pcvs.el
index dcba504..cb0494e 100644
--- a/lisp/vc/pcvs.el
+++ b/lisp/vc/pcvs.el
@@ -106,7 +106,6 @@
 ;; 	right now, it's killed without further ado.
 ;; - make `cvs-mode-ignore' allow manually entering a pattern.
 ;; 	to which dir should it apply ?
-;; - cvs-mode-ignore should try to remove duplicate entries.
 ;; - maybe poll/check CVS/Entries files to react to external `cvs' commands ?
 ;; - some kind of `cvs annotate' support ?
 ;; 	but vc-annotate can be used instead.
@@ -1972,7 +1971,8 @@ This command ignores files that are not flagged as `Unknown'."
   (interactive)
   (dolist (fi (cvs-mode-marked 'ignore))
     (vc-cvs-append-to-ignore (cvs-fileinfo->dir fi) (cvs-fileinfo->file fi)
-			  (eq (cvs-fileinfo->subtype fi) 'NEW-DIR))
+			  (eq (cvs-fileinfo->subtype fi) 'NEW-DIR)
+                          cvs-sort-ignore-file)
     (setf (cvs-fileinfo->type fi) 'DEAD))
   (cvs-cleanup-collection cvs-cookies nil nil nil))

diff --git a/lisp/vc/vc-cvs.el b/lisp/vc/vc-cvs.el
index 16566a8..b6afda6 100644
--- a/lisp/vc/vc-cvs.el
+++ b/lisp/vc/vc-cvs.el
@@ -1220,14 +1220,33 @@ is non-nil."
   "Return the administrative directory of FILE."
   (vc-find-root file "CVS"))

-(defun vc-cvs-ignore (file &optional _directory _remove)
-  "Ignore FILE under CVS."
-  (vc-cvs-append-to-ignore (file-name-directory file) file))
-
-(defun vc-cvs-append-to-ignore (dir str &optional old-dir)
+(defun vc-cvs-ignore (file &optional directory _remove)
+  "Ignore FILE under CVS.
+FILE is either absolute or relative to DIRECTORY.  The basename
+of FILE is written unmodified into the ignore file and is
+therefore evaluated by CVS as an ignore pattern which follows
+glob(7) syntax.  If the pattern should match any of the special
+characters ‘?*[\\\’ literally, they must be escaped with a
+backslash.
+
+CVS processes one ignore file for each subdirectory.  Patterns
+are separated by whitespace and only match files in the same
+directory.  Since FILE can be a relative filename with leading
+diretories, FILE is expanded against DIRECTORY to determine the
+correct absolute filename.  The directory name of this path is
+then used to determine the location of the ignore file.  The base
+name of this path is used as pattern for the ignore file.
+
+Since patterns are whitespace sparated, it is usually better to
+replace spaces in filenames with question marks ‘?’."
+  (setq file (directory-file-name (expand-file-name file directory)))
+  (vc-cvs-append-to-ignore (file-name-directory file) (file-name-nondirectory file)))
+
+(defun vc-cvs-append-to-ignore (dir str &optional old-dir sort)
   "In DIR, add STR to the .cvsignore file.
 If OLD-DIR is non-nil, then this is a directory that we don't want
-to hear about anymore."
+to hear about anymore.  If SORT is non-nil, sort the lines of the
+ignore file."
   (with-current-buffer
       (find-file-noselect (expand-file-name ".cvsignore" dir))
     (when (ignore-errors
@@ -1236,13 +1255,13 @@ to hear about anymore."
 		 (not (vc-editable-p buffer-file-name))))
       ;; CVSREAD=on special case
       (vc-checkout buffer-file-name t))
-    (goto-char (point-max))
-    (unless (bolp) (insert "\n"))
-    (insert str (if old-dir "/\n" "\n"))
-    ;; FIXME this is a pcvs variable.
-    (if (bound-and-true-p cvs-sort-ignore-file)
-        (sort-lines nil (point-min) (point-max)))
-    (save-buffer)))
+    (goto-char (point-min))
+    (save-match-data
+      (unless (re-search-forward (concat "^" (regexp-quote str) "$") nil 'move)
+        (unless (bolp) (insert "\n"))
+        (insert str (if old-dir "/\n" "\n"))
+        (if sort (sort-lines nil (point-min) (point-max)))
+        (save-buffer)))))

 (provide 'vc-cvs)

--
2.7.4