| Message ID | 20190812194301.5655-6-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: support more options | expand |
Hi Rohit This is looking better - there are a couple of memory management issues and minor nit-picks but apart from that it looks good. On 12/08/2019 20:42, Rohit Ashiwal wrote: > rebase am already has this flag to "lie" about the author date > by changing it to the committer (current) date. Let's add the same > for interactive machinery. > > Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> > --- > Documentation/git-rebase.txt | 6 +-- > builtin/rebase.c | 21 +++++++--- > sequencer.c | 55 ++++++++++++++++++++++++- > sequencer.h | 1 + > t/t3433-rebase-options-compatibility.sh | 16 +++++++ > 5 files changed, 88 insertions(+), 11 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 697ce8e6ff..24ad2dda0b 100644 > --- a/Documentation/git-rebase.txt > +++ b/Documentation/git-rebase.txt > @@ -388,8 +388,8 @@ See also INCOMPATIBLE OPTIONS below. > as the committer date. This implies --force-rebase. > > --ignore-date:: > - This flag is passed to 'git am' to change the author date > - of the rebased commits (see linkgit:git-am[1]). > + Instead of using the given author date, re-set reset is not hyphenated > it to the value > + same > as committer (current) date. This implies --force-rebase. s/value same as committer/same value as the committer/ > + > See also INCOMPATIBLE OPTIONS below. > > @@ -526,7 +526,6 @@ INCOMPATIBLE OPTIONS > > The following options: > > - * --ignore-date > * --whitespace > * -C > > @@ -552,6 +551,7 @@ In addition, the following pairs of options are incompatible: > * --preserve-merges and --rebase-merges > * --preserve-merges and --ignore-whitespace > * --preserve-merges and --committer-date-is-author-date > + * --preserve-merges and --ignore-date > * --rebase-merges and --ignore-whitespace > * --rebase-merges and --strategy > * --rebase-merges and --strategy-option > diff --git a/builtin/rebase.c b/builtin/rebase.c > index b1039f8db0..ed58ca8e5a 100644 > --- a/builtin/rebase.c > +++ b/builtin/rebase.c > @@ -83,6 +83,7 @@ struct rebase_options { > char *gpg_sign_opt; > int autostash; > int committer_date_is_author_date; > + int ignore_date; > char *cmd; > int allow_empty_message; > int rebase_merges, rebase_cousins; > @@ -117,6 +118,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) > replay.reschedule_failed_exec = opts->reschedule_failed_exec; > replay.committer_date_is_author_date = > opts->committer_date_is_author_date; > + replay.ignore_date = opts->ignore_date; > replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); > replay.strategy = opts->strategy; > > @@ -536,7 +538,10 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) > warning(_("--[no-]rebase-cousins has no effect without " > "--rebase-merges")); > > - if (opts.committer_date_is_author_date) > + if (opts.ignore_date) > + opts.committer_date_is_author_date = 0; > + if (opts.committer_date_is_author_date || > + opts.ignore_date) > opts.flags |= REBASE_FORCE; Is any of this used by rebase--preserve-merges.sh? > return !!run_rebase_interactive(&opts, command); > @@ -989,6 +994,8 @@ static int run_am(struct rebase_options *opts) > argv_array_push(&am.args, "--ignore-whitespace"); > if (opts->committer_date_is_author_date) > argv_array_push(&opts->git_am_opts, "--committer-date-is-author-date"); > + if (opts->ignore_date) > + argv_array_push(&opts->git_am_opts, "--ignore-date"); > if (opts->action && !strcmp("continue", opts->action)) { > argv_array_push(&am.args, "--resolved"); > argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg); > @@ -1435,8 +1442,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > OPT_BOOL(0, "committer-date-is-author-date", > &options.committer_date_is_author_date, > N_("make committer date match author date")), > - OPT_PASSTHRU_ARGV(0, "ignore-date", &options.git_am_opts, NULL, > - N_("passed to 'git am'"), PARSE_OPT_NOARG), > + OPT_BOOL(0, "ignore-date", &options.ignore_date, > + "ignore author date and use current date"), > OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"), > N_("passed to 'git apply'"), 0), > OPT_BOOL(0, "ignore-whitespace", &options.ignore_whitespace, > @@ -1705,13 +1712,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) > state_dir_base, cmd_live_rebase, buf.buf); > } > > - if (options.committer_date_is_author_date) > + if (options.ignore_date) > + options.committer_date_is_author_date = 0; We should probably print an error if the user gives --committer-date-is-author-date and --author-date-is-committer-date (and in cmd_rebase__interactive() if we keep this option there) > + if (options.committer_date_is_author_date || > + options.ignore_date) > options.flags |= REBASE_FORCE; > > for (i = 0; i < options.git_am_opts.argc; i++) { > const char *option = options.git_am_opts.argv[i], *p; > - if (!strcmp(option, "--ignore-date") || > - !strcmp(option, "--whitespace=fix") || > + if (!strcmp(option, "--whitespace=fix") || > !strcmp(option, "--whitespace=strip")) > options.flags |= REBASE_FORCE; > else if (skip_prefix(option, "-C", &p)) { > diff --git a/sequencer.c b/sequencer.c > index e186136ccc..aecd9b4ad8 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -148,6 +148,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") > */ > static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") > static GIT_PATH_FUNC(rebase_path_cdate_is_adate, "rebase-merge/cdate_is_adate") > +static GIT_PATH_FUNC(rebase_path_ignore_date, "rebase-merge/ignore_date") > static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") > static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") > static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") > @@ -919,6 +920,31 @@ static const char *read_author_ident(struct strbuf *buf) > return buf->buf; > } > > +static void ignore_author_date(const char **author) > +{ > + int len = strlen(*author); > + struct ident_split ident; > + struct strbuf new_author = STRBUF_INIT; > + > + split_ident_line(&ident, *author, len); > + len = ident.mail_end - ident.name_begin + 1; > + > + strbuf_addf(&new_author, "%.*s", len, *author); > + datestamp(&new_author); > + *author = strbuf_detach(&new_author, NULL); > +} It's good to see this using the indet api. I think this would be nicer if it took a char* returned the new author rather than changing the function argument, particularly as it does not free the string that is passed in so the ownership is unclear. > + > +static void push_dates(struct child_process *child) > +{ > + time_t now = time(NULL); > + struct strbuf date = STRBUF_INIT; > + > + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); > + argv_array_pushf(&child->args, "--date=%s", date.buf); it doesn't matter but it might have been nicer to set both dates the same way in the environment. > + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); > + strbuf_release(&date); > +} > + > static const char staged_changes_advice[] = > N_("you have staged changes in your working tree\n" > "If these changes are meant to be squashed into the previous commit, run:\n" > @@ -1020,10 +1046,18 @@ static int run_git_commit(struct repository *r, > > if (res <= 0) > res = error_errno(_("could not read '%s'"), defmsg); > - else > + else { > + if (opts->ignore_date) { > + if (!author) > + BUG("ignore-date can only be used with " > + "rebase, which must set the author " > + "before committing the tree"); > + ignore_author_date(&author); Is this leaking the old author? I'd rather see tmp_author = ignore_author_date(author); free(author); author = tmp_author; > + } > res = commit_tree(msg.buf, msg.len, cache_tree_oid, > NULL, &root_commit, author, > opts->gpg_sign); > + } > > strbuf_release(&msg); > strbuf_release(&script); > @@ -1053,6 +1087,8 @@ static int run_git_commit(struct repository *r, > argv_array_push(&cmd.args, "--amend"); > if (opts->gpg_sign) > argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); > + if (opts->ignore_date) > + push_dates(&cmd); > if (defmsg) > argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > else if (!(flags & EDIT_MSG)) > @@ -1515,6 +1551,11 @@ static int try_to_commit(struct repository *r, > > reset_ident_date(); > > + if (opts->ignore_date) { > + ignore_author_date(&author); > + free(author_to_free); Where is author_to_free set? We should always free the old author, see above. > + author_to_free = (char *)author; Why do we need the cast - is author const? Best Wishes Phillip > + } > if (commit_tree_extended(msg->buf, msg->len, &tree, parents, > oid, author, opts->gpg_sign, extra)) { > res = error(_("failed to write commit object")); > @@ -2592,6 +2633,11 @@ static int read_populate_opts(struct replay_opts *opts) > opts->committer_date_is_author_date = 1; > } > > + if (file_exists(rebase_path_ignore_date())) { > + opts->allow_ff = 0; > + opts->ignore_date = 1; > + } > + > if (file_exists(rebase_path_reschedule_failed_exec())) > opts->reschedule_failed_exec = 1; > > @@ -2676,6 +2722,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, > write_file(rebase_path_signoff(), "--signoff\n"); > if (opts->committer_date_is_author_date) > write_file(rebase_path_cdate_is_adate(), "%s", ""); > + if (opts->ignore_date) > + write_file(rebase_path_ignore_date(), "%s", ""); > if (opts->reschedule_failed_exec) > write_file(rebase_path_reschedule_failed_exec(), "%s", ""); > > @@ -3601,6 +3649,8 @@ static int do_merge(struct repository *r, > argv_array_push(&cmd.args, git_path_merge_msg(r)); > if (opts->gpg_sign) > argv_array_push(&cmd.args, opts->gpg_sign); > + if (opts->ignore_date) > + push_dates(&cmd); > > /* Add the tips to be merged */ > for (j = to_merge; j; j = j->next) > @@ -3874,7 +3924,8 @@ static int pick_commits(struct repository *r, > if (opts->allow_ff) > assert(!(opts->signoff || opts->no_commit || > opts->record_origin || opts->edit || > - opts->committer_date_is_author_date)); > + opts->committer_date_is_author_date || > + opts->ignore_date)); > if (read_and_refresh_cache(r, opts)) > return -1; > > diff --git a/sequencer.h b/sequencer.h > index e3881e9275..bf5a79afdb 100644 > --- a/sequencer.h > +++ b/sequencer.h > @@ -44,6 +44,7 @@ struct replay_opts { > int quiet; > int reschedule_failed_exec; > int committer_date_is_author_date; > + int ignore_date; > > int mainline; > > diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh > index b2419a2b75..c060fcd10b 100755 > --- a/t/t3433-rebase-options-compatibility.sh > +++ b/t/t3433-rebase-options-compatibility.sh > @@ -81,4 +81,20 @@ test_expect_success '--committer-date-is-author-date works with interactive back > test_cmp authortime committertime > ' > > +# Checking for +0000 in author time is enough since default > +# timezone is UTC, but the timezone used while committing > +# sets to +0530. > +test_expect_success '--ignore-date works with am backend' ' > + git commit --amend --date="$GIT_AUTHOR_DATE" && > + git rebase --ignore-date HEAD^ && > + git show HEAD --pretty="format:%ai" >authortime && > + grep "+0000" authortime > +' > + > +test_expect_success '--ignore-date works with interactive backend' ' > + git commit --amend --date="$GIT_AUTHOR_DATE" && > + git rebase --ignore-date -i HEAD^ && > + git show HEAD --pretty="format:%ai" >authortime && > + grep "+0000" authortime > +' > test_done >
Phillip Wood <phillip.wood123@gmail.com> writes: >> +static void push_dates(struct child_process *child) >> +{ >> + time_t now = time(NULL); >> + struct strbuf date = STRBUF_INIT; >> + >> + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); >> + argv_array_pushf(&child->args, "--date=%s", date.buf); > > it doesn't matter but it might have been nicer to set both dates the > same way in the environment. > + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); We can see that this date string lacks timezone information, which would likely fall back to whatever timezone the user is in. Is that what we want? I am guessing it is, as we are dealing with "now" timestamp, but wanted to double check. >> + if (opts->ignore_date) { >> + if (!author) >> + BUG("ignore-date can only be used with " >> + "rebase, which must set the author " >> + "before committing the tree"); >> + ignore_author_date(&author); > > Is this leaking the old author? I'd rather see > > tmp_author = ignore_author_date(author); > free(author); > author = tmp_author; Or make sure ignore_author_date() does not leak the original, when it rewrites its parameter. But I have a larger question at the higher design level. Why are we passing a single string "author" around, instead of parsed and split fields, like <name, email, timestamp, tz> tuple? That would allow us to replace only the time part a lot more easily. Would it make the other parts of the code more cumbersome (I didn't check---and if that is the case, then that is a valid reason why we want to stick to the current "a single string 'author' keeps the necessary info for the 4-tuple" design). >> + } >> res = commit_tree(msg.buf, msg.len, cache_tree_oid, >> NULL, &root_commit, author, >> opts->gpg_sign); >> + } >> strbuf_release(&msg); >> strbuf_release(&script); >> @@ -1053,6 +1087,8 @@ static int run_git_commit(struct repository *r, >> argv_array_push(&cmd.args, "--amend"); >> if (opts->gpg_sign) >> argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); >> + if (opts->ignore_date) >> + push_dates(&cmd); >> if (defmsg) >> argv_array_pushl(&cmd.args, "-F", defmsg, NULL); >> else if (!(flags & EDIT_MSG)) >> @@ -1515,6 +1551,11 @@ static int try_to_commit(struct repository *r, >> reset_ident_date(); >> + if (opts->ignore_date) { >> + ignore_author_date(&author); >> + free(author_to_free); > > Where is author_to_free set? We should always free the old author, see > above. Or require callers to pass a free()able memory to ignore_author_date() and have the callee free the original?
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes: > --ignore-date:: > - This flag is passed to 'git am' to change the author date > - of the rebased commits (see linkgit:git-am[1]). > + Instead of using the given author date, re-set it to the value > + same as committer (current) date. This implies --force-rebase. s/to the value same as .* date\./the current time./; The more important thing is that we record the current timestamp as the author date; that timestamp being very close to the committer date of the resulting commit is a mere consequence of the fact that we use the current time for committer date and much less important. Again, I think reset-author-date would be a better synonym to this one.
Hi Junio & Rohit On 13/08/2019 18:21, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > >>> +static void push_dates(struct child_process *child) >>> +{ >>> + time_t now = time(NULL); >>> + struct strbuf date = STRBUF_INIT; >>> + >>> + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); >>> + argv_array_pushf(&child->args, "--date=%s", date.buf); >> >> it doesn't matter but it might have been nicer to set both dates the >> same way in the environment. >> + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); > > We can see that this date string lacks timezone information, which > would likely fall back to whatever timezone the user is in. Is that > what we want? I am guessing it is, as we are dealing with "now" > timestamp, but wanted to double check. I think we probably want to use the local timezone as you suggest >>> + if (opts->ignore_date) { >>> + if (!author) >>> + BUG("ignore-date can only be used with " >>> + "rebase, which must set the author " >>> + "before committing the tree"); >>> + ignore_author_date(&author); >> >> Is this leaking the old author? I'd rather see >> >> tmp_author = ignore_author_date(author); >> free(author); >> author = tmp_author; > > Or make sure ignore_author_date() does not leak the original, when > it rewrites its parameter. > > But I have a larger question at the higher design level. Why are we > passing a single string "author" around, instead of parsed and split > fields, like <name, email, timestamp, tz> tuple? That would allow us > to replace only the time part a lot more easily. Would it make the > other parts of the code more cumbersome (I didn't check---and if > that is the case, then that is a valid reason why we want to stick > to the current "a single string 'author' keeps the necessary info > for the 4-tuple" design). It's a bit of a mess at the moment. There are places where we want a single author data string when calling commit_tree_extended(), and other places where we want to set the name, email and date in the environment when running 'git commit'. For the latter case we use a file which we read and write as the C version just follows what the shell script did. I think carrying round a <name, email, timestamp, tz> tuple would be the sensible way to go and we can build the author string when we need it. Doing that would hopefully eliminate having to read and write the author file so much. I haven't looked at how difficult it would be to change the existing code to do that. We should also really carry the commit message around in a variable and only write it to a file if a pick fails or we are editing the message and running 'git commit'. If we're just using commit_tree_extended() there is no need to be writing the message to a file and then reading it back in again later. Best Wishes Phillip >>> + } >>> res = commit_tree(msg.buf, msg.len, cache_tree_oid, >>> NULL, &root_commit, author, >>> opts->gpg_sign); >>> + } >>> strbuf_release(&msg); >>> strbuf_release(&script); >>> @@ -1053,6 +1087,8 @@ static int run_git_commit(struct repository *r, >>> argv_array_push(&cmd.args, "--amend"); >>> if (opts->gpg_sign) >>> argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); >>> + if (opts->ignore_date) >>> + push_dates(&cmd); >>> if (defmsg) >>> argv_array_pushl(&cmd.args, "-F", defmsg, NULL); >>> else if (!(flags & EDIT_MSG)) >>> @@ -1515,6 +1551,11 @@ static int try_to_commit(struct repository *r, >>> reset_ident_date(); >>> + if (opts->ignore_date) { >>> + ignore_author_date(&author); >>> + free(author_to_free); >> >> Where is author_to_free set? We should always free the old author, see >> above. > > Or require callers to pass a free()able memory to ignore_author_date() > and have the callee free the original? >
On 13/08/2019 22:45, Junio C Hamano wrote: > Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes: > >> --ignore-date:: >> - This flag is passed to 'git am' to change the author date >> - of the rebased commits (see linkgit:git-am[1]). >> + Instead of using the given author date, re-set it to the value >> + same as committer (current) date. This implies --force-rebase. > > s/to the value same as .* date\./the current time./; > > The more important thing is that we record the current timestamp as > the author date; that timestamp being very close to the committer > date of the resulting commit is a mere consequence of the fact that > we use the current time for committer date and much less important. That's an important distinction, particularly if GIT_COMMITTER_DATE is set in the environment - are we aiming to have the author and committer dates match or are we just resetting the author date to now? Rohit - do you know which --ignore-date does in the am based rebase? Best Wishes Phillip > > Again, I think reset-author-date would be a better synonym to this > one. >
Phillip Wood <phillip.wood123@gmail.com> writes: > That's an important distinction, particularly if GIT_COMMITTER_DATE is > set in the environment - are we aiming to have the author and > committer dates match or are we just resetting the author date to now? > Rohit - do you know which --ignore-date does in the am based rebase? The purpose "am --ignore-date" was to ignore "Date:" that came from the patch message, overriding it with the current date. It might have become harder to read in the C version, but "git show v2.0.0:git-am.sh" would be an easier way to read how "--ignore-date" wanted to behave.
On 14/08/2019 20:33, Junio C Hamano wrote: > Phillip Wood <phillip.wood123@gmail.com> writes: > >> That's an important distinction, particularly if GIT_COMMITTER_DATE is >> set in the environment - are we aiming to have the author and >> committer dates match or are we just resetting the author date to now? >> Rohit - do you know which --ignore-date does in the am based rebase? > > The purpose "am --ignore-date" was to ignore "Date:" that came from > the patch message, overriding it with the current date. It might > have become harder to read in the C version, but "git show v2.0.0:git-am.sh" > would be an easier way to read how "--ignore-date" wanted to behave. Thanks for the pointer, looking at the code I agree that --reset-author-date would be a better alias. If GIT_COMMITTER_DATE is not set then the author and committer dates match, otherwise the author date is reset to the current time. The code also shows how we should be combining --ignore-date and --committer-date-is-author-date. Best Wishes Phillip
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 697ce8e6ff..24ad2dda0b 100644 --- a/Documentation/git-rebase.txt +++ b/Documentation/git-rebase.txt @@ -388,8 +388,8 @@ See also INCOMPATIBLE OPTIONS below. as the committer date. This implies --force-rebase. --ignore-date:: - This flag is passed to 'git am' to change the author date - of the rebased commits (see linkgit:git-am[1]). + Instead of using the given author date, re-set it to the value + same as committer (current) date. This implies --force-rebase. + See also INCOMPATIBLE OPTIONS below. @@ -526,7 +526,6 @@ INCOMPATIBLE OPTIONS The following options: - * --ignore-date * --whitespace * -C @@ -552,6 +551,7 @@ In addition, the following pairs of options are incompatible: * --preserve-merges and --rebase-merges * --preserve-merges and --ignore-whitespace * --preserve-merges and --committer-date-is-author-date + * --preserve-merges and --ignore-date * --rebase-merges and --ignore-whitespace * --rebase-merges and --strategy * --rebase-merges and --strategy-option diff --git a/builtin/rebase.c b/builtin/rebase.c index b1039f8db0..ed58ca8e5a 100644 --- a/builtin/rebase.c +++ b/builtin/rebase.c @@ -83,6 +83,7 @@ struct rebase_options { char *gpg_sign_opt; int autostash; int committer_date_is_author_date; + int ignore_date; char *cmd; int allow_empty_message; int rebase_merges, rebase_cousins; @@ -117,6 +118,7 @@ static struct replay_opts get_replay_opts(const struct rebase_options *opts) replay.reschedule_failed_exec = opts->reschedule_failed_exec; replay.committer_date_is_author_date = opts->committer_date_is_author_date; + replay.ignore_date = opts->ignore_date; replay.gpg_sign = xstrdup_or_null(opts->gpg_sign_opt); replay.strategy = opts->strategy; @@ -536,7 +538,10 @@ int cmd_rebase__interactive(int argc, const char **argv, const char *prefix) warning(_("--[no-]rebase-cousins has no effect without " "--rebase-merges")); - if (opts.committer_date_is_author_date) + if (opts.ignore_date) + opts.committer_date_is_author_date = 0; + if (opts.committer_date_is_author_date || + opts.ignore_date) opts.flags |= REBASE_FORCE; return !!run_rebase_interactive(&opts, command); @@ -989,6 +994,8 @@ static int run_am(struct rebase_options *opts) argv_array_push(&am.args, "--ignore-whitespace"); if (opts->committer_date_is_author_date) argv_array_push(&opts->git_am_opts, "--committer-date-is-author-date"); + if (opts->ignore_date) + argv_array_push(&opts->git_am_opts, "--ignore-date"); if (opts->action && !strcmp("continue", opts->action)) { argv_array_push(&am.args, "--resolved"); argv_array_pushf(&am.args, "--resolvemsg=%s", resolvemsg); @@ -1435,8 +1442,8 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) OPT_BOOL(0, "committer-date-is-author-date", &options.committer_date_is_author_date, N_("make committer date match author date")), - OPT_PASSTHRU_ARGV(0, "ignore-date", &options.git_am_opts, NULL, - N_("passed to 'git am'"), PARSE_OPT_NOARG), + OPT_BOOL(0, "ignore-date", &options.ignore_date, + "ignore author date and use current date"), OPT_PASSTHRU_ARGV('C', NULL, &options.git_am_opts, N_("n"), N_("passed to 'git apply'"), 0), OPT_BOOL(0, "ignore-whitespace", &options.ignore_whitespace, @@ -1705,13 +1712,15 @@ int cmd_rebase(int argc, const char **argv, const char *prefix) state_dir_base, cmd_live_rebase, buf.buf); } - if (options.committer_date_is_author_date) + if (options.ignore_date) + options.committer_date_is_author_date = 0; + if (options.committer_date_is_author_date || + options.ignore_date) options.flags |= REBASE_FORCE; for (i = 0; i < options.git_am_opts.argc; i++) { const char *option = options.git_am_opts.argv[i], *p; - if (!strcmp(option, "--ignore-date") || - !strcmp(option, "--whitespace=fix") || + if (!strcmp(option, "--whitespace=fix") || !strcmp(option, "--whitespace=strip")) options.flags |= REBASE_FORCE; else if (skip_prefix(option, "-C", &p)) { diff --git a/sequencer.c b/sequencer.c index e186136ccc..aecd9b4ad8 100644 --- a/sequencer.c +++ b/sequencer.c @@ -148,6 +148,7 @@ static GIT_PATH_FUNC(rebase_path_refs_to_delete, "rebase-merge/refs-to-delete") */ static GIT_PATH_FUNC(rebase_path_gpg_sign_opt, "rebase-merge/gpg_sign_opt") static GIT_PATH_FUNC(rebase_path_cdate_is_adate, "rebase-merge/cdate_is_adate") +static GIT_PATH_FUNC(rebase_path_ignore_date, "rebase-merge/ignore_date") static GIT_PATH_FUNC(rebase_path_orig_head, "rebase-merge/orig-head") static GIT_PATH_FUNC(rebase_path_verbose, "rebase-merge/verbose") static GIT_PATH_FUNC(rebase_path_quiet, "rebase-merge/quiet") @@ -919,6 +920,31 @@ static const char *read_author_ident(struct strbuf *buf) return buf->buf; } +static void ignore_author_date(const char **author) +{ + int len = strlen(*author); + struct ident_split ident; + struct strbuf new_author = STRBUF_INIT; + + split_ident_line(&ident, *author, len); + len = ident.mail_end - ident.name_begin + 1; + + strbuf_addf(&new_author, "%.*s", len, *author); + datestamp(&new_author); + *author = strbuf_detach(&new_author, NULL); +} + +static void push_dates(struct child_process *child) +{ + time_t now = time(NULL); + struct strbuf date = STRBUF_INIT; + + strbuf_addf(&date, "@%"PRIuMAX, (uintmax_t)now); + argv_array_pushf(&child->args, "--date=%s", date.buf); + argv_array_pushf(&child->env_array, "GIT_COMMITTER_DATE=%s", date.buf); + strbuf_release(&date); +} + static const char staged_changes_advice[] = N_("you have staged changes in your working tree\n" "If these changes are meant to be squashed into the previous commit, run:\n" @@ -1020,10 +1046,18 @@ static int run_git_commit(struct repository *r, if (res <= 0) res = error_errno(_("could not read '%s'"), defmsg); - else + else { + if (opts->ignore_date) { + if (!author) + BUG("ignore-date can only be used with " + "rebase, which must set the author " + "before committing the tree"); + ignore_author_date(&author); + } res = commit_tree(msg.buf, msg.len, cache_tree_oid, NULL, &root_commit, author, opts->gpg_sign); + } strbuf_release(&msg); strbuf_release(&script); @@ -1053,6 +1087,8 @@ static int run_git_commit(struct repository *r, argv_array_push(&cmd.args, "--amend"); if (opts->gpg_sign) argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); + if (opts->ignore_date) + push_dates(&cmd); if (defmsg) argv_array_pushl(&cmd.args, "-F", defmsg, NULL); else if (!(flags & EDIT_MSG)) @@ -1515,6 +1551,11 @@ static int try_to_commit(struct repository *r, reset_ident_date(); + if (opts->ignore_date) { + ignore_author_date(&author); + free(author_to_free); + author_to_free = (char *)author; + } if (commit_tree_extended(msg->buf, msg->len, &tree, parents, oid, author, opts->gpg_sign, extra)) { res = error(_("failed to write commit object")); @@ -2592,6 +2633,11 @@ static int read_populate_opts(struct replay_opts *opts) opts->committer_date_is_author_date = 1; } + if (file_exists(rebase_path_ignore_date())) { + opts->allow_ff = 0; + opts->ignore_date = 1; + } + if (file_exists(rebase_path_reschedule_failed_exec())) opts->reschedule_failed_exec = 1; @@ -2676,6 +2722,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name, write_file(rebase_path_signoff(), "--signoff\n"); if (opts->committer_date_is_author_date) write_file(rebase_path_cdate_is_adate(), "%s", ""); + if (opts->ignore_date) + write_file(rebase_path_ignore_date(), "%s", ""); if (opts->reschedule_failed_exec) write_file(rebase_path_reschedule_failed_exec(), "%s", ""); @@ -3601,6 +3649,8 @@ static int do_merge(struct repository *r, argv_array_push(&cmd.args, git_path_merge_msg(r)); if (opts->gpg_sign) argv_array_push(&cmd.args, opts->gpg_sign); + if (opts->ignore_date) + push_dates(&cmd); /* Add the tips to be merged */ for (j = to_merge; j; j = j->next) @@ -3874,7 +3924,8 @@ static int pick_commits(struct repository *r, if (opts->allow_ff) assert(!(opts->signoff || opts->no_commit || opts->record_origin || opts->edit || - opts->committer_date_is_author_date)); + opts->committer_date_is_author_date || + opts->ignore_date)); if (read_and_refresh_cache(r, opts)) return -1; diff --git a/sequencer.h b/sequencer.h index e3881e9275..bf5a79afdb 100644 --- a/sequencer.h +++ b/sequencer.h @@ -44,6 +44,7 @@ struct replay_opts { int quiet; int reschedule_failed_exec; int committer_date_is_author_date; + int ignore_date; int mainline; diff --git a/t/t3433-rebase-options-compatibility.sh b/t/t3433-rebase-options-compatibility.sh index b2419a2b75..c060fcd10b 100755 --- a/t/t3433-rebase-options-compatibility.sh +++ b/t/t3433-rebase-options-compatibility.sh @@ -81,4 +81,20 @@ test_expect_success '--committer-date-is-author-date works with interactive back test_cmp authortime committertime ' +# Checking for +0000 in author time is enough since default +# timezone is UTC, but the timezone used while committing +# sets to +0530. +test_expect_success '--ignore-date works with am backend' ' + git commit --amend --date="$GIT_AUTHOR_DATE" && + git rebase --ignore-date HEAD^ && + git show HEAD --pretty="format:%ai" >authortime && + grep "+0000" authortime +' + +test_expect_success '--ignore-date works with interactive backend' ' + git commit --amend --date="$GIT_AUTHOR_DATE" && + git rebase --ignore-date -i HEAD^ && + git show HEAD --pretty="format:%ai" >authortime && + grep "+0000" authortime +' test_done
rebase am already has this flag to "lie" about the author date by changing it to the committer (current) date. Let's add the same for interactive machinery. Signed-off-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com> --- Documentation/git-rebase.txt | 6 +-- builtin/rebase.c | 21 +++++++--- sequencer.c | 55 ++++++++++++++++++++++++- sequencer.h | 1 + t/t3433-rebase-options-compatibility.sh | 16 +++++++ 5 files changed, 88 insertions(+), 11 deletions(-)