threads / patch / 30422

patch, 2 partst2020-checkout-detach: check for the number of orphaned commits

Subject: [PATCH 1/2] t2020-checkout-detach: check for the number of orphaned commits

## tl;dr

4 messages between May 4, 2012 and May 5, 2012. Diffs are folded; open one to read it.

replies: 3people: 3as markdown or json

Johannes Sixt· May 4, 2012, 18:13 UTC · lore

Change the test that orphans commits to leave 2 commits behind. Add a test that leaves only one of these behind.

The next patch will truncate the list of orphaned commits earlier. With this preliminary update, its effect will become more obvious.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 I noticed that when you have a detached history, and walk back only part
 of it, the warning lists all of the detached history even if the part
 that ends at the new HEAD is not orphaned. This patch prepares the tests,
 and the next patch changes git-checkout.

 t/t2020-checkout-detach.sh | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)

Show changes to t/t2020-checkout-detach.sh +16 −5

diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index b37ce25..1839b32 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -11,14 +11,13 @@ check_not_detached () {
 	git symbolic-ref -q HEAD >/dev/null
 }
 
-ORPHAN_WARNING='you are leaving .* commit.*behind'
 PREV_HEAD_DESC='Previous HEAD position was'
 check_orphan_warning() {
-	test_i18ngrep "$ORPHAN_WARNING" "$1" &&
+	test_i18ngrep "you are leaving $2 behind" "$1" &&
 	test_i18ngrep ! "$PREV_HEAD_DESC" "$1"
 }
 check_no_orphan_warning() {
-	test_i18ngrep ! "$ORPHAN_WARNING" "$1" &&
+	test_i18ngrep ! "you are leaving .* commit.*behind" "$1" &&
 	test_i18ngrep "$PREV_HEAD_DESC" "$1"
 }
 
@@ -110,12 +109,24 @@ test_expect_success 'checkout warns on orphan commits' '
 	git checkout --detach two &&
 	echo content >orphan &&
 	git add orphan &&
-	git commit -a -m orphan &&
+	git commit -a -m orphan1 &&
+	echo new content >orphan &&
+	git commit -a -m orphan2 &&
+	orphan2=$(git rev-parse HEAD) &&
 	git checkout master 2>stderr
 '
 
 test_expect_success 'checkout warns on orphan commits: output' '
-	check_orphan_warning stderr
+	check_orphan_warning stderr "2 commits"
+'
+
+test_expect_success 'checkout warns orphaning 1 of 2 commits' '
+	git checkout "$orphan2" &&
+	git checkout HEAD^ 2>stderr
+'
+
+test_expect_success 'checkout warns orphaning 1 of 2 commits: output' '
+	check_orphan_warning stderr "2 commits"
 '
 
 test_expect_success 'checkout does not warn leaving ref tip' '

-- 
1.7.10.1.462.g199f411

Johannes Sixt· May 4, 2012, 18:14 UTC · re: Johannes Sixt · lore

[PATCH 2/2] checkout (detached): truncate list of orphaned commits at the new HEAD

When git checkout switches from a detached HEAD to any other commit, then all orphaned commits were listed in a warning:

  Warning: you are leaving 2 commits behind...:

    a5e5396 another fixup
    6aa1af6 fixup foo

But if the new commit is actually one from this list (6aa1af6 in this example), then the list in the warning can be truncated at the new HEAD, because history beginning at HEAD is not "left behind". This makes it so.

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin/checkout.c         | 13 +++++++------
 t/t2020-checkout-detach.sh |  2 +-
 2 files changed, 8 insertions(+), 7 deletions(-)

Show changes to 2 files +8 −7

builtin/checkout.c, t/t2020-checkout-detach.sh

diff --git a/builtin/checkout.c b/builtin/checkout.c
index 23fc56d..c93efe4 100644
--- a/builtin/checkout.c
+++ b/builtin/checkout.c
@@ -672,10 +672,10 @@ static void suggest_reattach(struct commit *commit, struct rev_info *revs)
  * HEAD.  If it is not reachable from any ref, this is the last chance
  * for the user to do so without resorting to reflog.
  */
