Re: [PATCH v5 4/5] rebase -i: support --ignore-date

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Hi Danh

On 26/06/2020 15:09, Đoàn Trần Công Danh wrote:
On 2020-06-26 10:55:27+0100, Phillip Wood <phillip.wood123@xxxxxxxxx> wrote:
From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>

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@xxxxxxxxx>
Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
  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(-)

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 a7c3d5c92b..890dd4c588 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,
+			 "ignore author date and use current date"),

Nit: The options' description is subjected to l10n.

s/".*"/N_(&)/

Well spotted, thanks

  		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 if (ignore_whitespace) {
  			string_list_append (&strategy_options,
  					    "ignore-space-change");
diff --git a/sequencer.c b/sequencer.c
index 29f6d1bc39..f8e1e38623 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 = strlen(author);
+	struct ident_split ident;
+	struct strbuf new_author = STRBUF_INIT;
+
+	if (split_ident_line(&ident, author, len) < 0) {
+		error(_("malformed ident line '%s'"), author);
+		return NULL;
+	}
+
+	len = ident.mail_end - ident.name_begin + 1;
+	strbuf_addf(&new_author, "%.*s ", len, ident.name_begin);

I wonder if we can do this instead:

	strbuf_add(&new_author, ident.name_begin, len);

There is a space at the end of the format string, which I think is needed to separate the name and email from the date we add with datestamp() below

Best Wishes

Phillip

Quick skim through the code and nothing crashes when make test with:

	CFLAGS="-fstack-clash-protection -D_FORTIFY_SOURCE=2"

I think it's OK to be changed.

+	datestamp(&new_author);
+	return strbuf_detach(&new_author, NULL);
+}
+




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]

  Powered by Linux