Ambiguous date handling

Ambiguous date handling

[Chris Packham]

Hi,

I think this has come up before [1],[2] but we ran into this at $dayjob today.
Our default MUA has an annoying habit of using a non RFC822 date format when
saving an email as plaintext. This means the first 12 days of every month we
run into the ambiguous date problem (our date convention is dd/mm/yy).

I see code in date.c for refusing a date in the future which would have caught
this but it doesn't appear to be working for us.

Following this is a patch adding a testcase for this. With the following
results:

  ok 1 - apply patch with ambiguous date
  not ok 2 - check ambiguous date # TODO known breakage
  ok 3 - apply patch with european date separator
  ok 4 - check european date
  # still have 1 known breakage(s)
  # passed all remaining 3 test(s)
  1..4

Thanks,
Chris

--
[1] - http://thread.gmane.org/gmane.comp.version-control.git/18412/focus=18417
[2] - http://thread.gmane.org/gmane.comp.version-control.git/84512/focus=85735

[PATCH] Add test for ambiguous patch dates

[Chris Packham]

--
This testcase is only good for the next couple of months. For a longer term
test the current time would need to be set in the test setup.

---
 t/t4255-am-author-date.sh |   85 +++++++++++++++++++++++++++++++++++++++++++++
 1 file changed, 85 insertions(+)
 create mode 100755 t/t4255-am-author-date.sh

diff --git a/t/t4255-am-author-date.sh b/t/t4255-am-author-date.sh
new file mode 100755
index 0000000..62bceee
--- /dev/null
+++ b/t/t4255-am-author-date.sh
@@ -0,0 +1,85 @@
+#!/bin/sh
+
+test_description='git am with ambiguous date'
+. ./test-lib.sh
+
+cat >patch.diff <<EOF
+From:   A U Thor <au.thor@example.com>
+To:     C O Mmitter <co.mmitter@example.com>
+Date:   12/9/2012 12:00 AM
+Subject:       [PATCH] add file.txt
+---
+ file.txt |    7 +++++++
+ 1 file changed, 7 insertions(+)
+ create mode 100644 file.txt
+
+diff --git a/file.txt b/file.txt
+new file mode 100644
+index 0000000..fe745d6
+--- /dev/null
++++ b/file.txt
+@@ -0,0 +1,7 @@
++Lorem ipsum dolor sit amet, consectetur adipiscing elit. Aliquam pulvinar
++tempus ligula vitae ornare. Vestibulum ante ipsum primis in faucibus orci
++luctus et ultrices posuere cubilia Curae; Aenean dapibus mauris non quam
++commodo a porta sapien suscipit. Mauris venenatis, dui nec malesuada mattis,
++ante mauris ornare ipsum, ac tincidunt ipsum lectus aliquet tortor. Nulla ipsum
++felis, egestas at condimentum quis, accumsan nec arcu. Phasellus fringilla
++viverra tempus. Integer vel rhoncus odio.
+EOF
+
+test_expect_success 'apply patch with ambiguous date' '
+	git am patch.diff
+'
+
+cat >expected <<EOF
+Date:   Wed Sep 12 00:00:00 2012 +0000
+EOF
+
+test_expect_failure 'check ambiguous date' '
+	git show HEAD | grep Date >actual &&
+	test_cmp expected actual
+'
+
+cat >patch.diff <<EOF
+From:   A N Other <an.other@example.com>
+To:     C O Mmitter <co.mmitter@example.com>
+Date:   12.9.2012 12:00 AM
+Subject:       [PATCH] update file.txt
+---
+ file.txt |    9 +++++++++
+ 1 file changed, 9 insertions(+)
+
+diff --git a/file.txt b/file.txt
+index fe745d6..cd45361 100644
+--- a/file.txt
++++ b/file.txt
+@@ -5,3 +5,12 @@ commodo a porta sapien suscipit. Mauris venenatis, dui nec malesuada mattis,
+ ante mauris ornare ipsum, ac tincidunt ipsum lectus aliquet tortor. Nulla ipsum
+ felis, egestas at condimentum quis, accumsan nec arcu. Phasellus fringilla
+ viverra tempus. Integer vel rhoncus odio.
++
++Donec et ante eu mi aliquam sodales non ut massa. Nullam a luctus dui. Etiam ac
++eros elit. Pellentesque habitant morbi tristique senectus et netus et malesuada
++fames ac turpis egestas. Curabitur commodo ligula id leo iaculis vel lobortis
++leo pulvinar. Aenean adipiscing cursus arcu quis consectetur. Morbi eget lectus
++nec neque interdum lacinia. Nam quis metus eget ligula faucibus imperdiet in et
++ligula. Aenean eu urna sit amet metus sagittis interdum non cursus orci.
++Maecenas imperdiet feugiat tellus, non ultrices nulla dictum sed. Nulla vel
++lorem ac massa euismod faucibus et ut leo.
+EOF
+
+test_expect_success 'apply patch with european date separator' '
+	git am patch.diff
+'
+
+cat >expected <<EOF
+Date:   Wed Sep 12 00:00:00 2012 +0000
+EOF
+
+test_expect_success 'check european date' '
+	git show HEAD | grep Date >actual &&
+	test_cmp expected actual
+'
+
+test_done
-- 
1.7.10.4

