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

[PATCH v4 7/8] rebase: define linearization ordering and enforce it

From
Elijah Newren <newren@gmail.com>
Date
Dec 11, 2018, 16:11 UTC
Message-ID
<20181211161139.31686-8-newren@gmail.com>
In-Reply-To
<20181211161139.31686-1-newren@gmail.com>

Ever since commit 3f213981e44a ("add tests for rebasing merged history", 2013-06-06), t3425 has had tests which included the rebasing of merged history and whose order of applied commits was checked. Unfortunately, the tests expected different behavior depending on which backend was in use. Implementing these checks was the following four lines (including the TODO message) which were repeated verbatim three times in t3425:

    #TODO: make order consistent across all flavors of rebase
    test_run_rebase success 'e n o' ''
    test_run_rebase success 'e n o' -m
    test_run_rebase success 'n o e' -i

As part of the effort to reduce differences between the rebase backends so that users get more uniform behavior, let's define the correct behavior and modify the different backends so they all get the right answer. It turns out that the difference in behavior here is entirely due to topological sorting; since some backends require topological sorting (particularly when --rebase-merges is specified), require it for all modes. Modify the am and merge backends to implement this.

Performance Considerations:
I was unable to measure any appreciable performance difference with this
change.  Trying to control the run-to-run variation was difficult; I
eventually found a headless beefy box that I could ssh into, which
seemed to help.  Using git.git, I ran the following testcase:
    $ git reset --hard v2.20.0-rc1~2
    $ time git rebase --quiet v2.20.0-rc0~16

I first ran once to warm any disk caches, then ran five subsequent runs and recorded the times of those five. I observed the following results for the average time:

     Before this change:
       "real" timing: 1.340s (standard deviation: 0.040s)
       "user" timing: 1.050s (standard deviation: 0.041s)
       "sys"  timing: 0.270s (standard deviation: 0.011s)
     After  this change:
       "real" timing: 1.327s (standard deviation: 0.065s)
       "user" timing: 1.031s (standard deviation: 0.061s)
       "sys"  timing: 0.280s (standard deviation: 0.014s)

Measurements aside, I would expect the timing for walking revisions to be dwarfed by the work involved in creating and applying patches, so this isn't too surprising. Further, while somewhat counter-intuitive, it is possible that turning on topological sorting is actually a performance improvement: by way of comparison, turning on --topo-order made fast-export faster (see https://public-inbox.org/git/20090211135640.GA19600@coredump.intra.peff.net/).

Signed-off-by: Elijah Newren <newren@gmail.com>
---
 git-rebase--am.sh                 |  2 +-
 git-rebase--merge.sh              |  2 +-
 t/t3425-rebase-topology-merges.sh | 15 ++++++---------
 3 files changed, 8 insertions(+), 11 deletions(-)
diff --git a/git-rebase--am.sh b/git-rebase--am.sh
index 99b8c17787..6416716ee6 100644
--- a/git-rebase--am.sh
+++ b/git-rebase--am.sh
@@ -36,7 +36,7 @@ rm -f "$GIT_DIR/rebased-patches"
 
 git format-patch -k --stdout --full-index --cherry-pick --right-only \
 	--src-prefix=a/ --dst-prefix=b/ --no-renames --no-cover-letter \
-	--pretty=mboxrd \
+	--pretty=mboxrd --topo-order \
 	$git_format_patch_opt \
 	"$revisions" ${restrict_revision+^$restrict_revision} \
 	>"$GIT_DIR/rebased-patches"
diff --git a/git-rebase--merge.sh b/git-rebase--merge.sh
index 91250cbaed..ced38bb3a6 100644
--- a/git-rebase--merge.sh
+++ b/git-rebase--merge.sh
@@ -143,7 +143,7 @@ write_basic_state
 rm -f "$(git rev-parse --git-path REBASE_HEAD)"
 
 msgnum=0
-for cmt in $(git rev-list --reverse --no-merges "$revisions")
+for cmt in $(git rev-list --topo-order --reverse --no-merges "$revisions")
 do
 	msgnum=$(($msgnum + 1))
 	echo "$cmt" > "$state_dir/cmt.$msgnum"
diff --git a/t/t3425-rebase-topology-merges.sh b/t/t3425-rebase-topology-merges.sh
index 5f892e33d7..fd8efe84fe 100755
--- a/t/t3425-rebase-topology-merges.sh
+++ b/t/t3425-rebase-topology-merges.sh
@@ -70,9 +70,8 @@ test_run_rebase () {
 		test_linear_range "\'"$expected"\'" d..
 	"
 }
