"GOT", but the "O" is a cute, smiling pufferfish. Index | Thread | Search

From:
Stefan Sperling <stsp@stsp.name>
Subject:
fix packed vs. on-disk reference with got_ref_list()
To:
gameoftrees@openbsd.org
Date:
Mon, 5 Sep 2022 18:34:05 +0200

Download raw body.

Thread
I came across a bug where 'got branch -lt' shows the same branch twice:

$ got branch -lt
* zoo: 844bdd2dc1594079c107b206fc594f7df1bfbe6f
  master: 53094754dfbdac666a73a2bc15fbe51962a1f407
~ zoo: 53094754dfbdac666a73a2bc15fbe51962a1f407

This patch fixes the problem and adds a test for the issue. ok?
 
diff e15c42decfa8a80fb91cc1e19b467efc34a8c05d 954554d7bb97f20f338512045d2d27a5ec872a2e
commit - e15c42decfa8a80fb91cc1e19b467efc34a8c05d
commit + 954554d7bb97f20f338512045d2d27a5ec872a2e
blob - 78eb9ebab2c20cab529d7f47d39f8d4829ac8699
blob + a7ca5b927daf30815b370c13f3ec304c395a1051
--- lib/reference.c
+++ lib/reference.c
@@ -860,6 +860,27 @@ got_reflist_insert(struct got_reflist_entry **newp,
 	new->ref = ref;
 	*newp = new;
 
+	if (cmp_cb != got_ref_cmp_by_name &&
+	    (new->ref->flags & GOT_REF_IS_PACKED)) {
+		/*
+		 * If we are not sorting elements by name then we must still
+		 * detect collisions between a packed ref and an on-disk ref
+		 * using the same name. On-disk refs take precedence and are
+		 * already present on the list before packed refs get added.
+		 */
+		TAILQ_FOREACH(re, refs, entry) {
+			err = got_ref_cmp_by_name(NULL, &cmp,
+			    re->ref, new->ref);
+			if (err)
+				return err;
+			if (cmp == 0) {
+				free(new);
+				*newp = NULL;
+				return NULL;
+			}
+		}
+	}
+
 	/*
 	 * We must de-duplicate entries on insert because packed-refs may
 	 * contain redundant entries. On-disk refs take precedence.
blob - a025c5170abf06b7f707406ee2d9193c3fa6ff43
blob + cab7171d5632c0bdf72e14e1d6dda98d3a4ce810
--- regress/cmdline/branch.sh
+++ regress/cmdline/branch.sh
@@ -499,6 +499,44 @@ test_branch_show() {
 
 }
 
+test_branch_packed_ref_collision() {
+	local testroot=`test_init branch_packed_ref_collision`
+	local commit_id=`git_show_head $testroot/repo`
+
+	got br -r $testroot/repo zoo > $testroot/stdout
+	got co -b zoo $testroot/repo $testroot/wt > /dev/null
+	echo "modified alpha" > $testroot/wt/alpha
+
+	# sleep in order to ensure that a significant fraction of time
+	# passes between commits; required for got branch -t option below
+	sleep 1
+
+	(cd $testroot/wt && got commit -m "modified alpha" >/dev/null)
+	local commit_id2=`git_show_branch_head $testroot/repo zoo`
+
+	# Fabricate a packed reference which points to an older commit
+	# and collides with the existing on-disk reference
+	echo '# pack-refs with: peeled fully-peeled sorted' > \
+		$testroot/repo/.git/packed-refs
+	echo "$commit_id refs/heads/zoo" >> $testroot/repo/.git/packed-refs
+
+	# Bug: This command used to show both packed and on-disk
+	# variants of ref/heads/zoo:
+	(cd $testroot/wt && got br -lt > $testroot/stdout)
+
+	echo "* zoo: $commit_id2" > $testroot/stdout.expected
+	echo "  master: $commit_id" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout $testroot/stdout.expected
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	test_done "$testroot" "$ret"
+}
+
 test_parseargs "$@"
 run_test test_branch_create
 run_test test_branch_list
@@ -506,3 +544,4 @@ run_test test_branch_delete
 run_test test_branch_delete_current_branch
 run_test test_branch_delete_packed
 run_test test_branch_show
+run_test test_branch_packed_ref_collision