diff mbox series

[v7,4/5] rebase -i: support --ignore-date

Message ID 20200716173221.103295-5-phillip.wood123@gmail.com (mailing list archive)
State New, archived
Headers show
Series cleanup ra/rebase-i-more-options | expand

Commit Message

Phillip Wood July 16, 2020, 5:32 p.m. UTC
From: Phillip Wood <phillip.wood@dunelm.org.uk>

Rebase is implemented with two different backends - 'apply' and
'merge' each of which support a different set of options. In
particular the apply backend supports a number of options implemented
by 'git am' that are not implemented in the merge backend. This means
that the available options are different depending on which backend is
used which is confusing. This patch adds support for the --ignore-date
option to the merge backend. This option uses the current time as the
author date rather than reusing the original author date when
rewriting commits. We take care to handle the combination of
--ignore-date and --committer-date-is-author-date in the same way as
the apply backend.

Original-patch-by: Rohit Ashiwal <rohit.ashiwal265@gmail.com>
Signed-off-by: Phillip Wood <phillip.wood@dunelm.org.uk>
---
 Documentation/git-rebase.txt   |  7 +++--
 builtin/rebase.c               | 13 +++++---
 sequencer.c                    | 50 ++++++++++++++++++++++++++++--
 sequencer.h                    |  1 +
 t/t3436-rebase-more-options.sh | 56 ++++++++++++++++++++++++++++++++++
 5 files changed, 117 insertions(+), 10 deletions(-)

Comments

Johannes Schindelin Aug. 13, 2020, 2:07 p.m. UTC | #1
Hi Phillip,

On Thu, 16 Jul 2020, Phillip Wood wrote:

> @@ -957,7 +976,11 @@ static int run_git_commit(struct repository *r,
>
>  	if (opts->committer_date_is_author_date)
>  		argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
> +				 opts->ignore_date ?
> +				 "" :
>  				 author_date_from_env_array(&cmd.env_array));
> +	if (opts->ignore_date)
> +		argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");

Technically, if we switched those two `if` blocks, we would not have to
edit the committer date one. But this way is much clearer.

>
>  	argv_array_push(&cmd.args, "commit");
>
> @@ -1388,7 +1411,8 @@ static int try_to_commit(struct repository *r,
>  			    ident.date_begin,
>  			    (int)(ident.tz_end - ident.tz_begin),
>  			    ident.tz_begin);
> -		res = setenv("GIT_COMMITTER_DATE", date.buf, 1);
> +		res = setenv("GIT_COMMITTER_DATE",
> +			     opts->ignore_date ? "" : date.buf, 1);

Isn't this constructing the `date` string for nothing, if
`opts->ignore_date` is set?

I would much rather see it done this way:

