commit acca3e784b8d3ba91774e39aa50d325db895b434
from: Stefan Sperling <stsp@stsp.name>
via: Thomas Adam <thomas@xteddy.org>
date: Fri Jan 24 22:02:34 2025 UTC

do not delete objects reached via nested tags during 'gotadmin cleanup'

Make gotadmin cleanup resolve multiple levels of nested tags, in order
to ensure that objects referenced via nested tags will not get deleted.
Add test coverage for this edge case.

commit - 8517c370c59976628da6168ebb6e0ae4590f74b5
commit + acca3e784b8d3ba91774e39aa50d325db895b434
blob - d0e57adc0ed74a6bc614e805ff6e35851221db21
blob + 447c6f8af7e5c3ce28f544ad0f24a825e28567ad
--- lib/repository_admin.c
+++ lib/repository_admin.c
@@ -1008,33 +1008,55 @@ load_commit_or_tag(int *ncommits, struct got_object_id
 		}
 
 		if (tag) {
-			/* Find a tree object to scan. */
+			struct got_object_id *id;
+
 			obj_type = got_object_tag_get_object_type(tag);
+			while (obj_type == GOT_OBJ_TYPE_TAG) {
+				struct got_tag_object *next_tag;
+
+				id = got_object_tag_get_object_id(tag);
+				if (!got_object_idset_contains(traversed_ids,
+				    id)) {
+					err = got_object_idset_add(
+					    traversed_ids, id, NULL);
+					if (err)
+						goto done;
+				}
+
+				err = got_object_open_as_tag(&next_tag, repo,
+				    id);
+				if (err)
+					goto done;
+
+				got_object_tag_close(tag);
+				tag = next_tag;
+				obj_type = got_object_tag_get_object_type(tag);
+			}
+			id = got_object_tag_get_object_id(tag);
 			switch (obj_type) {
 			case GOT_OBJ_TYPE_COMMIT:
 				err = got_object_open_as_commit(&commit, repo,
-				    got_object_tag_get_object_id(tag));
+				    id);
 				if (err)
 					goto done;
 				tree_id = got_object_commit_get_tree_id(commit);
 				break;
 			case GOT_OBJ_TYPE_TREE:
-				tree_id = got_object_tag_get_object_id(tag);
+				tree_id = id;
 				break;
-			default:
-				/*
-				 * Tag points at something other than a
-				 * commit or tree. Leave this weird tag object
-				 * and the object it points to.
-				 */
+			case GOT_OBJ_TYPE_BLOB:
 				if (got_object_idset_contains(traversed_ids,
-				    got_object_tag_get_object_id(tag)))
+				    id))
 					break;
-				err = got_object_idset_add(traversed_ids,
-				    got_object_tag_get_object_id(tag), NULL);
+				err = got_object_idset_add(traversed_ids, id,
+				    NULL);
 				if (err)
 					goto done;
 				break;
