Re: Please make git-am handle \r\n-damaged patches
Nanako Shiraishi <nanako3@lavabit.com> writes:
Show 14 quoted lines
> Quoting Junio C Hamano <gitster@pobox.com>
>
>> Alex Riesen <raa.lkml@gmail.com> writes:
>>
>>> Maybe it is as simple as that (not tested yet,
>>> and sent through gmail, so please be careful):
>>
>> I thought about this approach, but it made me worried about a case where
>> an otherwise sane piece of e-mail has \r at the end of one line as the
>> real payload. But as long as we are talking about a text e-mail (and we
>> are talking about mailsplit here and a binary payload with a CTE applied
>> counts as text), I think we can safely use an approach like this.
>
> Is it safe to rebase a commit that introduces a carriage-return at the end of the line using the updated program?
Hmmm, good point. The approach does break rebase.
At least we could patch it up like this.
builtin-mailinfo.c | 2 +-
builtin-mailsplit.c | 11 +++++++++--
builtin.h | 2 +-
git-am.sh | 8 +++++++-
t/t3400-rebase.sh | 26 ++++++++++++++++++++++++--
5 files changed, 42 insertions(+), 7 deletions(-)
diff --git a/builtin-mailinfo.c b/builtin-mailinfo.c
index 92637ac..6e44d2c 100644
--- a/builtin-mailinfo.c
+++ b/builtin-mailinfo.c
@@ -835,7 +835,7 @@ static void handle_body(void)
strbuf_reset(&line);
if (strbuf_avail(&line) < 100)
strbuf_grow(&line, 100);
- } while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin)));
+ } while ((len = read_line_with_nul(line.buf, strbuf_avail(&line), fin, 0)));
handle_body_out:
strbuf_release(&prev);
diff --git a/builtin-mailsplit.c b/builtin-mailsplit.c
index ad5f6b5..f774b04 100644
--- a/builtin-mailsplit.c
+++ b/builtin-mailsplit.c
@@ -45,8 +45,10 @@ static int is_from_line(const char *line, int len)
/* Could be as small as 64, enough to hold a Unix "From " line. */
static char buf[4096];
+static int keep_cr;
+
/* We cannot use fgets() because our lines can contain NULs */
-int read_line_with_nul(char *buf, int size, FILE *in)
+int read_line_with_nul(char *buf, int size, FILE *in, int nuke_cr_at_eol)
{
int len = 0, c;
@@ -58,6 +60,9 @@ int read_line_with_nul(char *buf, int size, FILE *in)
if (c == '\n' || len + 1 >= size)
break;
}
+ if (nuke_cr_at_eol &&
+ (1 < len && buf[len - 2] == '\r' && buf[len - 1] == '\n'))
+ buf[len-- - 2] = '\n';
buf[len] = '\0';
return len;
@@ -93,7 +98,7 @@ static int split_one(FILE *mbox, const char *name, int allow_bare)
if (fwrite(buf, 1, len, output) != len)
die_errno("cannot write output");
- len = read_line_with_nul(buf, sizeof(buf), mbox);
+ len = read_line_with_nul(buf, sizeof(buf), mbox, !keep_cr);
if (len == 0) {
if (feof(mbox)) {
status = 1;
@@ -248,6 +253,8 @@ int cmd_mailsplit(int argc, const char **argv, const char *prefix)
nr = strtol(arg+2, NULL, 10);
} else if ( arg[1] == 'b' && !arg[2] ) {
allow_bare = 1;
+ } else if (!strcmp(arg, "--keep-cr")) {
+ keep_cr = 1;
} else if ( arg[1] == 'o' && arg[2] ) {
dir = arg+2;
} else if ( arg[1] == '-' && !arg[2] ) {
diff --git a/builtin.h b/builtin.h
index 825a96f..59dddee 100644
--- a/builtin.h
+++ b/builtin.h
@@ -13,7 +13,7 @@ extern const char git_more_info_string[];
extern void list_common_cmds_help(void);
extern const char *help_unknown_cmd(const char *cmd);
extern void prune_packed_objects(int);
-extern int read_line_with_nul(char *buf, int size, FILE *file);
+extern int read_line_with_nul(char *buf, int size, FILE *file, int nuke_cr_at_eol);
extern int fmt_merge_msg(int merge_summary, struct strbuf *in,
struct strbuf *out);
extern int commit_tree(const char *msg, unsigned char *tree,
diff --git a/git-am.sh b/git-am.sh
index d64d997..985226b 100755
--- a/git-am.sh
+++ b/git-am.sh
@@ -197,7 +197,13 @@ check_patch_format () {
split_patches () {
case "$patch_format" in
mbox)
- git mailsplit -d"$prec" -o"$dotest" -b -- "$@" > "$dotest/last" ||
+ case "$rebasing" in
+ '')
+ keep_cr= ;;
+ ?*)
+ keep_cr=--keep-cr ;;
+ esac
+ git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > "$dotest/last" ||
clean_abort
;;
stgit-series)
diff --git a/t/t3400-rebase.sh b/t/t3400-rebase.sh
index c5c29cc..4e6a44b 100755
--- a/t/t3400-rebase.sh
+++ b/t/t3400-rebase.sh
@@ -3,9 +3,10 @@
# Copyright (c) 2005 Amos Waterland
#
-test_description='git rebase should not destroy author information
+test_description='git rebase assorted tests
-This test runs git rebase and checks that the author information is not lost.
+This test runs git rebase and checks that the author information is not lost
+among other things.
'
. ./test-lib.sh
@@ -133,4 +134,25 @@ test_expect_success 'rebase -q is quiet' '
test ! -s output.out
'
+q_to_cr () {
+ tr Q '\015'
+}
+
+test_expect_success 'Rebase a commit that sprinkles CRs in' '
+ (
+ echo "One"
+ echo "TwoQ"
+ echo "Three"
+ echo "FQur"
+ echo "Five"
+ ) | q_to_cr >CR &&
+ git add CR &&
+ test_tick &&
+ git commit -a -m "A file with a line with CR" &&
+ git tag file-with-cr &&
+ git checkout HEAD^0 &&
+ git rebase --onto HEAD^^ HEAD^ &&
+ git diff --exit-code file-with-cr:CR HEAD:CR
+'
+
test_done
Junio C Hamano <gitster@pobox.com>