From: Dmitry Gutov <dmitry@gutov.dev>
To: Eli Zaretskii <eliz@gnu.org>
Cc: 65049@debbugs.gnu.org, habamax@gmail.com, juri@linkov.net
Subject: bug#65049: Minor update to the repro steps
Date: Sat, 26 Aug 2023 03:45:41 +0300 [thread overview]
Message-ID: <49d5e741-f97d-ae4d-f79c-ec418051d868@gutov.dev> (raw)
In-Reply-To: <83ttsnoda5.fsf@gnu.org>
On 25/08/2023 09:18, Eli Zaretskii wrote:
>>> But here's a modification of the scenario that fails (again: both with
>>> and without the patch): replace step 9 with
>>>
>>> 9. C-x v =
>>>
>>> The non-root diff looks a little different to begin with: it doesn't
>>> show those ^M chars at the end of lines (whereas the result of
>>> vc-root-diff shows them). That is likely the reason: buffer set up in a
>>> different way.
>>
>> Looks like it's this line:
>>
>> (coding-system-for-read
>> (if files (vc-coding-system-for-diff (car files)) 'undecided))
>>
>> near the beginning of vc-diff-internal that creates the difference.
>> Commenting it out makes the scenario work with both 'C-x v =' and 'C-x v D'.
>
> That code fragment is very old, so just removing it is scary, even if
> only in master.
Yeah, I noticed: it's from 2007 :-)
> What if you change that fragment to say
>
> (coding-system-for-read
> (if files (vc-coding-system-for-diff (car files)) 'undecided-unix))
>
> instead?
No change at all. The reasons are twofold:
- You changed the value that was seemingly used for the "root" case,
because in the individual diff's case files must not be nil: it would
contain the files to be diff'd. That's why that change doesn't affect
'C-x v ='.
- But it also doesn't affect 'C-x v D'. Because even in that case FILES
is non-nil ;-(. In that scenario FILES is a list with one item: the
repository's root directory.
So we can conclude that this code is at least a little buggy. But... (*)
> If that doesn't work, please tell to what value does
> vc-diff-internal set coding-system-for-read in your case there, and I
> will try to figure out what would needs to be done there.
(vc-coding-system-for-diff (car files)) either returns 'undecided when
FILES contains the directory (vc-root-diff), or 'undecided-dos when
FILES contains hello.txt as the sole element (because our scenario made
sure the file has that encoding), that's the vc-diff case.
These are the values coding-system-for-read is set to.
> (In general, I believe that using Git on Posix hosts with files that
> have DOS EOLs could have such problems in other use cases, where diffs
> are generated and then applied as patches. We just don't know about
> those cases because they are extremely rare in Real Life.)
I'm definitely curious which scenarios made Eric add that line.
(*) ... upon some reflection, though, it seems like our success here is
kind of relying on vc-root-diff's bug. Remember I mentioned the ^M chars
appearing at the ends of lines? That is because the encoding of the diff
buffer (utf-8-unix) doesn't match the encoding of the file (utf-8-dos).
That only happens with the root diff, but not with vc-diff, which
follows the old design and uses the return value of
vc-coding-system-for-diff (undecided-dos). As luck would have it,
though, our patch generation and application works well with the former
behavior but not the latter.
Still, Eric's old design did not make allowance for root diffs. Not sure
what to do with that; though I suppose we could post-process the diff
outputs instead: read the name of the first file in there, then detect
its encoding on disk, and then re-decode the diff contents if the
current value of buffer-file-coding-system doesn't match. And *then* we
would need to fix vc-git-checkin-patch in that scenario (and maybe other
backends as well).
Or we decide that seeing ^M in diff buffers is a good thing under those
conditions, and delete the line in question.
WDYT?
next prev parent reply other threads:[~2023-08-26 0:45 UTC|newest]
Thread overview: 49+ messages / expand[flat|nested] mbox.gz Atom feed top
2023-08-04 7:50 bug#65049: 29.1; vc-do-command fails in windows emacs 29.1 Maxim Kim
2023-08-04 8:02 ` bug#65049: Minor update to the repro steps Maxim Kim
2023-08-04 11:05 ` Eli Zaretskii
2023-08-04 11:24 ` Maxim Kim
2023-08-04 17:56 ` Juri Linkov
2023-08-06 23:04 ` Maxim Kim
2023-08-07 1:09 ` Maxim Kim
2023-08-07 16:24 ` Eli Zaretskii
2023-08-07 23:17 ` Maxim Kim
2023-08-20 16:49 ` Juri Linkov
2023-08-20 18:25 ` Eli Zaretskii
2023-08-21 6:53 ` Juri Linkov
2023-08-21 11:00 ` Eli Zaretskii
2023-08-21 11:39 ` Maxim Kim
2023-08-21 12:18 ` Eli Zaretskii
2023-08-21 23:10 ` Maxim Kim
2023-08-22 12:52 ` Eli Zaretskii
2023-08-22 13:12 ` Maxim Kim
2023-08-22 13:17 ` Eli Zaretskii
2023-08-22 23:43 ` Maxim Kim
2023-08-23 4:28 ` Maxim Kim
2023-08-23 16:21 ` Eli Zaretskii
2023-08-23 17:42 ` Juri Linkov
2023-08-23 18:43 ` Eli Zaretskii
2023-08-23 20:13 ` Dmitry Gutov
2023-08-24 4:54 ` Eli Zaretskii
2023-08-24 21:06 ` Dmitry Gutov
2023-08-24 21:35 ` Dmitry Gutov
2023-08-24 21:44 ` Dmitry Gutov
2023-08-25 6:18 ` Eli Zaretskii
2023-08-26 0:45 ` Dmitry Gutov [this message]
2023-08-26 8:50 ` Eli Zaretskii
2023-08-27 1:14 ` Dmitry Gutov
2023-08-27 5:36 ` Eli Zaretskii
2023-08-27 22:32 ` Dmitry Gutov
2023-08-28 12:12 ` Eli Zaretskii
2023-08-28 13:45 ` Dmitry Gutov
2023-08-28 16:12 ` Eli Zaretskii
2023-08-28 16:51 ` Dmitry Gutov
2023-08-28 16:57 ` Eli Zaretskii
2023-08-28 17:39 ` Dmitry Gutov
2023-08-28 18:26 ` Eli Zaretskii
2023-08-31 2:07 ` Richard Stallman
2023-08-31 2:14 ` Dmitry Gutov
2023-08-31 6:00 ` Eli Zaretskii
2023-08-23 23:46 ` Maxim Kim
2024-10-02 17:45 ` bug#65049: 29.1; vc-do-command fails in windows emacs 29.1 Sebastián Monía
2024-10-02 18:51 ` Eli Zaretskii
2024-10-02 21:32 ` Dmitry Gutov
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
List information: https://www.gnu.org/software/emacs/
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=49d5e741-f97d-ae4d-f79c-ec418051d868@gutov.dev \
--to=dmitry@gutov.dev \
--cc=65049@debbugs.gnu.org \
--cc=eliz@gnu.org \
--cc=habamax@gmail.com \
--cc=juri@linkov.net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
Code repositories for project(s) associated with this public inbox
https://git.savannah.gnu.org/cgit/emacs.git
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).