git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Salikh Zakirov <Salikh.Zakirov@Intel.com>
To: unlisted-recipients:; (no To-header on input)
Cc: git@vger.kernel.org
Subject: [PATCH] Fixed Cygwin CR-munging problem in mailsplit
Date: Sat, 27 May 2006 21:57:29 +0400	[thread overview]
Message-ID: <44789309.1030002@Intel.com> (raw)
In-Reply-To: <7virnsk6fe.fsf@assigned-by-dhcp.cox.net>


Do not open mailbox file as fopen(..., "rt")
as this strips CR characters from the diff,
thus breaking the patch context for changes 
in CRLF files.

Signed-off-by: Salikh Zakirov <Salikh.Zakirov@Intel.com>

---

Junio C Hamano wrote:
> 
> 	git init-db
> 	echo 'abc@' | tr '[@]' '[\015]' >a
>         git add a
>         git commit -m initial
> 	echo 'def@' | tr '[@]' '[\015]' >>a
>         git commit -a -m second
>         git format-patch HEAD^
> 	git reset --hard HEAD^
>         git am 0*.txt
> 

This reproduction scenario results in exactly the same problem.
The problem is observed on Cygwin.
My initial evaluation of the problem turned out to be completely bogus.

I've tracked the problem down to the fopen(file, "rt") in mailsplit.c,
which then truncates the CR character from the patch file.
This changes the patch context lines and it no longer applies.
Changing it to fopen(file, "r") fixes the problem.

> Having said that, CRLF is unsafe for E-mail transfers anyway, so
> I think we would need a special option to tell git-apply that it
> should match '\n' that appears in the patch with '\r\n' in the
> file being patched.  But I do not think that has anything to do
> with the breakage you saw in your reproduction recipe.

My use case does not involve e-mail transfers at all.
I'm using git-format-patch and git-am to rewrite the
patch sequence with different commit messages. 

Unfortunately, some of my fellow developers are not quite
careful, and occasionally some of the source files acquire
CR characters, sometimes in several lines only.

fd405a0843f3efd474bc7897b06d813d6498fbf4
 mailsplit.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

fd405a0843f3efd474bc7897b06d813d6498fbf4
diff --git mailsplit.c mailsplit.c
index c529e2d..70a569c 100644
--- mailsplit.c
+++ mailsplit.c
@@ -162,7 +162,7 @@ int main(int argc, const char **argv)
 
 	while (*argp) {
 		const char *file = *argp++;
-		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "rt");
+		FILE *f = !strcmp(file, "-") ? stdin : fopen(file, "r");
 		int file_done = 0;
 
 		if ( !f )
-- 
1.3.3.gfd40

  reply	other threads:[~2006-05-27 17:58 UTC|newest]

Thread overview: 8+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2006-05-26 16:00 git-apply can't apply patches to CRLF-files Salikh Zakirov
2006-05-26 18:05 ` Junio C Hamano
2006-05-27 17:57   ` Salikh Zakirov [this message]
2006-05-27 18:21     ` [PATCH] Fixed Cygwin CR-munging problem in mailsplit Junio C Hamano
2006-05-27 18:25       ` Junio C Hamano
  -- strict thread matches above, loose matches on Subject: below --
2006-05-27 20:57 Zakirov, Salikh
2006-05-28 16:39 ` Christopher Faylor
2006-05-31  4:49 ` Junio C Hamano

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

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=44789309.1030002@Intel.com \
    --to=salikh.zakirov@intel.com \
    --cc=git@vger.kernel.org \
    /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.
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 NNTP newsgroup(s).