bug#4451: 23.1; EOL problems with vc-diff and cygwin

[Reiner Steib]

tags 4451 + patch
quit
----- end of commands for control@..., bcc-ed -----

[ Sorry for the very late reply.  With Stefan's patch, I had no
  problems so far.  But an upgrade to Emacs 23.2 brought the unpatched
  version and thus the problem back. ]

On Mon, Oct 05 2009, Eli Zaretskii wrote:

>> Date: Mon, 05 Oct 2009 18:07:53 +0200
>> From: Reiner Steib <reiner.steib@gmx.de>
>> Cc: 4451@emacsbugs.donarmstrong.com, monnier@iro.umontreal.ca
>> 
>> The repository file (K3.xml,v) has Unix EOLs.  But if I do a fresh
>> checkout, I get a file K3.xml with DOS EOLs (I think this is the usual
>> behavoir of the Windows cvs binaries[1] for text files unless you
>> specify the switch "-ko").
>
> Yes, that's normal behavior of the ported CVS.
>
>> However, in my workflow I overwrite the file with a Unix EOL file
>> (exported from some application)
>
> Why are you doing that, and what is that ``some application'' which
> converts the DOS EOLs into Unix?

It doesn't convert EOLs.  It's a commercial application (used by my
employer).  It stores its configuration in XML files including very
limited version control.  But it doesn't offer stuff like diff etc
which CVS does.  But we want to be able to diff revisions and/or
different configuration files.  So we export the config files from the
application (-> Unix EOL) - more precise it is out testing environment
-, add newlines to simplify diffing, review the changes and commit the
file to CVS.  We also specific (stable, tested) versions of the config
file from CVS and import it into the application (production
environment).

>> Probably this conversion is also the reason that "cvs diff --binary"
>> outputs "^M^M$" for the old file and "^M$" for the new file.  "cvs diff"
>> outputs consitent DOS EOLs (both diff markers and the text):
>> 
>> $ cvs diff SK3.xml | cat --show-all | grep -F -v '^M' | wc -l
>
> It sounds like your port of CVS assumes that text files have DOS
> EOLs.  If you violate that assumption, it produces badly formatted
> files, as far as EOL is concerned, and all hell will break loose on
> you; Emacs showing the ugly ^M characters is just one of the
> symptoms.
>
>> > Another possibility is that somewhere along the chain of processing
>> > the output, we force EOL conversion to be Unix-style, instead of
>> > detecting EOLs dynamically, or maybe even forcing it to DOS (if we
>> > have clear evidence for doing the latter).
>> 
>> `vc-coding-system-for-diff' returns `utf-8-unix' in both calls.
>
> Because the file is already visited in an Emacs buffer, I presume, and
> vc-coding-system-for-diff uses that.
>
> So I agree with Stefan that you should simply not overwrite the
> checked-out file with Unix EOLs.  We could install the change
> suggested by Stefan (based on this analysis, I no longer object to
> it), 

If nobody else objects, may I install[1] Stefan's patch?  Or we could
make it depend on some variable as well ...

--8<---------------cut here---------------start------------->8---
--- vc.el.~1~   2010-04-03 18:26:12.000000000 +0200
+++ vc.el       2010-07-13 12:09:53.972342500 +0200
@@ -1398,6 +1398,12 @@
 ;;          (vc-call-backend ',(vc-backend f)
 ;;                           'diff (list ',f) ',rev1 ',rev2))))))

+(defvar vc-coding-system-strip-eol t ;; nil
+  "When non-nil, don't inherit the EOL part of the coding-system.")
+;; Don't inherit the EOL part of the coding-system in
+;; `vc-coding-system-for-diff', because some diff tools may choose to
+;; use another one.  bug#4451.
+
 (defun vc-coding-system-for-diff (file)
   "Return the coding system for reading diff output for FILE."
   (or coding-system-for-read
@@ -1405,7 +1411,12 @@
       ;; use the buffer's coding system
       (let ((buf (find-buffer-visiting file)))
         (when buf (with-current-buffer buf
-                   buffer-file-coding-system)))
+                   (if vc-coding-system-strip-eol
+                       ;; Don't inherit the EOL part of the coding-system,
+                       ;; because some diff tools may choose to use
+                       ;; another one.  bug#4451.
+                       (coding-system-base buffer-file-coding-system)
+                     buffer-file-coding-system))))
       ;; otherwise, try to find one based on the file name
       (car (find-operation-coding-system 'insert-file-contents file))
       ;; and a final fallback

--8<---------------cut here---------------end--------------->8---

> but I'm quite sure this won't be the end of your trouble.  For
> example, Patch will most probably fail to apply the diffs, because
> they have DOS EOLs while the source file has Unix EOLs.  This means
> that CVS merges will also fail.  Etc., etc. -- that way lies
> madness.

Up to now, I didn't see other EOL related problems.  Patch works fine
with the diffs from Emacs buffer.

> If you really want Unix EOLs, you will have to hack CVS to use binary
> I/O.  (I think you can force that by using some special Cygwin options
> when ``mounting'' the volume, but don't ask me for details, because I
> don't use Cygwin.)

I can't do that because I don't control the cygwin configuration (and
I would not like to change it since it might cause problems).

> Thanks.

Thanks for your help.

Bye, Reiner.

[1] It will take a while since I didn't set up bazaar upto now.
-- 
       ,,,
      (o o)
---ooO-(_)-Ooo---  |  PGP key available  |  http://rsteib.home.pages.de/