On Mon, Mar 18, 2024 at 09:55:53PM +0100, Christian Weisgerber wrote:
> Now we're getting somewhere.  The contents of refs/tags/ correspond
> to the output of "ref -l tags".  I can send you the full outputs
> off-list, but looking at the contents and timestamps of objects/pack/
> and refs/tags/, I think that I must have run "git gc" and the contents
> of ref/tags/ reflect the tags that have been added since.

When you ran git gc the existing refs were moved into the packed-refs file.
The problem is that packed-refs was not searched if an argument was given
to got ref -l.

Additionally, got ref -l tags/1.0.0 works, but got ref -l 1.0.0 wouldn't
work even for non-packed refs.

This patch adds regression tests to ensure that got ref -l behaviour is
identical for packed and non-packed refs, and fixes both bugs.

ok?

diff refs/heads/main refs/heads/list-packed-refs
commit - 9c177e8d04414b7be4a02d64ed06dcc402a7b0a8
commit + fb28d3d7c5fe48ff982110807c6343077f19719a
blob - da5a08c3bc2211a734d318e4a4cbb22062bfbd18
blob + c578dc9c58e835b547a901f7a3dbec472b4517fa
--- lib/reference.c
+++ lib/reference.c
@@ -857,6 +857,8 @@ gather_on_disk_refs(struct got_reflist_head *refs, con
 	const struct got_error *err = NULL;
 	DIR *d = NULL;
 	char *path_subdir;
+	struct got_reference *ref;
+	struct got_reflist_entry *new;
 
 	while (subdir[0] == '/')
 		subdir++;
@@ -865,12 +867,40 @@ gather_on_disk_refs(struct got_reflist_head *refs, con
 		return got_error_from_errno("asprintf");
 
 	d = opendir(path_subdir);
-	if (d == NULL)
-		goto done;
+	if (d == NULL) {
+		char *refname;
 
+		if (errno != ENOTDIR)
+			goto done;
+
+		/* This could be a regular on-disk reference file. */
+		free(path_subdir);
+		err = got_path_dirname(&path_subdir, subdir);
+		if (err)
+			return err;
+		err = got_path_basename(&refname, subdir);
+		if (err) {
+			free(path_subdir);
+			return err;
+		}
+		err = open_ref(&ref, path_refs, path_subdir, refname,
+		    0, got_repo_get_object_format(repo));
+		free(path_subdir);
+		free(refname);
+		if (err) {
+			if (err->code == GOT_ERR_NOT_REF)
+				return NULL;
+			return err;
+		}
+		err = got_reflist_insert(&new, refs, ref,
+		    cmp_cb, cmp_arg);
+		if (err || new == NULL /* duplicate */)
+			got_ref_close(ref);
+		return err;
+	}
+
 	for (;;) {
 		struct dirent *dent;
-		struct got_reference *ref;
 		char *child;
 		int type;
 
@@ -895,7 +925,6 @@ gather_on_disk_refs(struct got_reflist_head *refs, con
 			if (err)
 				goto done;
 			if (ref) {
-				struct got_reflist_entry *new;
 				err = got_reflist_insert(&new, refs, ref,
 				    cmp_cb, cmp_arg);
 				if (err || new == NULL /* duplicate */)
@@ -925,6 +954,33 @@ done:
 	return err;
 }
 
+static int
+match_packed_ref(struct got_reference *ref, const char *ref_namespace)
+{
+	const char *name = got_ref_get_name(ref);
+	int namespace_is_absolute = (strncmp(ref_namespace, "refs/", 5) == 0);
+
+	if (namespace_is_absolute) {
+		return (strcmp(name, ref_namespace) == 0 ||
+		    got_path_is_child(name, ref_namespace,
+		    strlen(ref_namespace)));
+	}
+
+	/* Match all "subdirectories" as we do with on-disk refs. */
+	while (*name != '\0') {
+		while (*name == '/')
+			name++;
+		if (strcmp(name, ref_namespace) == 0 ||
+		    got_path_is_child(name, ref_namespace,
+		    strlen(ref_namespace)))
+			return 1;
+		while (*name != '\0' && *name != '/')
+			name++;
+	}
+
+	return 0;
+}
+
 const struct got_error *
 got_ref_list(struct got_reflist_head *refs, struct got_repository *repo,
     const char *ref_namespace, got_ref_cmp_cb cmp_cb, void *cmp_arg)
@@ -955,14 +1011,7 @@ got_ref_list(struct got_reflist_head *refs, struct got
 			goto done;
 	} else {
 		/* Try listing a single reference. */
-		const char *refname = ref_namespace;
-		path_refs = get_refs_dir_path(repo, refname);
-		if (path_refs == NULL) {
-			err = got_error_from_errno("get_refs_dir_path");
-			goto done;
-		}
-		err = open_ref(&ref, path_refs, "", refname, 0,
-		    got_repo_get_object_format(repo));
+		err = got_ref_open(&ref, repo, ref_namespace, 0);
 		if (err) {
 			if (err->code != GOT_ERR_NOT_REF)
 				goto done;
@@ -1049,15 +1098,10 @@ got_ref_list(struct got_reflist_head *refs, struct got
 			if (err)
 				goto done;
 			if (ref) {
-				if (ref_namespace) {
-					const char *name;
-					name = got_ref_get_name(ref);
-					if (!got_path_is_child(name,
-					    ref_namespace,
-					    strlen(ref_namespace))) {
-						got_ref_close(ref);
-						continue;
-					}
+				if (ref_namespace &&
+				    !match_packed_ref(ref, ref_namespace)) {
+					got_ref_close(ref);
+					continue;
 				}
 				err = got_reflist_insert(&new, refs, ref,
 				    cmp_cb, cmp_arg);
blob - f19cabc30854c8e77db6d827d2b8ed078c8362f5
blob + 12ee9f0325d237ae1d8a5a962b5aa23fb7261605
--- regress/cmdline/ref.sh
+++ regress/cmdline/ref.sh
@@ -438,6 +438,91 @@ test_ref_list() {
 	test_done "$testroot" "$ret"
 }
 
+test_ref_list_packed_refs() {
+	local testroot=`test_init ref_list_packed_refs`
+	local commit_id=`git_show_head $testroot/repo`
+	local tag=1.0.0
+	local tag2=2.0.0
+
+	# create tag with Git
+	git -C $testroot/repo tag -a -m 'test' $tag
+	# create tag with Got
+	(cd $testroot/repo && got tag -m 'test' $tag2 > /dev/null)
+
+	tag_id=`got ref -r $testroot/repo -l \
+		| grep "^refs/tags/$tag" | tr -d ' ' | cut -d: -f2`
+	local tagger_time=`git_show_tagger_time $testroot/repo $tag`
+	d1=`date -u -r $tagger_time +"%a %b %e %X %Y UTC"`
+	tag_id2=`got ref -r $testroot/repo -l \
+		| grep "^refs/tags/$tag2" | tr -d ' ' | cut -d: -f2`
+	local tagger_time2=`git_show_tagger_time $testroot/repo $tag2`
+	d2=`date -u -r $tagger_time2 +"%a %b %e %X %Y UTC"`
+
+	for i in 1 2; do
+		if [ $i -eq 2 ]; then
+			# Move all refs into the packed-refs file
+			git -C $testroot/repo pack-refs --all
+		fi
+
+		got ref -r $testroot/repo -l > $testroot/stdout
+
+		cat > $testroot/stdout.expected <<EOF
+HEAD: refs/heads/master
+refs/heads/master: $commit_id
+refs/tags/1.0.0: $tag_id
+refs/tags/2.0.0: $tag_id2
+EOF
+		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
+
+		got ref -r $testroot/repo -l tags/$tag > $testroot/stdout
+
+		cat > $testroot/stdout.expected <<EOF
+refs/tags/1.0.0: $tag_id
+EOF
+		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
+
+		got ref -r $testroot/repo -l $tag2 > $testroot/stdout
+
+		cat > $testroot/stdout.expected <<EOF
+refs/tags/2.0.0: $tag_id2
+EOF
+		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
+
+		got ref -r $testroot/repo -l tags > $testroot/stdout
+		cat > $testroot/stdout.expected <<EOF
+refs/tags/1.0.0: $tag_id
+refs/tags/2.0.0: $tag_id2
+EOF
+		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
+	done
+
+	test_done "$testroot" "0"
+}
+
 test_ref_commit_keywords() {
 	local testroot=$(test_init ref_commit_keywords)
 	local repo="$testroot/repo"
@@ -519,4 +604,5 @@ test_parseargs "$@"
 run_test test_ref_create
 run_test test_ref_delete
 run_test test_ref_list
+run_test test_ref_list_packed_refs
 run_test test_ref_commit_keywords