-	if (opts->committer_date_is_author_date) {
+	if (opts->committer_date_is_author_date && opts->ignore_date)
+		setenv("GIT_COMMITTER_DATE", "", 1);
+	else if (opts->committer_date_is_author_date) {

Not enough of a reason to re-roll, though.

>  		strbuf_release(&date);
>
>  		if (res)
> @@ -1454,6 +1478,16 @@ static int try_to_commit(struct repository *r,
>
>  	reset_ident_date();
>
> +	if (opts->ignore_date) {
> +		author = ignore_author_date(author);
> +		if (!author) {
> +			res = -1;
> +			goto out;
> +		}
> +		free(author_to_free);

A better cadence might be to first `free(author_to_free)`, then assign
`author = author_to_free = ignore_author_date(author);` (at least in my
perspective, it reads more naturally).

But again, not a big reason for a re-roll.

The rest of the patch looks good to me.

Thank you,
Dscho

> +		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 +2617,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;
>
> @@ -2675,6 +2714,8 @@ int write_basic_state(struct replay_opts *opts, const char *head_name,
>  		write_file(rebase_path_keep_redundant_commits(), "%s", "");
>  	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", "");
>
> @@ -3597,7 +3638,11 @@ static int do_merge(struct repository *r,
>
>  		if (opts->committer_date_is_author_date)
>  			argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
> +					 opts->ignore_date ?
> +					 "" :
>  					 author_date_from_env_array(&cmd.env_array));
> +		if (opts->ignore_date)
> +			argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");
>
>  		cmd.git_cmd = 1;
>  		argv_array_push(&cmd.args, "merge");
> @@ -3877,7 +3922,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 4ab94119ae..3587878e3b 100644
> --- a/sequencer.h
> +++ b/sequencer.h
> @@ -46,6 +46,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/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
> index 50a63d8ebe..0ede2b8900 100755
> --- a/t/t3436-rebase-more-options.sh
> +++ b/t/t3436-rebase-more-options.sh
> @@ -108,6 +108,62 @@ test_expect_success '--committer-date-is-author-date works when committing confl
>  	test_ctime_is_atime -1
>  '
>
> +# Checking for +0000 in the author date is sufficient since the
> +# default timezone is UTC but the timezone used while committing is
> +# +0530. The inverted logic in the grep is necessary to check all the
> +# author dates in the file.
> +test_ctime_is_ignored () {
> +	git log $1 --format=%ai >authortime &&
> +	! grep -v +0000 authortime
> +}
> +
> +test_expect_success '--ignore-date works with apply backend' '
> +	git commit --amend --date="$GIT_AUTHOR_DATE" &&
> +	git rebase --apply --ignore-date HEAD^ &&
> +	test_ctime_is_ignored -1
> +'
> +
> +test_expect_success '--ignore-date works with merge backend' '
> +	git commit --amend --date="$GIT_AUTHOR_DATE" &&
> +	git rebase --ignore-date -m HEAD^ &&
> +	test_ctime_is_ignored -1
> +'
> +
> +test_expect_success '--ignore-date works after conflict resolution' '
> +	test_must_fail git rebase --ignore-date -m \
> +		--onto commit2^^ commit2^ commit2 &&
> +	echo resolved >foo &&
> +	git add foo &&
> +	git rebase --continue &&
> +	test_ctime_is_ignored -1
> +'
> +
> +test_expect_success '--ignore-date works with rebase -r' '
> +	git checkout side &&
> +	git merge --no-ff commit3 &&
> +	git rebase -r --root --ignore-date &&
> +	test_ctime_is_ignored
> +'
> +
> +test_expect_success '--ignore-date with --committer-date-is-author-date works' '
> +	test_must_fail git rebase -m --committer-date-is-author-date \
> +		--ignore-date --onto commit2^^ commit2^ commit3 &&
> +	git checkout --theirs foo &&
> +	git add foo &&
> +	git rebase --continue &&
> +	test_ctime_is_atime -2 &&
> +	test_ctime_is_ignored -2
> +'
> +
> +test_expect_success '--ignore-date --committer-date-is-author-date works when forking merge' '
> +	GIT_SEQUENCE_EDITOR="echo \"merge -C $(git rev-parse HEAD) commit3\">" \
> +		PATH="./test-bin:$PATH" git rebase -i --strategy=test \
> +				--ignore-date --committer-date-is-author-date \
> +				side side &&
> +	test_ctime_is_atime -1 &&
> +	test_ctime_is_ignored -1
> + '
> +
>  # This must be the last test in this file
>  test_expect_success '$EDITOR and friends are unchanged' '
>  	test_editor_unchanged
> --
> 2.27.0
>
>
diff mbox series

Patch

diff --git a/Documentation/git-rebase.txt b/Documentation/git-rebase.txt
index dfa70263e6..e2717e20e6 100644
--- a/Documentation/git-rebase.txt
+++ b/Documentation/git-rebase.txt
@@ -450,8 +450,9 @@  See also INCOMPATIBLE OPTIONS below.
 	date. This option implies --force-rebase.
 
 --ignore-date::
-	This flag is passed to 'git am' to change the author date
-	of each rebased commit (see linkgit:git-am[1]).
+	Instead of using the author date of the original commit, use
+	the current time as the	author date of the rebased commit.  This
+	option implies `--force-rebase`.
 +
 See also INCOMPATIBLE OPTIONS below.
 
@@ -589,7 +590,6 @@  INCOMPATIBLE OPTIONS
 The following options:
 
  * --apply
- * --ignore-date
  * --whitespace
  * -C
 
@@ -617,6 +617,7 @@  In addition, the following pairs of options are incompatible:
  * --preserve-merges and --empty=
  * --preserve-merges and --ignore-whitespace
  * --preserve-merges and --committer-date-is-author-date
+ * --preserve-merges and --ignore-date
  * --keep-base and --onto
  * --keep-base and --root
 
diff --git a/builtin/rebase.c b/builtin/rebase.c
index 2579380729..583ac96fc7 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -89,6 +89,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;
@@ -127,6 +128,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;
 
@@ -1503,8 +1505,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,
+			 N_("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", &ignore_whitespace,
@@ -1797,13 +1799,12 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 	    options.autosquash) {
 		allow_preemptive_ff = 0;
 	}
-	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"))
 			allow_preemptive_ff = 0;
 		else if (skip_prefix(option, "-C", &p)) {
@@ -1862,6 +1863,8 @@  int cmd_rebase(int argc, const char **argv, const char *prefix)
 		if (options.committer_date_is_author_date)
 			argv_array_push(&options.git_am_opts,
 					"--committer-date-is-author-date");
+		if (options.ignore_date)
+			argv_array_push(&options.git_am_opts, "--ignore-date");
 	} else {
 		/* REBASE_MERGE and PRESERVE_MERGES */
 		if (ignore_whitespace) {
diff --git a/sequencer.c b/sequencer.c
index 29f6d1bc39..878fafdc41 100644
--- a/sequencer.c
+++ b/sequencer.c
@@ -150,6 +150,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")
@@ -889,6 +890,24 @@  static const char *author_date_from_env_array(const struct argv_array *env)
 	BUG("GIT_AUTHOR_DATE missing from author script");
 }
 
+/* Construct a free()able author string with current time as the author date */
+static char *ignore_author_date(const char *author)
+{
+	int len;
+	struct ident_split ident;
+	struct strbuf new_author = STRBUF_INIT;
+
+	if (split_ident_line(&ident, author, strlen(author)) < 0) {
+		error(_("invalid author identity: %s"), author);
+		return NULL;
+	}
+
+	len = ident.mail_end - ident.name_begin + 1;
+	strbuf_addf(&new_author, "%.*s ", len, ident.name_begin);
+	datestamp(&new_author);
+	return 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"
@@ -957,7 +976,11 @@  static int run_git_commit(struct repository *r,
 
 	if (opts->committer_date_is_author_date)
 		argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
+				 opts->ignore_date ?
+				 "" :
 				 author_date_from_env_array(&cmd.env_array));
+	if (opts->ignore_date)
+		argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");
 
 	argv_array_push(&cmd.args, "commit");
 
@@ -1388,7 +1411,8 @@  static int try_to_commit(struct repository *r,
 			    ident.date_begin,
 			    (int)(ident.tz_end - ident.tz_begin),
 			    ident.tz_begin);
-		res = setenv("GIT_COMMITTER_DATE", date.buf, 1);
+		res = setenv("GIT_COMMITTER_DATE",
+			     opts->ignore_date ? "" : date.buf, 1);
 		strbuf_release(&date);
 
 		if (res)
@@ -1454,6 +1478,16 @@  static int try_to_commit(struct repository *r,
 
 	reset_ident_date();
 
+	if (opts->ignore_date) {
+		author = ignore_author_date(author);
+		if (!author) {
+			res = -1;
+			goto out;
+		}
+		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 +2617,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;
 
@@ -2675,6 +2714,8 @@  int write_basic_state(struct replay_opts *opts, const char *head_name,
 		write_file(rebase_path_keep_redundant_commits(), "%s", "");
 	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", "");
 
@@ -3597,7 +3638,11 @@  static int do_merge(struct repository *r,
 
 		if (opts->committer_date_is_author_date)
 			argv_array_pushf(&cmd.env_array, "GIT_COMMITTER_DATE=%s",
+					 opts->ignore_date ?
+					 "" :
 					 author_date_from_env_array(&cmd.env_array));
+		if (opts->ignore_date)
+			argv_array_push(&cmd.env_array, "GIT_AUTHOR_DATE=");
 
 		cmd.git_cmd = 1;
 		argv_array_push(&cmd.args, "merge");
@@ -3877,7 +3922,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 4ab94119ae..3587878e3b 100644
--- a/sequencer.h
+++ b/sequencer.h
@@ -46,6 +46,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/t3436-rebase-more-options.sh b/t/t3436-rebase-more-options.sh
index 50a63d8ebe..0ede2b8900 100755
--- a/t/t3436-rebase-more-options.sh
+++ b/t/t3436-rebase-more-options.sh
@@ -108,6 +108,62 @@  test_expect_success '--committer-date-is-author-date works when committing confl
 	test_ctime_is_atime -1
 '
 
+# Checking for +0000 in the author date is sufficient since the
+# default timezone is UTC but the timezone used while committing is
+# +0530. The inverted logic in the grep is necessary to check all the
+# author dates in the file.
+test_ctime_is_ignored () {
+	git log $1 --format=%ai >authortime &&
+	! grep -v +0000 authortime
+}
+
+test_expect_success '--ignore-date works with apply backend' '
+	git commit --amend --date="$GIT_AUTHOR_DATE" &&
+	git rebase --apply --ignore-date HEAD^ &&
+	test_ctime_is_ignored -1
+'
+
+test_expect_success '--ignore-date works with merge backend' '
+	git commit --amend --date="$GIT_AUTHOR_DATE" &&
+	git rebase --ignore-date -m HEAD^ &&
+	test_ctime_is_ignored -1
+'
+
+test_expect_success '--ignore-date works after conflict resolution' '
+	test_must_fail git rebase --ignore-date -m \
+		--onto commit2^^ commit2^ commit2 &&
+	echo resolved >foo &&
+	git add foo &&
+	git rebase --continue &&
+	test_ctime_is_ignored -1
+'
+
+test_expect_success '--ignore-date works with rebase -r' '
+	git checkout side &&
+	git merge --no-ff commit3 &&
+	git rebase -r --root --ignore-date &&
+	test_ctime_is_ignored
+'
+
+test_expect_success '--ignore-date with --committer-date-is-author-date works' '
+	test_must_fail git rebase -m --committer-date-is-author-date \
+		--ignore-date --onto commit2^^ commit2^ commit3 &&
+	git checkout --theirs foo &&
+	git add foo &&
+	git rebase --continue &&
+	test_ctime_is_atime -2 &&
+	test_ctime_is_ignored -2
+'
+
+test_expect_success '--ignore-date --committer-date-is-author-date works when forking merge' '
+	GIT_SEQUENCE_EDITOR="echo \"merge -C $(git rev-parse HEAD) commit3\">" \
+		PATH="./test-bin:$PATH" git rebase -i --strategy=test \
+				--ignore-date --committer-date-is-author-date \
+				side side &&
+	test_ctime_is_atime -1 &&
+	test_ctime_is_ignored -1
+ '
+
 # This must be the last test in this file
 test_expect_success '$EDITOR and friends are unchanged' '
 	test_editor_unchanged