Re: [PATCH] git-am: flag suspiciously old or futuristic commits

[Paul Gortmaker <paul.gortmaker@windriver.com>]

On 2015-07-30 12:35 AM, Jacob Keller wrote:
> On Wed, Jul 29, 2015 at 3:20 PM, Stefan Beller <sbeller@google.com> wrote:
>> On Wed, Jul 29, 2015 at 3:01 PM, Paul Gortmaker
>> <paul.gortmaker@windriver.com> wrote:
>>> The linux kernel repository has some commits in it with dates from
>>> the year 1970 and also 2030 (and possibly others).  We probably shouldi
>>> warn people when the dates look suspect.
>>>
>>> For commits in the future,  note that a committer in Australia
>>> could commit on New Years Day, and send it to a maintainer in North
>>> America and that would trip the notification on the maintainer's
>>> New Years Eve.  But that is unlikely, and the note is still
>>> correct; that the commit is from a future year.
>>>
>>> For commits in the past, I chose a somewhat arbitrary 30 year
>>> limit, which will allow stuff from post 1985; the thought being
>>> that someone might want to import an old repo into git from some
>>> other SCM.  We could alternatively set it to 5, which would then
>>> catch computers with a dead CMOS battery, at the risk of pestering
>>> the hypothetical museum curator of old bits.
>>>
>>> Sample output:
>>>
>>> paul@builder:~/git/linux-head$ grep Date: *patch
>>> future.patch:Date: Sat, 18 Jul 2037 21:22:19 -0400
>>> past.patch:Date: Sat, 18 Jul 1977 21:22:19 -0400
>>>
>>> paul@builder:~/git/linux-head$ git am future.patch
>>> note: commit is from future year 2037.
>>> Applying: arch/sh: make heartbeat driver explicitly non-modular
>>> paul@builder:~/git/linux-head$ git reset --hard HEAD~ > /dev/null
>>> paul@builder:~/git/linux-head$ git am past.patch
>>> note: commit is from implausibly old year 1977.
>>> Applying: arch/sh: make heartbeat driver explicitly non-modular
>>> paul@builder:~/git/linux-head$
>>>
>>> Signed-off-by: Paul Gortmaker <paul.gortmaker@windriver.com>
>>> ---
>>>  git-am.sh | 15 +++++++++++++++
>>>  1 file changed, 15 insertions(+)
>>>
>>> diff --git a/git-am.sh b/git-am.sh
>>
>> [+cc paul tan, who rewrote am in c as a GSoC project.]
>>
>>> index 3af351ffaaf3..ff6deb8047a4 100755
>>> --- a/git-am.sh
>>> +++ b/git-am.sh
>>> @@ -766,6 +766,21 @@ To restore the original branch and stop patching run \"\$cmdline --abort\"."
>>>                 stop_here $this
>>>         fi
>>>
>>> +       if test -n "$GIT_AUTHOR_DATE"
>>> +       then
>>> +               THIS_YEAR=`date +%Y`
>>> +               TOO_OLD=$(expr $THIS_YEAR - 30)
>>> +               TOO_NEW=$(expr $THIS_YEAR + 1)
>>> +               GIT_AUTHOR_YEAR=`date -d "$GIT_AUTHOR_DATE" +%Y`
>>
>> Would it make sense to not operate on year but on unix time, so the problem
>> you mentioned in the commit message goes away?
>>
>> Another thought:
>> Having this check in am seems a bit arbitrary to me (or rather
>> workflow adapted ;) as
>> we could also check in commit or pull (not sure if I actually mean the
>> fetch or merge thereof)
>>
> 
> I think this makes most sense in am, as it is most likely to show up,
> in my mind, due to a format-patch mistake. If we do it during pull,
> would we just warn? how would we reject commits? that doesn't really
> fit..

So, it turns out this breaks t3400-rebase test since I was not aware
of the git internal date format (which that test suite uses).

And on top of that, in talking with a *BSD user, it seems the "-d"
flag isn't universal.  Over in that camp it means set DST offset. :(

So while I think the idea is sound, and worthwhile, scripting it will
become a lot more cumbersome and klunky.  If git-am becomes C, then
it would be easier to handle there, I think....

Paul.
--

> 
> We can't do it during commit, as obviously the broken machine will
> likely not be able to notice it at all.. We could check remotes during
> push but that doesn't solve this either..
> 
> Maybe just emitting a warning during a fetch or am (since am doesn't
> use pull) would make the most sense?
> 
> I don't think we can reject things when doing a fetch though, but we could warn.
> 
> Regards,
> Jake
>