threads / patch / 21802

patchbuiltin-commit: add --date option

Subject: [PATCH] builtin-commit: add --date option

## tl;dr

12 messages between Dec 1, 2009 and Dec 2, 2009. Diffs are folded; open one to read it.

replies: 11people: 3as markdown or json

Miklos Vajna· Dec 1, 2009, 00:27 UTC · lore

This is useful in case git commit --amend is used but the user wants to set the date of the new commit to a specified one, since GIT_AUTHOR_DATE is ignored in such a situation.

Signed-off-by: Miklos Vajna <vmiklos@frugalware.org>
---
 Documentation/git-commit.txt |    6 +++++-
 builtin-commit.c             |    6 +++++-
 t/t7501-commit.sh            |   15 +++++++++++++++
 3 files changed, 25 insertions(+), 2 deletions(-)
Show changes to 3 files +25 −2

Documentation/git-commit.txt, builtin-commit.c, t/t7501-commit.sh

diff --git a/Documentation/git-commit.txt b/Documentation/git-commit.txt
index 3ea80c8..3b9a7c3 100644
--- a/Documentation/git-commit.txt
+++ b/Documentation/git-commit.txt
@@ -11,7 +11,7 @@ SYNOPSIS
 'git commit' [-a | --interactive] [-s] [-v] [-u<mode>] [--amend] [--dry-run]
 	   [(-c | -C) <commit>] [-F <file> | -m <msg>]
 	   [--allow-empty] [--no-verify] [-e] [--author=<author>]
-	   [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
+	   [--date=<date>] [--cleanup=<mode>] [--] [[-i | -o ]<file>...]
 
 DESCRIPTION
 -----------
@@ -80,6 +80,10 @@ OPTIONS
 	an existing commit that matches the given string and its author
 	name is used.
 
+--date=<date>::
+	Override the date used in the commit.  The format is the Git
+	native one and is `<time> SP <offutc>`.
+
 -m <msg>::
 --message=<msg>::
 	Use the given <msg> as the commit message.
diff --git a/builtin-commit.c b/builtin-commit.c
index 09d2840..594328e 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -52,7 +52,7 @@ static char *edit_message, *use_message;
 static char *author_name, *author_email, *author_date;
 static int all, edit_flag, also, interactive, only, amend, signoff;
 static int quiet, verbose, no_verify, allow_empty, dry_run;
-static char *untracked_files_arg;
+static char *untracked_files_arg, *force_date;
 /*
  * The default commit message cleanup mode will remove the lines
  * beginning with # (shell comments) and leading and trailing
@@ -90,6 +90,7 @@ static struct option builtin_commit_options[] = {
 
 	OPT_FILENAME('F', "file", &logfile, "read log from file"),
 	OPT_STRING(0, "author", &force_author, "AUTHOR", "override author for commit"),
+	OPT_STRING(0, "date", &force_date, "DATE", "override date for commit"),
 	OPT_CALLBACK('m', "message", &message, "MESSAGE", "specify commit message", opt_parse_m),
 	OPT_STRING('c', "reedit-message", &edit_message, "COMMIT", "reuse and edit message from specified commit "),
 	OPT_STRING('C', "reuse-message", &use_message, "COMMIT", "reuse message from specified commit"),
@@ -409,6 +410,9 @@ static void determine_author_info(void)
 		email = xstrndup(lb + 2, rb - (lb + 2));
 	}
 
+	if (force_date)
+		date = force_date;
+
 	author_name = name;
 	author_email = email;
 	author_date = date;
diff --git a/t/t7501-commit.sh b/t/t7501-commit.sh
index a603f6d..a529701 100755
--- a/t/t7501-commit.sh
+++ b/t/t7501-commit.sh
@@ -211,6 +211,21 @@ test_expect_success 'amend commit to fix author' '
 
 '
 
+test_expect_success 'amend commit to fix date' '
+
+	test_tick &&
+	newtick=$GIT_AUTHOR_DATE &&
+	git reset --hard &&
+	git cat-file -p HEAD |
+	sed -e "s/author.*/author $author $newtick/" \
+		-e "s/^\(committer.*> \).*$/\1$GIT_COMMITTER_DATE/" > \
+		expected &&
+	git commit --amend --date="$newtick" &&
+	git cat-file -p HEAD > current &&
+	test_cmp expected current
+
+'
+
 test_expect_success 'sign off (1)' '
 
 	echo 1 >positive &&
-- 
1.6.5.2
Miklos Vajna· Dec 2, 2009, 17:11 UTC · re: Miklos Vajna · lore

Re: [PATCH] builtin-commit: add --date option

On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
> +--date=<date>::
> +	Override the date used in the commit.  The format is the Git
> +	native one and is `<time> SP <offutc>`.

I just noticed that fmt_ident() calls parse_date(), so other formats are supported as well.

Is there any documentation describing what does parse_date() accept?

Based on t0006-date.sh and the comments in the source, I see 3 supported formats:

1) <unix timestamp> <timezone>
2) A format like 2008-12-02 18:04:00
3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)
Thanks.
Junio C Hamano· Dec 2, 2009, 17:35 UTC · re: Miklos Vajna · lore

