[v6,0/5] rebase: teach rebase --keep-base

Message

Denton Liu April 17, 2019, 6:01 p.m. UTC

Thanks for the comments, Junio and Phillip.

I've fixed the ASCII art graphs and also refactored the if-else into a
goto tower.

Hopefully, this will be the last reroll of this series ;)

---

This patchset now depends "[PATCH 1/8] tests (rebase): spell out the
`--keep-empty` option" which is the first patch of Johannes's "Do not
use abbreviated options in tests" patchset[1]. (Thanks for catching
that, Johannes!)

Changes since v1:

* Squashed old set into one patch
* Fixed indentation style and dangling else
* Added more documentation after discussion with Ævar

Changes since v2:

* Add testing for rebase --fork-point behaviour
* Add testing for rebase fast-forward behaviour
* Make rebase --onto fast-forward in more cases
* Update documentation to include use-case

Changes since v3:

* Fix tests failing on bash 4.2
* Fix typo in t3431 comment

Changes since v4:

* Make rebase --fork-point fast-forward in more cases

Changes since v5:

* Fix graph illustrations so that the "branch off" is visually in the
  correct place
* Refactor if-else in can_fast_forward into one-level-deep ifs to
  increase clarity

[1]: https://public-inbox.org/git/a1b4[e-mail]/

Denton Liu (6):
  t3431: add rebase --fork-point tests
  t3432: test rebase fast-forward behavior
  rebase: refactor can_fast_forward into goto tower
  rebase: fast-forward --onto in more cases
  rebase: fast-forward --fork-point in more cases
  rebase: teach rebase --keep-base

 Documentation/git-rebase.txt     | 30 +++++++++--
 builtin/rebase.c                 | 86 +++++++++++++++++++++++---------
 t/t3400-rebase.sh                |  2 +-
 t/t3404-rebase-interactive.sh    |  2 +-
 t/t3416-rebase-onto-threedots.sh | 57 +++++++++++++++++++++
 t/t3431-rebase-fork-point.sh     | 57 +++++++++++++++++++++
 t/t3432-rebase-fast-forward.sh   | 83 ++++++++++++++++++++++++++++++
 7 files changed, 289 insertions(+), 28 deletions(-)
 create mode 100755 t/t3431-rebase-fork-point.sh
 create mode 100755 t/t3432-rebase-fast-forward.sh

Range-diff against v5:
1:  0f1e9ac5c8 ! 1:  eb64f6c91d t3431: add rebase --fork-point tests
    @@ -18,9 +18,9 @@
     +
     +. ./test-lib.sh
     +
    -+# A---B---D---E       (master)
    -+#     \
    -+#      C*---F---G (side)
    ++# A---B---D---E    (master)
    ++#      \
    ++#       C*---F---G (side)
     +#
     +# C was formerly part of master but master was rewound to remove C
     +#
2:  148027a14b = 2:  4c087ec041 t3432: test rebase fast-forward behavior
-:  ---------- > 3:  3d348d2c37 rebase: refactor can_fast_forward into goto tower
3:  ec55da0719 ! 4:  2559ab54a2 rebase: fast-forward --onto in more cases
    @@ -5,8 +5,8 @@
         Before, when we had the following graph,
     
                 A---B---C (master)
    -                \
    -                 D (side)
    +                 \
    +                  D (side)
     
         running 'git rebase --onto master... master side' would result in D
         being always rebased, no matter what. However, the desired behavior is
    @@ -49,6 +49,7 @@
     
         While we're at it, remove a trailing whitespace from rebase.c.
     
    +    Helped-by: Phillip Wood <[e-mail]>
         Signed-off-by: Denton Liu <[e-mail]>
     
      diff --git a/builtin/rebase.c b/builtin/rebase.c
    @@ -64,45 +65,26 @@
     +			    struct object_id *head_oid, struct object_id *merge_base)
      {
      	struct commit *head = lookup_commit(the_repository, head_oid);
    --	struct commit_list *merge_bases;
    --	int res;
    -+	struct commit_list *merge_bases = NULL;
    -+	int res = 0;
    - 
    - 	if (!head)
    - 		return 0;
    + 	struct commit_list *merge_bases = NULL;
     @@
    - 	merge_bases = get_merge_bases(onto, head);
    - 	if (merge_bases && !merge_bases->next) {
    - 		oidcpy(merge_base, &merge_bases->item->object.oid);
    --		res = oideq(merge_base, &onto->object.oid);
    -+		if (!oideq(merge_base, &onto->object.oid))
    -+			goto done;
    - 	} else {
    - 		oidcpy(merge_base, &null_oid);
    --		res = 0;
    -+		goto done;
    - 	}
    -+
    + 	if (!oideq(merge_base, &onto->object.oid))
    + 		goto done;
    + 
     +	if (!upstream)
     +		goto done;
     +
    - 	free_commit_list(merge_bases);
    ++	free_commit_list(merge_bases);
     +	merge_bases = get_merge_bases(upstream, head);
    -+	if (merge_bases && !merge_bases->next) {
    -+		if (!oideq(&onto->object.oid, &merge_bases->item->object.oid))
    -+			goto done;
    -+	} else
    ++	if (!merge_bases || merge_bases->next) {
     +		goto done;
    ++	}
     +
    -+	res = 1;
    ++	if (!oideq(&onto->object.oid, &merge_bases->item->object.oid))
    ++		goto done;
     +
    -+done:
    -+	if (merge_bases)
    -+		free_commit_list(merge_bases);
    - 	return res && is_linear_history(onto, head);
    - }
    + 	res = 1;
      
    + done:
     @@
      
      	/*
    @@ -115,7 +97,8 @@
     -	    !is_interactive(&options) && !options.restrict_revision &&
     -	    options.upstream &&
     -	    !oidcmp(&options.upstream->object.oid, &options.onto->object.oid)) {
    -+	if (can_fast_forward(options.onto, options.upstream, &options.orig_head, &merge_base) &&
    ++	if (can_fast_forward(options.onto, options.upstream, &options.orig_head,
    ++		    &merge_base) &&
     +	    !is_interactive(&options) && !options.restrict_revision) {
      		int flag;
      
4:  2256a902c1 ! 5:  0a466e830f rebase: fast-forward --fork-point in more cases
    @@ -27,8 +27,8 @@
      {
      	struct commit *head = lookup_commit(the_repository, head_oid);
     @@
    + 	if (!oideq(merge_base, &onto->object.oid))
      		goto done;
    - 	}
      
     +	if (restrict_revision && !oideq(&restrict_revision->object.oid, merge_base))
     +		goto done;
    @@ -40,7 +40,8 @@
      	 * Check if we are already based on onto with linear history,
      	 * but this should be done if this is not an interactive rebase.
      	 */
    --	if (can_fast_forward(options.onto, options.upstream, &options.orig_head, &merge_base) &&
    +-	if (can_fast_forward(options.onto, options.upstream, &options.orig_head,
    +-		    &merge_base) &&
     -	    !is_interactive(&options) && !options.restrict_revision) {
     +	if (can_fast_forward(options.onto, options.upstream, options.restrict_revision,
     +		    &options.orig_head, &merge_base) &&
5:  d6e7e1ca4b = 6:  c542c7afc1 rebase: teach rebase --keep-base