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

[Wolfgang Scherer]

Am 22.01.20 um 13:43 schrieb Lars Ingebrigtsen:
> Wolfgang Scherer <Wolfgang.Scherer@gmx.de> writes:
>
>> Just to clarifiy: my patch does not introduce this FIXME. Only the
>> indentation is changed. This question should be handled in a separate
>> bug report.
> Right; sorry.  Looking at the patch again, I don't quite get the logic here:
>
>> -(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-ignore (file &optional directory _remove)
>> +  "Ignore FILE under CVS.
>> +FILE is either absolute or relative to DIRECTORY."
>> +  (setq file (directory-file-name (expand-file-name file directory)))
>> +  (vc-cvs-append-to-ignore (file-name-directory file) 
>> (file-name-nondirectory file)))
> This is basically
>
> (file-name-nondirectory (directory-file-name (expand-file-name "foo" 
> directory)))
>
> isn't it?
It is for `file` equal to "foo" (a simple basename).
>   In what circumstances does that evaluate to something other
> than "foo"?
If "foo" is something other than a simple basename (see below).
>   That is, what DIRECTORY is doesn't seem to matter, if I'm
> reading this right?

Your assumption, that `file` is always a simple basename is wrong.

(defun xx-vc-cvs-ignore (file &optional directory _remove)
  "Ignore FILE under CVS.
FILE is either absolute or relative to DIRECTORY."
  (let* ((norm-path (directory-file-name (expand-file-name file directory)))
     (norm-dir (file-name-directory norm-path))
     (norm-file (file-name-nondirectory norm-path)))
  (format "'%S\n'%S"
          (list :equal: (if (equal norm-file file) "yes" "no ")
        :file: norm-file :orig: file)
          (list :equal: (if (equal norm-dir directory) "yes" "no ")
        :dir_: norm-dir :orig: directory))))

Default directory is "/home/ws/tmp/", home directory is also "/home/ws/".

;; (insert (format "\n;; 1)\n%s" (xx-vc-cvs-ignore "x-vc-repair.el" nil)))
;; 1)
'(:equal: "yes" :file: "x-vc-repair.el" :orig: "x-vc-repair.el")
'(:equal: "no " :dir_: "/home/ws/tmp/" :orig: nil)

;; (insert (format "\n;; 2)\n%s" (xx-vc-cvs-ignore "x-vc-repair.el" 
"/root/emacs-init/.emacs.def/")))
;; 2)
'(:equal: "yes" :file: "x-vc-repair.el" :orig: "x-vc-repair.el")
'(:equal: "yes" :dir_: "/root/emacs-init/.emacs.def/" :orig: 
"/root/emacs-init/.emacs.def/")

;; (insert (format "\n;; 3)\n%s" (xx-vc-cvs-ignore "~/x-vc-repair.el" 
"/root/other-dir/")))
;; 3)
'(:equal: "no " :file: "x-vc-repair.el" :orig: "~/x-vc-repair.el")
'(:equal: "no " :dir_: "/home/ws/" :orig: "/root/other-dir/")

;; (insert (format "\n;; 4)\n%s" (xx-vc-cvs-ignore 
"/root/emacs-init/.emacs.def/x-vc-repair.el" "/root/other-dir/")))
;; 4)
'(:equal: "no " :file: "x-vc-repair.el" :orig: 
"/root/emacs-init/.emacs.def/x-vc-repair.el")
'(:equal: "no " :dir_: "/root/emacs-init/.emacs.def/" :orig: "/root/other-dir/")

;; (insert (format "\n;; 5)\n%s" (xx-vc-cvs-ignore ".emacs.def/x-vc-repair.el" 
"/root/emacs-init/")))
;; 5)
'(:equal: "no " :file: "x-vc-repair.el" :orig: ".emacs.def/x-vc-repair.el")
'(:equal: "no " :dir_: "/root/emacs-init/.emacs.def/" :orig: 
"/root/emacs-init/")
