| Message ID | 20190806173638.17510-6-rohit.ashiwal265@gmail.com (mailing list archive) |
|---|---|
| State | New, archived |
| Headers | show |
| Series | rebase -i: support more options | expand |
Hi Rohit, On Tue, 6 Aug 2019, Rohit Ashiwal wrote: > @@ -1046,6 +1066,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) > + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); > if (defmsg) > argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > else if (!(flags & EDIT_MSG)) I need this patch to make the code _at least_ compile on Windows again (I don't know whether it breaks the test suite yet): -- snipsnap -- Subject: [PATCH] fixup! rebase -i: support --ignore-date It is a mistake to believe that the return value of `time()` is always an `unsigned long`. Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> --- sequencer.c | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sequencer.c b/sequencer.c index 539c0ef601b..a4c932d3407 100644 --- a/sequencer.c +++ b/sequencer.c @@ -1070,7 +1070,8 @@ static int run_git_commit(struct repository *r, if (opts->gpg_sign) argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); if (opts->ignore_date) - argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); + argv_array_pushf(&cmd.args, "--date=%"PRIuMAX, + (uintmax_t)time(NULL)); if (defmsg) argv_array_pushl(&cmd.args, "-F", defmsg, NULL); else if (!(flags & EDIT_MSG)) @@ -3642,7 +3643,8 @@ static int do_merge(struct repository *r, argv_array_push(&cmd.args, opts->gpg_sign); if (opts->ignore_date) argv_array_pushf(&cmd.args, - "GIT_AUTHOR_DATE=%ld", time(NULL)); + "GIT_AUTHOR_DATE=%"PRIuMAX, + (uintmax_t)time(NULL)); /* Add the tips to be merged */ for (j = to_merge; j; j = j->next) -- 2.22.0.windows.1.6.g271c090e89
Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > On Tue, 6 Aug 2019, Rohit Ashiwal wrote: > >> @@ -1046,6 +1066,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) >> + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); >> if (defmsg) >> argv_array_pushl(&cmd.args, "-F", defmsg, NULL); >> else if (!(flags & EDIT_MSG)) > > I need this patch to make the code _at least_ compile on Windows again > (I don't know whether it breaks the test suite yet): > > -- snipsnap -- > Subject: [PATCH] fixup! rebase -i: support --ignore-date > > It is a mistake to believe that the return value of `time()` is always > an `unsigned long`. Good catch. We can at least expect it to be some integral type ;-) With or without this fix-up, I think the patch is still not quite right. Output from time() formatted as an integer is a string of digits, and the side that reads that string and interprets it as a timestamp does so by calling parse_date(); it is up to that function to decide what datestring format it is in, and it does not necessarily take it as seconds since epoch. It is safer to force the "seconds since epoch" interpretation by writing the timestamp string like so: argv_array_pushf(&args, "--date=@%ld", time(NULL)); See 14ac2864 ("commit: accept more date formats for "--date"", 2014-05-01), which gives a good hint on how to do this right, and 2c733fb2 ("parse_date(): '@' prefix forces git-timestamp", 2012-02-02) for a backstory. > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > --- > sequencer.c | 6 ++++-- > 1 file changed, 4 insertions(+), 2 deletions(-) > > diff --git a/sequencer.c b/sequencer.c > index 539c0ef601b..a4c932d3407 100644 > --- a/sequencer.c > +++ b/sequencer.c > @@ -1070,7 +1070,8 @@ static int run_git_commit(struct repository *r, > if (opts->gpg_sign) > argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); > if (opts->ignore_date) > - argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); > + argv_array_pushf(&cmd.args, "--date=%"PRIuMAX, > + (uintmax_t)time(NULL)); > if (defmsg) > argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > else if (!(flags & EDIT_MSG)) > @@ -3642,7 +3643,8 @@ static int do_merge(struct repository *r, > argv_array_push(&cmd.args, opts->gpg_sign); > if (opts->ignore_date) > argv_array_pushf(&cmd.args, > - "GIT_AUTHOR_DATE=%ld", time(NULL)); > + "GIT_AUTHOR_DATE=%"PRIuMAX, > + (uintmax_t)time(NULL)); > > /* Add the tips to be merged */ > for (j = to_merge; j; j = j->next) > -- > 2.22.0.windows.1.6.g271c090e89
Hi Junio and Dscho On Thu, Aug 8, 2019 at 1:52 AM Junio C Hamano <gitster@pobox.com> wrote: > > Johannes Schindelin <Johannes.Schindelin@gmx.de> writes: > > > On Tue, 6 Aug 2019, Rohit Ashiwal wrote: > > > >> @@ -1046,6 +1066,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) > >> + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); > >> if (defmsg) > >> argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > >> else if (!(flags & EDIT_MSG)) > > > > I need this patch to make the code _at least_ compile on Windows again > > (I don't know whether it breaks the test suite yet): > > > > -- snipsnap -- > > Subject: [PATCH] fixup! rebase -i: support --ignore-date > > > > It is a mistake to believe that the return value of `time()` is always > > an `unsigned long`. > > Good catch. We can at least expect it to be some integral type ;-) > > With or without this fix-up, I think the patch is still not quite > right. Output from time() formatted as an integer is a string of > digits, and the side that reads that string and interprets it as a > timestamp does so by calling parse_date(); it is up to that function > to decide what datestring format it is in, and it does not > necessarily take it as seconds since epoch. It is safer to force > the "seconds since epoch" interpretation by writing the timestamp > string like so: > > argv_array_pushf(&args, "--date=@%ld", time(NULL)); > > See 14ac2864 ("commit: accept more date formats for "--date"", > 2014-05-01), which gives a good hint on how to do this right, and > 2c733fb2 ("parse_date(): '@' prefix forces git-timestamp", > 2012-02-02) for a backstory. > > > Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de> > > --- > > sequencer.c | 6 ++++-- > > 1 file changed, 4 insertions(+), 2 deletions(-) > > > > diff --git a/sequencer.c b/sequencer.c > > index 539c0ef601b..a4c932d3407 100644 > > --- a/sequencer.c > > +++ b/sequencer.c > > @@ -1070,7 +1070,8 @@ static int run_git_commit(struct repository *r, > > if (opts->gpg_sign) > > argv_array_pushf(&cmd.args, "-S%s", opts->gpg_sign); > > if (opts->ignore_date) > > - argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); > > + argv_array_pushf(&cmd.args, "--date=%"PRIuMAX, > > + (uintmax_t)time(NULL)); > > if (defmsg) > > argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > > else if (!(flags & EDIT_MSG)) > > @@ -3642,7 +3643,8 @@ static int do_merge(struct repository *r, > > argv_array_push(&cmd.args, opts->gpg_sign); > > if (opts->ignore_date) > > argv_array_pushf(&cmd.args, > > - "GIT_AUTHOR_DATE=%ld", time(NULL)); > > + "GIT_AUTHOR_DATE=%"PRIuMAX, > > + (uintmax_t)time(NULL)); > > > > /* Add the tips to be merged */ > > for (j = to_merge; j; j = j->next) > > -- > > 2.22.0.windows.1.6.g271c090e89 Thanks for suggestions, I'll incorporate these changes along with changes suggested by Junio and re-send the patch. - Rohit
On 06/08/2019 18:36, 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 | 17 +++++---- > sequencer.c | 48 ++++++++++++++++++++++--- > sequencer.h | 1 + > t/t3433-rebase-options-compatibility.sh | 16 +++++++++ > 5 files changed, 74 insertions(+), 14 deletions(-) > > diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt > index 83c5bbe06c..a5cdf8518b 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]). > + Lie about the author date by re-setting it to the value > + same as committer (current) date. This implies --force-rebase. I'm not keen on talking about lying with the implication of deceit. Just say that this resets the author date, there are perfectly legitimate reasons to do this. > + > 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 41fe9ebff8..7f464fc9ba 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; > @@ -116,6 +117,7 @@ static struct replay_opts get_replay_opts(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; > > @@ -535,7 +537,8 @@ 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.committer_date_is_author_date || > + opts.ignore_date) > opts.flags |= REBASE_FORCE; > > return !!run_rebase_interactive(&opts, command); > @@ -978,6 +981,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); > @@ -1428,8 +1433,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, > @@ -1694,13 +1699,13 @@ 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.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 d24a6fd585..c882dc5d5f 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") > @@ -928,6 +929,17 @@ static int setenv_committer_date_to_author_date(void) > return res; > } > > +static void ignore_author_date(const char **author) > +{ > + struct strbuf new_author = STRBUF_INIT; > + char *idx = memchr(*author, '>', strlen(*author)); strchr? I'd be happy to see this using the functions in ident.c which check the author is formatted correctly, as it stands this code will happily continue if idx == NULL. > + > + strbuf_add(&new_author, *author, idx - *author); > + strbuf_addstr(&new_author, "> "); > + datestamp(&new_author); > + *author = strbuf_detach(&new_author, NULL); > +} > + > 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" > @@ -985,7 +997,7 @@ static int run_git_commit(struct repository *r, > { > struct child_process cmd = CHILD_PROCESS_INIT; > > - if (opts->committer_date_is_author_date && > + if (opts->committer_date_is_author_date && !opts->ignore_date && > setenv_committer_date_to_author_date()) > return 1; We read the author script again just below, can set the committer date there by parsing the author string, that would mean you could use the same function that works on an author string in try_to_commit() (this comment should be on patch 3 I think) > if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) { > @@ -1013,10 +1025,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 -i, which must set the " I know it's only a bug message but it's not just 'rebase -i' but 'rebase -k', 'rebase -m' ... that use this code path, it would be better just to say 'rebase' > + "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); > @@ -1046,6 +1066,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) > + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); I think this is racy as it only sets the author date, the committer date may end up being different. > if (defmsg) > argv_array_pushl(&cmd.args, "-F", defmsg, NULL); > else if (!(flags & EDIT_MSG)) > @@ -1425,7 +1447,7 @@ static int try_to_commit(struct repository *r, > if (parse_head(r, ¤t_head)) > return -1; > if (!(flags & AMEND_MSG) && opts->committer_date_is_author_date && > - setenv_committer_date_to_author_date()) > + !opts->ignore_date && setenv_committer_date_to_author_date()) > return -1; > if (flags & AMEND_MSG) { > const char *exclude_gpgsig[] = { "gpgsig", NULL }; > @@ -1447,7 +1469,7 @@ static int try_to_commit(struct repository *r, > res = error(_("unable to parse commit author")); > goto out; > } > - if (opts->committer_date_is_author_date) { > + if (opts->committer_date_is_author_date && !opts->ignore_date) { If we only handled committer_date_is_author_date in a single place it wouldn't need to be changed twice in this patch > char *date; > int len = strlen(author); > char *idx = memchr(author, '>', len); > @@ -1507,6 +1529,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")); > @@ -2583,6 +2610,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; > > @@ -2667,6 +2699,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", ""); > > @@ -3484,6 +3518,9 @@ 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) > + argv_array_pushf(&cmd.args, > + "GIT_AUTHOR_DATE=%ld", time(NULL)); > > /* Add the tips to be merged */ > for (j = to_merge; j; j = j->next) > @@ -3756,7 +3793,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 e6cba468db..73d0515a3e 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 ceab48a831..95d99c4b7b 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. That sounds potentially fragile but I guess the timezone is unlikely to change in the future Best Wishes Phillip > +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 >
On 08/08/2019 12:42, Phillip Wood wrote: > On 06/08/2019 18:36, 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> >> --- >> [...[ >> 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" >> @@ -985,7 +997,7 @@ static int run_git_commit(struct repository *r, >> { >> struct child_process cmd = CHILD_PROCESS_INIT; >> - if (opts->committer_date_is_author_date && >> + if (opts->committer_date_is_author_date && !opts->ignore_date && >> setenv_committer_date_to_author_date()) >> return 1; > > We read the author script again just below, can set the committer date > there by parsing the author string, that would mean you could use the > same function that works on an author string in try_to_commit() (this > comment should be on patch 3 I think) It's a bit more complicated as you've done this to avoid the duplication in the previous version. I think it should be possible to do it by reading the author identity upfront (protected by 'if is_rebase_i(opts)') and processing it appropriately in the two code paths. (You might need to refactor some of the functions like read_env_script() to do this.) An alternative would be to refactor try_to_commit() so that it can create the root commit and delete the code here that does that. Best Wishes Phillip > >> if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) { >> @@ -1013,10 +1025,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 -i, which must set the " > > I know it's only a bug message but it's not just 'rebase -i' but 'rebase > -k', 'rebase -m' ... that use this code path, it would be better just to > say 'rebase' > >> + "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); >> @@ -1046,6 +1066,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) >> + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); > > I think this is racy as it only sets the author date, the committer date > may end up being different. > >> if (defmsg) >> argv_array_pushl(&cmd.args, "-F", defmsg, NULL); >> else if (!(flags & EDIT_MSG)) >> @@ -1425,7 +1447,7 @@ static int try_to_commit(struct repository *r, >> if (parse_head(r, ¤t_head)) >> return -1; >> if (!(flags & AMEND_MSG) && opts->committer_date_is_author_date && >> - setenv_committer_date_to_author_date()) >> + !opts->ignore_date && setenv_committer_date_to_author_date()) >> return -1; >> if (flags & AMEND_MSG) { >> const char *exclude_gpgsig[] = { "gpgsig", NULL }; >> @@ -1447,7 +1469,7 @@ static int try_to_commit(struct repository *r, >> res = error(_("unable to parse commit author")); >> goto out; >> } >> - if (opts->committer_date_is_author_date) { >> + if (opts->committer_date_is_author_date && !opts->ignore_date) { > > If we only handled committer_date_is_author_date in a single place it > wouldn't need to be changed twice in this patch > >> char *date; >> int len = strlen(author); >> char *idx = memchr(author, '>', len); >> @@ -1507,6 +1529,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")); >> @@ -2583,6 +2610,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; >> @@ -2667,6 +2699,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", ""); >> @@ -3484,6 +3518,9 @@ 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) >> + argv_array_pushf(&cmd.args, >> + "GIT_AUTHOR_DATE=%ld", time(NULL)); >> /* Add the tips to be merged */ >> for (j = to_merge; j; j = j->next) >> @@ -3756,7 +3793,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 e6cba468db..73d0515a3e 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 ceab48a831..95d99c4b7b 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. > > That sounds potentially fragile but I guess the timezone is unlikely to > change in the future > > Best Wishes > > Phillip > >> +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 >>
diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt index 83c5bbe06c..a5cdf8518b 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]). + Lie about the author date by re-setting 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 41fe9ebff8..7f464fc9ba 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; @@ -116,6 +117,7 @@ static struct replay_opts get_replay_opts(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; @@ -535,7 +537,8 @@ 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.committer_date_is_author_date || + opts.ignore_date) opts.flags |= REBASE_FORCE; return !!run_rebase_interactive(&opts, command); @@ -978,6 +981,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); @@ -1428,8 +1433,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, @@ -1694,13 +1699,13 @@ 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.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 d24a6fd585..c882dc5d5f 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") @@ -928,6 +929,17 @@ static int setenv_committer_date_to_author_date(void) return res; } +static void ignore_author_date(const char **author) +{ + struct strbuf new_author = STRBUF_INIT; + char *idx = memchr(*author, '>', strlen(*author)); + + strbuf_add(&new_author, *author, idx - *author); + strbuf_addstr(&new_author, "> "); + datestamp(&new_author); + *author = strbuf_detach(&new_author, NULL); +} + 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" @@ -985,7 +997,7 @@ static int run_git_commit(struct repository *r, { struct child_process cmd = CHILD_PROCESS_INIT; - if (opts->committer_date_is_author_date && + if (opts->committer_date_is_author_date && !opts->ignore_date && setenv_committer_date_to_author_date()) return 1; if ((flags & CREATE_ROOT_COMMIT) && !(flags & AMEND_MSG)) { @@ -1013,10 +1025,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 -i, 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); @@ -1046,6 +1066,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) + argv_array_pushf(&cmd.args, "--date=%ld", time(NULL)); if (defmsg) argv_array_pushl(&cmd.args, "-F", defmsg, NULL); else if (!(flags & EDIT_MSG)) @@ -1425,7 +1447,7 @@ static int try_to_commit(struct repository *r, if (parse_head(r, ¤t_head)) return -1; if (!(flags & AMEND_MSG) && opts->committer_date_is_author_date && - setenv_committer_date_to_author_date()) + !opts->ignore_date && setenv_committer_date_to_author_date()) return -1; if (flags & AMEND_MSG) { const char *exclude_gpgsig[] = { "gpgsig", NULL }; @@ -1447,7 +1469,7 @@ static int try_to_commit(struct repository *r, res = error(_("unable to parse commit author")); goto out; } - if (opts->committer_date_is_author_date) { + if (opts->committer_date_is_author_date && !opts->ignore_date) { char *date; int len = strlen(author); char *idx = memchr(author, '>', len); @@ -1507,6 +1529,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")); @@ -2583,6 +2610,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; @@ -2667,6 +2699,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", ""); @@ -3484,6 +3518,9 @@ 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) + argv_array_pushf(&cmd.args, + "GIT_AUTHOR_DATE=%ld", time(NULL)); /* Add the tips to be merged */ for (j = to_merge; j; j = j->next) @@ -3756,7 +3793,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 e6cba468db..73d0515a3e 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 ceab48a831..95d99c4b7b 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 | 17 +++++---- sequencer.c | 48 ++++++++++++++++++++++--- sequencer.h | 1 + t/t3433-rebase-options-compatibility.sh | 16 +++++++++ 5 files changed, 74 insertions(+), 14 deletions(-)