Re: [GSoC][PATCH v2 2/2] rebase -i: support --committer-date-is-author-date
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 19, 2019, 22:36 UTC
- Message-ID
- <xmqqwogdtzxc.fsf@gitster-ct.c.googlers.com>
- In-Reply-To
- <20190718190314.20549-3-rohit.ashiwal265@gmail.com>
Rohit Ashiwal <rohit.ashiwal265@gmail.com> writes:
Show 15 quoted lines
> @@ -1688,10 +1699,12 @@ 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)
> + 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, "--committer-date-is-author-date") ||
> - !strcmp(option, "--ignore-date") ||
> + if (!strcmp(option, "--ignore-date") ||
> !strcmp(option, "--whitespace=fix") ||
> !strcmp(option, "--whitespace=strip"))
> options.flags |= REBASE_FORCE;This is needed here, because am-opts no longer has the committer-date-is-author-date passed through with the parse_options() call in cmd_rebase(), which makes sense.
Show 15 quoted lines
> diff --git a/sequencer.c b/sequencer.c
> index a2d7b0925e..a65f01a422 100644
> --- a/sequencer.c
> +++ b/sequencer.c
> @@ -986,10 +999,17 @@ static int run_git_commit(struct repository *r,
>
> if (res <= 0)
> res = error_errno(_("could not read '%s'"), defmsg);
> - else
> + else {
> + if (opts->committer_date_is_author_date) {
> + char *date = read_author_date_or_die();
> + setenv("GIT_COMMITTER_DATE", date, 1);
> + free(date);
> + }Hmph, are we sure that author-script is always available at this point so that a call to read_author_date_or_die() is safe? There are three callers to the run_git_commit() function and I am not sure if codepaths that reach all of them prepared the input to the read_author_script() helper.
Show 22 quoted lines
> @@ -1019,6 +1039,11 @@ 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->committer_date_is_author_date) {
> + char *date = read_author_date_or_die();
> + argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s", date);
> + free(date);
> + }
> if (defmsg)
> argv_array_pushl(&cmd.args, "-F", defmsg, NULL);
> else if (!(flags & EDIT_MSG))
> @@ -1467,6 +1492,12 @@ static int try_to_commit(struct repository *r,
>
> reset_ident_date();
>
> + if (opts->committer_date_is_author_date) {
> + char *date = read_author_date_or_die();
> + setenv("GIT_COMMITTER_DATE", date, 1);
> + free(date);
> + }
> +In the same function, we seem to be grabbing the author ident by calling get_author(message), where the message is an in-core copy of a commit object, which suggests me that we may not necessarily be working with the on-disk information read_author_date_or_die() is prepared to deal with. Are we sure we have the needed information on disk so that read_author_date_or_die() will read the correct information from the disk?
Show 20 quoted lines
> @@ -2538,6 +2569,11 @@ static int read_populate_opts(struct replay_opts *opts)
> opts->signoff = 1;
> }
>
> + if (file_exists(rebase_path_cdate_is_adate())) {
> + opts->allow_ff = 0;
> + opts->committer_date_is_author_date = 1;
> + }
> +
> if (file_exists(rebase_path_reschedule_failed_exec()))
> opts->reschedule_failed_exec = 1;
>
> @@ -2620,6 +2656,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
> write_file(rebase_path_gpg_sign_opt(), "-S%s\n", opts->gpg_sign);
> if (opts->signoff)
> write_file(rebase_path_signoff(), "--signoff\n");
> + if (opts->committer_date_is_author_date)
> + write_file(rebase_path_cdate_is_adate(), "%s", "");
> if (opts->reschedule_failed_exec)
> write_file(rebase_path_reschedule_failed_exec(), "%s", "");These two are propagating the option across rebase invocations, and they should be correct (as correct as how correctly other options are propagated ;-)).
Show 9 quoted lines
> @@ -3437,6 +3475,12 @@ 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->committer_date_is_author_date) {
> + char *date = read_author_date_or_die();
> + argv_array_pushf(&cmd.env_array,
> + "GIT_COMMITTER_DATE=%s", date);
> + free(date);This codepath does have read_env_script() which reads from the author script an earlier call to write_author_script() would have left on disk (at least when do_merge() is called with 'commit' argument, anyway), so we should be able to read from it (or the existing code is already buggy---and this patch just makes it slightly worse by depending on a wrong assumption even more). OK.