git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH v2 00/18] builtin rebase options

From
Junio C Hamano <gitster@pobox.com>
Date
Sep 6, 2018, 19:50 UTC
Message-ID
<xmqqmusuz9ql.fsf@gitster-ct.c.googlers.com>
In-Reply-To
<pull.33.v2.git.gitgitgadget@gmail.com>

"Johannes Schindelin via GitGitGadget" <gitgitgadget@gmail.com> writes:

> This patch series completes the support for all rebase options in the
> builtin rebase, e.g. --signoff, rerere-autoupdate, etc.
>
> It is based on pk/rebase -in-c-3-acts.

... which in turn was based on pk/rebase-in-c-2-basic that just got rerolled, so I would assume that you want pk/rebase-in-c-3-acts I have rebased on top of the result of applying the updated 2-basic series.

I've rebuilt the collection of topics up to pk/rebase-in-c-6-final with these two updated series twice, once doing it manually, like I did the last time, and another using "rebase -i -r" on top of the updated pk/rebase-in-c-4-opts. The resulting trees match, of course.

I did it twice to try out how it feels to use "rebase -i -r" because I wanted to make sure what we are shipping in 'master' behaves sensibly ;-)

Two things I noticed about the recreation of the merge ...
	Reminder to bystanders.  We need to merge ag/rebase-i-in-c
	topic on top of pk/reabse-in-c-5-test topic before applying
	a patch to adjust rebase to call rebase-i using the latter's
	new calling convention.  The topics look like
	- pk/rebase-in-c has three patches on master
	- pk/rebase-in-c-2-basic builds on it, and being replaced
	- pk/rebase-in-c-3-acts builds on 2-basic (no update this time)
	- pk/rebase-in-c-4-opts builds on 3-acts, and being replaced
	- pk/rebase-in-c-5-test builds on 4-opts (no update this time)
	- js/rebase-in-c-5.5 builds on 5-test and merges ag/rebase-in-c
	  topic before applying one patch on it (no update this time)
	- pk/rebase-in-c-6-final builds on 5.5 (no update this time)
	and we are replacing 2-basic with 11 patches and 4-opts with
	18 patches.
... using "rebase -i -r" are that 
 (1) it rebuilt, or at least offered to rebuild, the entire side
     branch, even though there is absolutely no need to.  Leaving
     "pick"s untouched, based on the correct fork point, resulted in
     all picks fast forwarded, but it was somewhat alarming.
 (2) "merge -C <original merge commit> ag/rebase-i-in-c" appeared as
     the insn to merge in the (possibly rebuilt) side branch.  And
     just like "commit -C", it took the merge message from the
     original merge commit, which means that the summary of the
     merged side branch is kept stale.  In this particular case, I
     did not even want to see ag/rebase-i-in-c topic touched, so I
     knew I want to keep the original merge summary, but if the user
     took the offer to rewrite the side branch (e.g. with a "reword"
     to retitle), using the original merge message would probably
     disappoint the user.

I think (1) actually is a feature. Not everybody is an integrator who does not want to touch any commit on the topic branch(es) while rebuilding a single-strand-of-pearls that has many commits and an occasional merge of the tip of another topic branch. It's just that the feature does not suit the workflow I use when I am playing the top-level integrator role.