+			default:
+				/* should not happen */
+				err = got_error(GOT_ERR_OBJ_TYPE);
+				goto done;
 			}
 		} else if (tree_id == NULL) {
 			/* Blob which has already been marked as traversed. */
blob - 8b953f1ee28370617e8dbb0865f36ea6604711e0
blob + f55eceed41973eb52bc69e9fb467054747cde376
--- regress/cmdline/cleanup.sh
+++ regress/cmdline/cleanup.sh
@@ -499,13 +499,105 @@ test_cleanup_non_commit_ref() {
 
 	# create a reference which points at the tree
 	got ref -r $testroot/repo -c "$tree_id" treeref
+
+	inner_tag_date=$(date +%s)
+
+	# Create a nested tag of another tree.
+	# Ensure that gotadmin cleanup follows chains of tags and
+	# all objects referenced via this chain.
+
+	echo bar > $testroot/t/bar
+	bar_id=$(git -C $testroot/repo hash-object -t blob -w $testroot/t/bar)
+
+	printf "10644 blob $foo_id\tfoo\n" > $testroot/tree-desc2
+	printf "10644 blob $bar_id\tbar\n" >> $testroot/tree-desc2
+	tree_id2=$(git -C $testroot/repo mktree < $testroot/tree-desc2)
+
+	# verify that the second tree object can be read
+	got cat -r $testroot/repo "$tree_id2" > $testroot/stdout
+	printf "$bar_id 0010644 bar\n" > $testroot/stdout.expected
+	printf "$foo_id 0010644 foo\n" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	cat > $testroot/inner-tag-desc <<EOF
+object $tree_id2
+type tree
+tag treetag
+tagger $GOT_AUTHOR $inner_tag_date +0000
+
+tagging a tree
+EOF
+	inner_tag_id=$(git -C $testroot/repo hash-object -t tag -w \
+		$testroot/inner-tag-desc)
+
+	# verify that the inner tag object can be read
+	got cat -r $testroot/repo "$inner_tag_id" > $testroot/stdout
+
+	cat > $testroot/stdout.expected <<EOF
+object $tree_id2
+type tree
+tag treetag
+tagger $GOT_AUTHOR $inner_tag_date +0000
+messagelen 16
+
+tagging a tree
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	tag_date=$(date +%s)
+
+	cat > $testroot/tag-desc <<EOF
+object $inner_tag_id
+type tag
+tag tagtag
+tagger $GOT_AUTHOR $tag_date +0000
+
+tagging a tag
+EOF
+	tag_id=$(git -C $testroot/repo hash-object -t tag -w $testroot/tag-desc)
+
+	# verify that the tag object can be read
+	got cat -r $testroot/repo "$tag_id" > $testroot/stdout
+
+	cat > $testroot/stdout.expected <<EOF
+object $inner_tag_id
+type tag
+tag tagtag
+tagger $GOT_AUTHOR $tag_date +0000
+messagelen 15
 
+tagging a tag
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# create a reference which points at the outer tag
+	got ref -r $testroot/repo -c "$tag_id" tagref
+
 	# list references
 	got ref -r $testroot/repo -l > $testroot/stdout
 	cat > $testroot/stdout.expected <<EOF
 HEAD: refs/heads/master
 refs/blobref: $foo_id
 refs/heads/master: $commit_id
+refs/tagref: $tag_id
 refs/treeref: $tree_id
 EOF
 	cmp -s $testroot/stdout.expected $testroot/stdout
@@ -547,6 +639,58 @@ EOF
 		return 1
 	fi
 
+	# verify that the second tree object can be read
+	got cat -r $testroot/repo "$tree_id2" > $testroot/stdout
+	printf "$bar_id 0010644 bar\n" > $testroot/stdout.expected
+	printf "$foo_id 0010644 foo\n" >> $testroot/stdout.expected
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# verify that the inner tag object can be read
+	got cat -r $testroot/repo "$inner_tag_id" > $testroot/stdout
+
+	cat > $testroot/stdout.expected <<EOF
+object $tree_id2
+type tree
+tag treetag
+tagger $GOT_AUTHOR $inner_tag_date +0000
+messagelen 16
+
+tagging a tree
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
+	# verify that the outer tag object can be read
+	got cat -r $testroot/repo "$tag_id" > $testroot/stdout
+
+	cat > $testroot/stdout.expected <<EOF
+object $inner_tag_id
+type tag
+tag tagtag
+tagger $GOT_AUTHOR $tag_date +0000
+messagelen 15
+
+tagging a tag
+EOF
+	cmp -s $testroot/stdout.expected $testroot/stdout
+	ret=$?
+	if [ $ret -ne 0 ]; then
+		diff -u $testroot/stdout.expected $testroot/stdout
+		test_done "$testroot" "$ret"
+		return 1
+	fi
+
 	test_done "$testroot" "$ret"
 }