Re: [PATCH v2 10/14] rebase: cleanup reset_head() calls

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

 



On 09/12/2021 19:26, Junio C Hamano wrote:
"Phillip Wood via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes:

From: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>

If ORIG_HEAD is not set by passing RESET_ORIG_HEAD then there is no
need to pass anything for reflog_orig_head. In addition to the callers
fixed in this commit move_to_original_branch() also passes
reflog_orig_head without setting ORIG_HEAD. That caller is mistakenly
passing the message it wants to put in the branch reflog which is not
currently possible so we delay fixing that caller until we can pass
the message as the branch reflog.

As I hinted elsewhere, these rules should be spelled out in a
comment before "int reset_head(...)" either in reset.[ch].

For this particular one, I wonder if

  (A) we can lose RESET_ORIG_HEAD bit and use the presence of
      reflog_orig_head to mean what that bit currently means, or

  (B) we keep the current code strucure, but make it a BUG() if
      a non-NULL reflog_orig_head is given without RESET_ORIG_HEAD
      bit set.

We do (B) later in the series, it cannot be done in this commit as we leave move_to_original_branch() unchanged here and that would BUG() out. I've added a comment it the commit message to explain this.

Best Wishes

Phillip


Signed-off-by: Phillip Wood <phillip.wood@xxxxxxxxxxxxx>
---
  builtin/rebase.c | 4 ++--
  1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/builtin/rebase.c b/builtin/rebase.c
index 3d78b5c8bef..fdd822c470f 100644
--- a/builtin/rebase.c
+++ b/builtin/rebase.c
@@ -675,7 +675,7 @@ static int run_am(struct rebase_options *opts)
reset_head(the_repository, &opts->orig_head,
  			   opts->head_name, 0,
-			   "HEAD", NULL, DEFAULT_REFLOG_ACTION);
+			   NULL, NULL, DEFAULT_REFLOG_ACTION);
  		error(_("\ngit encountered an error while preparing the "
  			"patches to replay\n"
  			"these revisions:\n"
@@ -1777,7 +1777,7 @@ int cmd_rebase(int argc, const char **argv, const char *prefix)
  			options.head_name ? options.head_name : "detached HEAD",
  			oid_to_hex(&options.onto->object.oid));
  		reset_head(the_repository, NULL, options.head_name,
-			   RESET_HEAD_REFS_ONLY, "HEAD", msg.buf, NULL);
+			   RESET_HEAD_REFS_ONLY, NULL, msg.buf, NULL);
  		strbuf_release(&msg);
  		ret = finish_rebase(&options);
  		goto cleanup;




[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