I am not sure what should be the ideal behaviour for (2). I would imagine that

 - I do want to keep the original title the merge (e.g. "into
   <target branch>", if left to "git merge" to come up with the
   title during "rebase -i" session, would be lost and become "into
   HEAD", which is not what we want);
 - I do want to keep the original commentary in the merge (e.g. what
   you would see in "git log --first-parent master..next" that gives
   summary of each topic getting merged) so that I can update it as
   needed; but 
 - I do want the topic summary fmt-merge-msg produces to be based on
   the updated side branch.

I am not sure if the last item can reliably be filtered out of the original and replaced with newly generated summary. If we can do so, that would be ideal, I guess.

Another observation was that after rebuiding pk/rebase-in-c-6^0 on top of the updated pk'/rebase-in-c-4 using "rebase -i -r", I of course still needed to "branch -f" to update pk/rebase-in-c-5, js/reabse-in-c-5.5, and pk/rebase-in-c-6 branches to point at appropriate commits. I do not think it is a good idea to let "rebase -i" munge these dependent branches by default, but it might be worth considering it as an option. Since I want to be more in control of what happens to the tips of topic branches, I did not mind at all having to run "branch -f" and having the chance to run "diff" before doing so, but at the same time, that means doing these manually in steps building 5 on 4, 5.5 on 5 and then 6 on 5.5, instead of building 6 on top of 4 using "rebase -i" and then tagging the intermediate states, gives me more control without forcing me more work.

I guess that is the answer to a question you asked earlier, which I haven't answered so far because I didn't have a good grasp of where my preference was coming from when it was asked. Now I know, so...

Previous: Pratik Karki via GitGitGadgetNext: Junio C Hamano
Message 42 of 44 in “builtin rebase options”
  1. Pratik KarkiAug 8, 2018
  2. 01/18 builtin rebase: allow selecting the rebase "backend"Pratik Karki, Aug 8, 2018
  3. 02/18 builtin rebase: support --signoffPratik Karki, Aug 8, 2018
  4. 03/18 builtin rebase: support --rerere-autoupdatePratik Karki, Aug 8, 2018
  5. 04/18 builtin rebase: support --committer-date-is-author-datePratik Karki, Aug 8, 2018
  6. 05/18 builtin rebase: support `ignore-whitespace` optionPratik Karki, Aug 8, 2018
  7. 06/18 builtin rebase: support `ignore-date` optionPratik Karki, Aug 8, 2018
  8. 07/18 builtin rebase: support `keep-empty` optionPratik Karki, Aug 8, 2018
  9. Johannes SchindelinAug 24, 2018
  10. 08/18 builtin rebase: support `--autosquash`Pratik Karki, Aug 8, 2018
  11. 09/18 builtin rebase: support `--gpg-sign` optionPratik Karki, Aug 8, 2018
  12. 10/18 builtin rebase: support `-C` and `--whitespace=<type>`Pratik Karki, Aug 8, 2018
  13. 11/18 builtin rebase: support `--autostash` optionPratik Karki, Aug 8, 2018
  14. Duy NguyenAug 18, 2018
  15. Johannes SchindelinAug 24, 2018
  16. 12/18 builtin rebase: support `--exec`Pratik Karki, Aug 8, 2018
  17. 13/18 builtin rebase: support `--allow-empty-message` optionPratik Karki, Aug 8, 2018
  18. 14/18 builtin rebase: support --rebase-merges[=[no-]rebase-cousins]Pratik Karki, Aug 8, 2018
  19. 15/18 merge-base --fork-point: extract libified functionPratik Karki, Aug 8, 2018
  20. 16/18 builtin rebase: support `fork-point` optionPratik Karki, Aug 8, 2018
  21. 17/18 builtin rebase: add support for custom merge strategiesPratik Karki, Aug 8, 2018
  22. 18/18 builtin rebase: support --rootPratik Karki, Aug 8, 2018
  23. 00/18 builtin rebase optionsJohannes Schindelin via GitGitGadget, Sep 4, 2018
  24. 01/18 builtin rebase: allow selecting the rebase "backend"Pratik Karki via GitGitGadget, Sep 4, 2018
  25. 02/18 builtin rebase: support --signoffPratik Karki via GitGitGadget, Sep 4, 2018
  26. 03/18 builtin rebase: support --rerere-autoupdatePratik Karki via GitGitGadget, Sep 4, 2018
  27. 04/18 builtin rebase: support --committer-date-is-author-datePratik Karki via GitGitGadget, Sep 4, 2018
  28. 05/18 builtin rebase: support `ignore-whitespace` optionPratik Karki via GitGitGadget, Sep 4, 2018
  29. 06/18 builtin rebase: support `ignore-date` optionPratik Karki via GitGitGadget, Sep 4, 2018
  30. 07/18 builtin rebase: support `keep-empty` optionPratik Karki via GitGitGadget, Sep 4, 2018
  31. 08/18 builtin rebase: support `--autosquash`Pratik Karki via GitGitGadget, Sep 4, 2018
  32. 09/18 builtin rebase: support `--gpg-sign` optionPratik Karki via GitGitGadget, Sep 4, 2018
  33. 11/18 builtin rebase: support `--autostash` optionPratik Karki via GitGitGadget, Sep 4, 2018
  34. 10/18 builtin rebase: support `-C` and `--whitespace=<type>`Pratik Karki via GitGitGadget, Sep 4, 2018
  35. 12/18 builtin rebase: support `--exec`Pratik Karki via GitGitGadget, Sep 4, 2018
  36. 13/18 builtin rebase: support `--allow-empty-message` optionPratik Karki via GitGitGadget, Sep 4, 2018
  37. 14/18 builtin rebase: support --rebase-merges[=[no-]rebase-cousins]Pratik Karki via GitGitGadget, Sep 4, 2018
  38. 15/18 merge-base --fork-point: extract libified functionPratik Karki via GitGitGadget, Sep 4, 2018
  39. 16/18 builtin rebase: support `fork-point` optionPratik Karki via GitGitGadget, Sep 4, 2018
  40. 17/18 builtin rebase: add support for custom merge strategiesPratik Karki via GitGitGadget, Sep 4, 2018
  41. 18/18 builtin rebase: support --rootPratik Karki via GitGitGadget, Sep 4, 2018
  42. Junio C HamanoSep 6, 2018
  43. Junio C HamanoSep 6, 2018
  44. Johannes SchindelinOct 12, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.