Re: [PATCH] builtin-commit: add --date option

Miklos Vajna <vmiklos@frugalware.org> writes:
Show 18 quoted lines
> On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna <vmiklos@frugalware.org> wrote:
>> +--date=<date>::
>> +	Override the date used in the commit.  The format is the Git
>> +	native one and is `<time> SP <offutc>`.
>
> I just noticed that fmt_ident() calls parse_date(), so other formats are
> supported as well.
>
> Is there any documentation describing what does parse_date() accept?
>
> Based on t0006-date.sh and the comments in the source, I see 3 supported
> formats:
>
> 1) <unix timestamp> <timezone>
>
> 2) A format like 2008-12-02 18:04:00
>
> 3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)

The above are all supported (you can label 2 as ISO even though the official ISO8601 wants "T" instead of " " between date and time).

For more amusing ones, see
    http://article.gmane.org/gmane.comp.version-control.git/12241
and follow the discussion there ;-)
Junio C Hamano· Dec 2, 2009, 17:55 UTC · re: Junio C Hamano · lore

Re: [PATCH] builtin-commit: add --date option

Junio C Hamano <gitster@pobox.com> writes:
Show 21 quoted lines
> Miklos Vajna <vmiklos@frugalware.org> writes:
> ...
>> Is there any documentation describing what does parse_date() accept?
>>
>> Based on t0006-date.sh and the comments in the source, I see 3 supported
>> formats:
>>
>> 1) <unix timestamp> <timezone>
>>
>> 2) A format like 2008-12-02 18:04:00
>>
>> 3) RFC 2822 (Thu, 21 Dec 2000 16:01:07 +0200)
>
> The above are all supported (you can label 2 as ISO even though the
> official ISO8601 wants "T" instead of " " between date and time).
>
> For more amusing ones, see
>
>     http://article.gmane.org/gmane.comp.version-control.git/12241
>
> and follow the discussion there ;-)

I should have mentioned ones with more importance in real-life before referring you to the amusing ones: US and European dates.

date.c::match_multi_number() groks these:
    mm/dd/yy[yy] (US)
    dd.mm.yy[yy] (European)
Miklos Vajna· Dec 2, 2009, 21:40 UTC · re: Junio C Hamano · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 09:55:24AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
Show 7 quoted lines
> I should have mentioned ones with more importance in real-life before
> referring you to the amusing ones: US and European dates.
> 
> date.c::match_multi_number() groks these:
> 
>     mm/dd/yy[yy] (US)
>     dd.mm.yy[yy] (European)

As far as I see these are also about approxidate() as well. But thanks for the ISO number, now I have enough info to add documentation about it. Patch is coming soon... ;)

Jeff King· Dec 2, 2009, 19:24 UTC · re: Junio C Hamano · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 09:35:48AM -0800, Junio C Hamano wrote:
Show 10 quoted lines
> > Is there any documentation describing what does parse_date() accept?
> [...]
> The above are all supported (you can label 2 as ISO even though the
> official ISO8601 wants "T" instead of " " between date and time).
> 
> For more amusing ones, see
> 
>     http://article.gmane.org/gmane.comp.version-control.git/12241
> 
> and follow the discussion there ;-)

Aren't the amusing ones the result of approxidate, and not parse_date? At least that is my recollection from working on the date code when I ate 30 hot dogs last August.

-Peff
Miklos Vajna· Dec 2, 2009, 20:33 UTC · re: Jeff King · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 02:24:25PM -0500, Jeff King <peff@peff.net> wrote:
Show 16 quoted lines
> On Wed, Dec 02, 2009 at 09:35:48AM -0800, Junio C Hamano wrote:
> 
> > > Is there any documentation describing what does parse_date() accept?
> > [...]
> > The above are all supported (you can label 2 as ISO even though the
> > official ISO8601 wants "T" instead of " " between date and time).
> > 
> > For more amusing ones, see
> > 
> >     http://article.gmane.org/gmane.comp.version-control.git/12241
> > 
> > and follow the discussion there ;-)
> 
> Aren't the amusing ones the result of approxidate, and not parse_date?
> At least that is my recollection from working on the date code when I
> ate 30 hot dogs last August.