-static void orphaned_commit_warning(struct commit *commit)
+static void orphaned_commit_warning(struct commit *old, struct commit *new)
 {
 	struct rev_info revs;
-	struct object *object = &commit->object;
+	struct object *object = &old->object;
 	struct object_array refs;
 
 	init_revisions(&revs, NULL);
@@ -685,16 +685,17 @@ static void orphaned_commit_warning(struct commit *commit)
 	add_pending_object(&revs, object, sha1_to_hex(object->sha1));
 
 	for_each_ref(add_pending_uninteresting_ref, &revs);
+	add_pending_sha1(&revs, "HEAD", new->object.sha1, UNINTERESTING);
 
 	refs = revs.pending;
 	revs.leak_pending = 1;
 
 	if (prepare_revision_walk(&revs))
 		die(_("internal error in revision walk"));
-	if (!(commit->object.flags & UNINTERESTING))
-		suggest_reattach(commit, &revs);
+	if (!(old->object.flags & UNINTERESTING))
+		suggest_reattach(old, &revs);
 	else
-		describe_detached_head(_("Previous HEAD position was"), commit);
+		describe_detached_head(_("Previous HEAD position was"), old);
 
 	clear_commit_marks_for_object_array(&refs, ALL_REV_FLAGS);
 	free(refs.objects);
@@ -731,7 +732,7 @@ static int switch_branches(struct checkout_opts *opts, struct branch_info *new)
 	}
 
 	if (!opts->quiet && !old.path && old.commit && new->commit != old.commit)
-		orphaned_commit_warning(old.commit);
+		orphaned_commit_warning(old.commit, new->commit);
 
 	update_refs_for_switch(opts, &old, new);
 
diff --git a/t/t2020-checkout-detach.sh b/t/t2020-checkout-detach.sh
index 1839b32..8100537 100755
--- a/t/t2020-checkout-detach.sh
+++ b/t/t2020-checkout-detach.sh
@@ -126,7 +126,7 @@ test_expect_success 'checkout warns orphaning 1 of 2 commits' '
 '
 
 test_expect_success 'checkout warns orphaning 1 of 2 commits: output' '
-	check_orphan_warning stderr "2 commits"
+	check_orphan_warning stderr "1 commit"
 '
 
 test_expect_success 'checkout does not warn leaving ref tip' '

-- 
1.7.10.1.462.g199f411

Junio C Hamano· May 4, 2012, 19:51 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] checkout (detached): truncate list of orphaned commits at the new HEAD

Johannes Sixt <j6t@kdbg.org> writes:

Show 11 quoted lines

> When git checkout switches from a detached HEAD to any other commit, then
> all orphaned commits were listed in a warning:
>
>   Warning: you are leaving 2 commits behind...:
>
>     a5e5396 another fixup
>     6aa1af6 fixup foo
>
> But if the new commit is actually one from this list (6aa1af6 in this
> example), then the list in the warning can be truncated at the new HEAD,
> because history beginning at HEAD is not "left behind". This makes it so.

If you did:

	git checkout master^0
        git commit --allow-empty -m emp0
        git commit --allow-empty -m emp1
        tip=$(git rev-parse HEAD)
        git reset --hard HEAD^

and then

	git checkout $tip

it used to warn about "emp0" but because you are checking out "emp1", which is a descendant of "emp0", nothing is lost and no lossage is reported with the updated code.

Makes sense; thanks.

Jeff King· May 5, 2012, 09:01 UTC · re: Johannes Sixt · lore

Re: [PATCH 2/2] checkout (detached): truncate list of orphaned commits at the new HEAD

On Fri, May 04, 2012 at 08:14:48PM +0200, Johannes Sixt wrote:

Show 11 quoted lines

> When git checkout switches from a detached HEAD to any other commit, then
> all orphaned commits were listed in a warning:
> 
>   Warning: you are leaving 2 commits behind...:
> 
>     a5e5396 another fixup
>     6aa1af6 fixup foo
> 
> But if the new commit is actually one from this list (6aa1af6 in this
> example), then the list in the warning can be truncated at the new HEAD,
> because history beginning at HEAD is not "left behind". This makes it so.

Thanks, this has bugged me a few times in the past, but I've never gotten around to investigating and fixing it.

>  builtin/checkout.c         | 13 +++++++------
>  t/t2020-checkout-detach.sh |  2 +-
>  2 files changed, 8 insertions(+), 7 deletions(-)

The patch looks right to me.

-Peff