Re: Ambiguous date handling

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> Our default MUA has an annoying habit of using a non RFC822 date format when
> saving an email as plaintext. This means the first 12 days of every month we
> run into the ambiguous date problem (our date convention is dd/mm/yy).
>
> I see code in date.c for refusing a date in the future which would have caught
> this...

The most sane thing to do when you know that your MUA *consistently*
does dd/mm/yy (even though it may annoy you) is to massage its
output before feeding it to Git.  And it should be a very simple
matter of a one-liner filter, no?

Regardless of the correctness of that "we reject timestamps way into
the future" logic, it should be taken as the last resort.  If you
are on September 1st, both 9/12 and 12/9 will look like into the
future for more than ten days (which is the cut-off, I think).  If
you are on December 28th, both look like sufficiently in the past.

Re: Ambiguous date handling

[Chris Packham]

On 09/12/2012 09:48 PM, Junio C Hamano wrote:
> Chris Packham <judge.packham@gmail.com> writes:
> 
>> Our default MUA has an annoying habit of using a non RFC822 date format when
>> saving an email as plaintext. This means the first 12 days of every month we
>> run into the ambiguous date problem (our date convention is dd/mm/yy).
>>
>> I see code in date.c for refusing a date in the future which would have caught
>> this...
> 
> The most sane thing to do when you know that your MUA *consistently*
> does dd/mm/yy (even though it may annoy you) is to massage its
> output before feeding it to Git.  And it should be a very simple
> matter of a one-liner filter, no?

Consistent as long as you save as the default .txt. Some people have
trained themselves to use the save as .eml option which uses RFC822
style output. sed 's|Date: (\d+)/(\d+)/(\d+)|\1.\2.\3|' should correct
the former and ignore the latter. Could this be done in a applypatch-msg
hook?

> 
> Regardless of the correctness of that "we reject timestamps way into
> the future" logic, it should be taken as the last resort.  If you
> are on September 1st, both 9/12 and 12/9 will look like into the
> future for more than ten days (which is the cut-off, I think).  If
> you are on December 28th, both look like sufficiently in the past.
> 

Duly noted. And I'm implying that the reject timestamps in future isn't
actually working. I've just started looking at t0006-date.sh so see if I
can prove it.

Re: Ambiguous date handling

[Junio C Hamano]

Chris Packham <judge.packham@gmail.com> writes:

> Consistent as long as you save as the default .txt. Some people have
> trained themselves to use the save as .eml option which uses RFC822
> style output.

Yuck.

> Could this be done in a applypatch-msg
> hook?

Isn't the hook about fixing up the log message?  Also I do not think
the name of the original file is given to the hook, so there is no
sufficient information to allow it to switch between two behaviours
based on .txt or .eml.

But if you are massaging the _input_ to "git am", then you can
certainly do the massaging even _before_ you feed it to "git am", no?

We could think about adding a new hook to "git am", though.  It
cannot just be an option to "git am" (or "git mailinfo") that says
"if the input is .txt, assume European date order for \d+/\d+/\d+
dates, and otherwise assume US style", as that is too specific to
your particular set-up and will not match general needs.  If we were
to add such a hook, $GIT_DIR/hooks/am-input-filter might look
something like this (it is left as an exercise to enhance it to
avoid munging a payload outside the header that happens to begin
with "Date: "):

	#!/bin/sh
        case "$#" in
	0)
		cat
        	;;
	*)
        	for i
                do
                	case "$i" in
		        *.txt)
				sed -e 's/^\(Date: \)(\d+/)(\d+/)(\d+)/\1\3\2\4/' "$i"
				;;
			*)
                        	cat "$i"
				;;
			esac
		done
                ;;
	esac

and then teach "am" to use the hook, perhaps like the attached.

But at that point, wouldn't it be far simpler and cleaner if you did

	$ my-mbox-munge mail.txt | git am

in the first place?

 git-am.sh | 11 ++++++++++-
 1 file changed, 10 insertions(+), 1 deletion(-)

diff --git i/git-am.sh w/git-am.sh
index c682d34..42654a0 100755
--- i/git-am.sh
+++ w/git-am.sh
@@ -265,7 +265,16 @@ split_patches () {
 		else
 		    keep_cr=
 		fi
-		git mailsplit -d"$prec" -o"$dotest" -b $keep_cr -- "$@" > "$dotest/last" ||
+
+		if test -x "$GIT_DIR"/hooks/am-input-filter
+		then
+			mif="$GIT_DIR"/hooks/am-input-filter
+		else
+			mif=cat
+		fi
+
+		"$mif" "$@" |
+		git mailsplit -d"$prec" -o"$dotest" -b $keep_cr >"$dotest/last" ||
 		clean_abort
 		;;
 	stgit-series)