# [PATCH] builtin-commit: add --date option 12 messages from 2009-12-01 to 2009-12-02. Participants: Miklos Vajna, Junio C Hamano, Jeff King. Thread: https://gitlist.dev/t/21802 ## Miklos Vajna, 2009-12-01 00:27 Subject: [PATCH] builtin-commit: add --date option Message-ID: <1259627252-21615-1-git-send-email-vmiklos@frugalware.org> URL: https://gitlist.dev/e/1259627252-21615-1-git-send-email-vmiklos%40frugalware.org ``` 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 --- Documentation/git-commit.txt | 6 +++++- builtin-commit.c | 6 +++++- t/t7501-commit.sh | 15 +++++++++++++++ 3 files changed, 25 insertions(+), 2 deletions(-) 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] [--amend] [--dry-run] [(-c | -C) ] [-F | -m ] [--allow-empty] [--no-verify] [-e] [--author=] - [--cleanup=] [--] [[-i | -o ]...] + [--date=] [--cleanup=] [--] [[-i | -o ]...] DESCRIPTION ----------- @@ -80,6 +80,10 @@ OPTIONS an existing commit that matches the given string and its author name is used. +--date=:: + Override the date used in the commit. The format is the Git + native one and is ` SP `. + -m :: --message=:: Use the given 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, 2009-12-02 17:11 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202171117.GY31763@genesis.frugalware.org> URL: https://gitlist.dev/e/20091202171117.GY31763%40genesis.frugalware.org In-Reply-To: <1259627252-21615-1-git-send-email-vmiklos@frugalware.org> ``` On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna wrote: > +--date=:: > + Override the date used in the commit. The format is the Git > + native one and is ` SP `. 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) 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, 2009-12-02 17:35 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <7vpr6xgtxn.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vpr6xgtxn.fsf%40alter.siamese.dyndns.org In-Reply-To: <20091202171117.GY31763@genesis.frugalware.org> ``` Miklos Vajna writes: > On Tue, Dec 01, 2009 at 01:27:32AM +0100, Miklos Vajna wrote: >> +--date=:: >> + Override the date used in the commit. The format is the Git >> + native one and is ` SP `. > > 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) > > 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, 2009-12-02 17:55 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <7vaay1gt0z.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vaay1gt0z.fsf%40alter.siamese.dyndns.org In-Reply-To: <7vpr6xgtxn.fsf@alter.siamese.dyndns.org> ``` Junio C Hamano writes: > Miklos Vajna 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) >> >> 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) ``` ## Jeff King, 2009-12-02 19:24 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202192425.GC30778@coredump.intra.peff.net> URL: https://gitlist.dev/e/20091202192425.GC30778%40coredump.intra.peff.net In-Reply-To: <7vpr6xgtxn.fsf@alter.siamese.dyndns.org> ``` 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. -Peff ``` ## Jeff King, 2009-12-02 19:26 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202192614.GD30778@coredump.intra.peff.net> URL: https://gitlist.dev/e/20091202192614.GD30778%40coredump.intra.peff.net In-Reply-To: <1259627252-21615-1-git-send-email-vmiklos@frugalware.org> ``` 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, 2009-12-02 19:38 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <7vpr6xcgki.fsf@alter.siamese.dyndns.org> URL: https://gitlist.dev/e/7vpr6xcgki.fsf%40alter.siamese.dyndns.org In-Reply-To: <20091202192614.GD30778@coredump.intra.peff.net> ``` Jeff King writes: > 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, 2009-12-02 20:33 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202203351.GB31763@genesis.frugalware.org> URL: https://gitlist.dev/e/20091202203351.GB31763%40genesis.frugalware.org In-Reply-To: <20091202192425.GC30778@coredump.intra.peff.net> ``` On Wed, Dec 02, 2009 at 02:24:25PM -0500, Jeff King wrote: > 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". ``` ## Miklos Vajna, 2009-12-02 21:07 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202210751.GC31763@genesis.frugalware.org> URL: https://gitlist.dev/e/20091202210751.GC31763%40genesis.frugalware.org In-Reply-To: <7vpr6xcgki.fsf@alter.siamese.dyndns.org> ``` On Wed, Dec 02, 2009 at 11:38:05AM -0800, Junio C Hamano 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: 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, 2009-12-02 21:40 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202214034.GD31763@genesis.frugalware.org> URL: https://gitlist.dev/e/20091202214034.GD31763%40genesis.frugalware.org In-Reply-To: <7vaay1gt0z.fsf@alter.siamese.dyndns.org> ``` On Wed, Dec 02, 2009 at 09:55:24AM -0800, Junio C Hamano wrote: > 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... ;) ``` ## Miklos Vajna, 2009-12-02 22:12 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202221232.GF31763@genesis.frugalware.org> URL: https://gitlist.dev/e/20091202221232.GF31763%40genesis.frugalware.org In-Reply-To: <20091202192614.GD30778@coredump.intra.peff.net> ``` On Wed, Dec 02, 2009 at 02:26:14PM -0500, Jeff King 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, 2009-12-02 22:22 Subject: Re: [PATCH] builtin-commit: add --date option Message-ID: <20091202222257.GA9691@coredump.intra.peff.net> URL: https://gitlist.dev/e/20091202222257.GA9691%40coredump.intra.peff.net In-Reply-To: <20091202221232.GF31763@genesis.frugalware.org> ``` 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. ;) > > 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 ```