-#TODO: make order consistent across all flavors of rebase
-test_run_rebase success 'e n o' ''
-test_run_rebase success 'e n o' -m
+test_run_rebase success 'n o e' ''
+test_run_rebase success 'n o e' -m
 test_run_rebase success 'n o e' -i
 
 test_run_rebase () {
@@ -87,9 +86,8 @@ test_run_rebase () {
 		test_linear_range "\'"$expected"\'" c..
 	"
 }
-#TODO: make order consistent across all flavors of rebase
-test_run_rebase success 'd e n o' ''
-test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' ''
+test_run_rebase success 'd n o e' -m
 test_run_rebase success 'd n o e' -i
 
 test_run_rebase () {
@@ -104,9 +102,8 @@ test_run_rebase () {
 		test_linear_range "\'"$expected"\'" c..
 	"
 }
-#TODO: make order consistent across all flavors of rebase
-test_run_rebase success 'd e n o' ''
-test_run_rebase success 'd e n o' -m
+test_run_rebase success 'd n o e' ''
+test_run_rebase success 'd n o e' -m
 test_run_rebase success 'd n o e' -i
 
 if ! test_have_prereq REBASE_P; then
-- 
2.20.0.8.g5de428d695
Previous: Elijah NewrenNext: Elijah Newren
Message 42 of 67 in “Reimplement rebase --merge via interactive machinery”
  1. 0/2 Reimplement rebase --merge via interactive machineryElijah Newren, Nov 8, 2018
  2. 1/2 git-rebase, sequencer: extend --quiet option for the interactive machineryElijah Newren, Nov 8, 2018
  3. Johannes SchindelinNov 12, 2018
  4. 2/2 rebase: Implement --merge via git-rebase--interactiveElijah Newren, Nov 8, 2018
  5. Johannes SchindelinNov 12, 2018
  6. Phillip WoodNov 12, 2018
  7. Johannes SchindelinNov 13, 2018
  8. Elijah NewrenNov 14, 2018
  9. Elijah NewrenNov 13, 2018
  10. Elijah NewrenNov 14, 2018
  11. Johannes SchindelinNov 15, 2018
  12. Elijah NewrenNov 8, 2018
  13. 0/7 Reimplement rebase --merge via interactive machineryElijah Newren, Nov 22, 2018
  14. 4/7 git-rebase, sequencer: extend --quiet option for the interactive machineryElijah Newren, Nov 22, 2018
  15. 2/7 t5407: add a test demonstrating how interactive handles --skip differentlyElijah Newren, Nov 22, 2018
  16. 1/7 rebase: fix incompatible options error messageElijah Newren, Nov 22, 2018
  17. Johannes SchindelinNov 28, 2018
  18. Elijah NewrenNov 28, 2018
  19. Duy NguyenNov 28, 2018
  20. Elijah NewrenNov 28, 2018
  21. 7/7 rebase: Implement --merge via the interactive machineryElijah Newren, Nov 22, 2018
  22. 5/7 git-legacy-rebase: simplify unnecessary triply-nested ifElijah Newren, Nov 22, 2018
  23. 3/7 am, rebase--merge: do not overlook --skip'ed commits with post-rewriteElijah Newren, Nov 22, 2018
  24. 6/7 rebase: define linearization ordering and enforce itElijah Newren, Nov 22, 2018
  25. 0/8 Reimplement rebase --merge via interactive machineryElijah Newren, Dec 11, 2018
  26. 1/8 rebase: make builtin and legacy script error messages the sameElijah Newren, Dec 11, 2018
  27. 4/8 am, rebase--merge: do not overlook --skip'ed commits with post-rewriteElijah Newren, Dec 11, 2018
  28. Johannes SchindelinJan 21, 2019
  29. Elijah NewrenJan 21, 2019
  30. Johannes SchindelinJan 21, 2019
  31. 2/8 rebase: fix incompatible options error messageElijah Newren, Dec 11, 2018
  32. 6/8 git-legacy-rebase: simplify unnecessary triply-nested ifElijah Newren, Dec 11, 2018
  33. 8/8 rebase: Implement --merge via the interactive machineryElijah Newren, Dec 11, 2018
  34. 5/8 git-rebase, sequencer: extend --quiet option for the interactive machineryElijah Newren, Dec 11, 2018
  35. Johannes SchindelinJan 21, 2019
  36. Elijah NewrenJan 21, 2019
  37. Johannes SchindelinJan 21, 2019
  38. Johannes SchindelinJan 21, 2019
  39. Junio C HamanoJan 22, 2019
  40. Phillip WoodFeb 20, 2019
  41. Elijah NewrenFeb 21, 2019
  42. 7/8 rebase: define linearization ordering and enforce itElijah Newren, Dec 11, 2018
  43. 3/8 t5407: add a test demonstrating how interactive handles --skip differentlyElijah Newren, Dec 11, 2018
  44. Elijah NewrenJan 7, 2019
  45. Junio C HamanoJan 7, 2019
  46. Junio C HamanoJan 7, 2019
  47. Elijah NewrenJan 7, 2019
  48. Elijah NewrenJan 11, 2019
  49. Johannes SchindelinJan 18, 2019
  50. Johannes SchindelinJan 18, 2019
  51. Junio C HamanoJan 18, 2019
  52. Elijah NewrenJan 18, 2019
  53. Johannes SchindelinJan 18, 2019
  54. Junio C HamanoJan 18, 2019
  55. Johannes SchindelinJan 21, 2019
  56. Johannes SchindelinJan 21, 2019
  57. Johannes SchindelinJan 21, 2019
  58. Elijah NewrenJan 21, 2019
  59. 0/8 Reimplement rebase --merge via interactive machineryElijah Newren, Jan 29, 2019
  60. 1/8 rebase: make builtin and legacy script error messages the sameElijah Newren, Jan 29, 2019
  61. 6/8 git-legacy-rebase: simplify unnecessary triply-nested ifElijah Newren, Jan 29, 2019
  62. 4/8 am, rebase--merge: do not overlook --skip'ed commits with post-rewriteElijah Newren, Jan 29, 2019
  63. 8/8 rebase: implement --merge via the interactive machineryElijah Newren, Jan 29, 2019
  64. 5/8 git-rebase, sequencer: extend --quiet option for the interactive machineryElijah Newren, Jan 29, 2019
  65. 7/8 rebase: define linearization ordering and enforce itElijah Newren, Jan 29, 2019
  66. 2/8 rebase: fix incompatible options error messageElijah Newren, Jan 29, 2019
  67. 3/8 t5407: add a test demonstrating how interactive handles --skip differentlyElijah Newren, Jan 29, 2019

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.