I think you are right, at least --date="2008-12-02 18:04:00" works here, but not --date="teatime".

Jeff King· Dec 2, 2009, 19:26 UTC · re: Miklos Vajna · lore

Re: [PATCH] builtin-commit: add --date option

On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna wrote:
> This is useful in case git commit --amend is used but the user wants to
> set the date of the new commit to a specified one, since GIT_AUTHOR_DATE
> is ignored in such a situation.

Do you really want to set the date to something arbitrary, or do you just want to set it to "now"? If the latter case, do you really just want the recently discussed --reset-author?

Also, is there a good reason why GIT_AUTHOR_DATE is not respected in this case? If not, should we simply be fixing that bug instead?

-Peff
Junio C Hamano· Dec 2, 2009, 19:38 UTC · re: Jeff King · lore

Re: [PATCH] builtin-commit: add --date option

Jeff King <peff@peff.net> writes:
Show 6 quoted lines
> Do you really want to set the date to something arbitrary, or do you
> just want to set it to "now"? If the latter case, do you really just
> want the recently discussed --reset-author?
>
> Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
> this case?  If not, should we simply be fixing that bug instead?

I expect I won't be on the list for the rest of the day (I've started the preparation to tag -rc1), but I have two-and-half points before this discussion goes too far:

 - The "--reset-author" patch will be in 1.6.6 (it is already in 'master'
   yesterday, I think);
 - We should honor GIT_AUTHOR_DATE if --reset-author is given.
 - I _think_ we should ignore GIT_AUTHOR_DATE if --reset-author is not
   given, as --amend/-c/-C is stronger for being command line options than
   an environment variable.

So I do not think --date is something we urgently need, even though it might be nice to have it to be consistent with --author.

Miklos Vajna· Dec 2, 2009, 21:07 UTC · re: Junio C Hamano · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano <gitster@pobox.com> wrote:
>  - We should honor GIT_AUTHOR_DATE if --reset-author is given.
This is already a situation, as far as I see.
>  - I _think_ we should ignore GIT_AUTHOR_DATE if --reset-author is not
>    given, as --amend/-c/-C is stronger for being command line options than
>    an environment variable.

We already ignore GIT_AUTHOR_DATE if --reset-author is not given, also when I change it like this:

Show changes to builtin-commit.c +2 −1
diff --git a/builtin-commit.c b/builtin-commit.c
index e93a647..7234c7d 100644
--- a/builtin-commit.c
+++ b/builtin-commit.c
@@ -397,7 +397,8 @@ static void determine_author_info(void)

                name = xstrndup(a + 8, lb - (a + 8));
                email = xstrndup(lb + 2, rb - (lb + 2));
-               date = xstrndup(rb + 2, eol - (rb + 2));
+               if (!date)
+                       date = xstrndup(rb + 2, eol - (rb + 2));
        }

        if (force_author) {

tests 27, 33, 38 and 39 fail in t7501-commit.sh.
Miklos Vajna· Dec 2, 2009, 22:12 UTC · re: Jeff King · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 02:26:14PM -0500, Jeff King <peff@peff.net> wrote:
> Do you really want to set the date to something arbitrary, or do you
> just want to set it to "now"? If the latter case, do you really just
> want the recently discussed --reset-author?

No, I want to set it to two days ago, for example when I forgot to commit at the end of the day and I notice it two days later before I start to work on something again.

> Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
> this case?  If not, should we simply be fixing that bug instead?

GIT_AUTHOR_DATE is respected by --reset-author, just given that we have --author already, using environment variables for a user can be uncomfortable.

Jeff King· Dec 2, 2009, 22:22 UTC · re: Miklos Vajna · lore

Re: [PATCH] builtin-commit: add --date option

On Wed, Dec 02, 2009 at 11:12:32PM +0100, Miklos Vajna wrote:
> No, I want to set it to two days ago, for example when I forgot to
> commit at the end of the day and I notice it two days later before I
> start to work on something again.

OK. I think you are being overly meticulous about your commit date, but that is your right. ;)

Show 6 quoted lines
> > Also, is there a good reason why GIT_AUTHOR_DATE is not respected in
> > this case?  If not, should we simply be fixing that bug instead?
> 
> GIT_AUTHOR_DATE is respected by --reset-author, just given that we have
> --author already, using environment variables for a user can be
> uncomfortable.
OK, I can agree with that reasoning.
-Peff

← back to recent threads