GNU bug report logs -
#65049
29.1; vc-do-command fails in windows emacs 29.1
Previous Next
Reported by: Maxim Kim <habamax <at> gmail.com>
Date: Fri, 4 Aug 2023 07:51:01 UTC
Severity: normal
Found in version 29.1
Done: Dmitry Gutov <dmitry <at> gutov.dev>
Bug is archived. No further changes may be made.
Full log
View this message in rfc822 format
On 23/08/2023 19:21, Eli Zaretskii wrote:
>> From: Maxim Kim <habamax <at> gmail.com>
>> Cc: juri <at> linkov.net, 65049 <at> debbugs.gnu.org
>> Date: Wed, 23 Aug 2023 14:28:10 +1000
>>
>> Eli Zaretskii <eliz <at> gnu.org> writes:
>>
>> So, it works for the repo with files having windows CRLF.
>> And doesn't work if the file has EOL type: LF.
>
> Thanks, this finally allowed me to nail the sucker. The fix is a bit
> more complex than I thought, because this bug has two parts: one of
> them in "C-x v D", the other in vc-git-checkin. The patch is below;
> please give it a try with all your real-life use cases.
>
> Dmitry, do you see any problems with installing this on the release
> branch? Or do you prefer to keep it on master for a while, and then
> backport if no one complains?
It doesn't look "obviously correct" to me, and I'm no pro in the coding
system department.
If the patch shouldn't affect non-Windows systems (which seems to be
clear for the second hunk, but not for the first one), I'd say we test
(and ask to test) several orthogonal scenarios: when the system is in
the standard language environment, and when it's in the "unixy" one. And
the same regarding the repository.
If everything's okay, maybe it's good for emacs-29. It will have a few
months to stabilize, right? Just as long as the patch goes into master
too (that's where the bulk of the stabilization will happen, since we
have generally switched to that branch now for daily use).
> diff --git a/lisp/vc/vc-git.el b/lisp/vc/vc-git.el
> index 7ae763d..218696c 100644
> --- a/lisp/vc/vc-git.el
> +++ b/lisp/vc/vc-git.el
> @@ -1051,7 +1051,15 @@ vc-git-checkin
> (user-error "Index not empty"))
> (setq pos (point))))))
> (unless (string-empty-p vc-git-patch-string)
> - (let ((patch-file (make-nearby-temp-file "git-patch")))
> + (let ((patch-file (make-nearby-temp-file "git-patch"))
> + ;; Temporarily countermand the let-binding at the
> + ;; beginning of this function.
> + (coding-system-for-write
> + (coding-system-change-eol-conversion
> + ;; On DOS/Windows, it is important for the patch file
> + ;; to have the Unix EOL format, because Git expects
> + ;; that, even on Windows.
> + (or pcsw vc-git-commits-coding-system) 'unix)))
Any chance this change could negatively affect non-Windows systems? Is
it a given that the 'unix' line endings are always used there?
> (with-temp-file patch-file
> (insert vc-git-patch-string))
> (unwind-protect
> diff --git a/lisp/vc/vc.el b/lisp/vc/vc.el
> index 410fe5c..4e008de 100644
> --- a/lisp/vc/vc.el
> +++ b/lisp/vc/vc.el
> @@ -1917,8 +1917,12 @@ vc-diff-internal
> (generate-new-buffer-name " *vc-diff-clone*") nil))))
> ;; On MS-Windows and MS-DOS, Diff is likely to produce DOS-style
> ;; EOLs, which will look ugly if (car files) happens to have Unix
> - ;; EOLs.
> - (if (memq system-type '(windows-nt ms-dos))
> + ;; EOLs. But for Git, we must force Unix EOLs in the diffs, since
> + ;; Git always produces Unix EOLs in the parts that didn't come
> + ;; from the file, and wants to see any CR characters when applying
> + ;; patches.
> + (if (and (memq system-type '(windows-nt ms-dos))
> + (not (eq (vc-deduce-backend) 'Git)))
> (setq coding-system-for-read
> (coding-system-change-eol-conversion coding-system-for-read
> 'dos)))
:thumbsup:
(All the coding system juggling in these files is already too scary for
my taste, but I don't have any better suggestions.)
This bug report was last modified 231 days ago.
Previous Next
GNU bug tracking system
Copyright (C) 1999 Darren O. Benham,
1997,2003 nCipher Corporation Ltd,
1994-97 Ian Jackson.