Lead a snapshot name to its tag ahead of a branch

The refs listing offers a tag's snapshot under the stem of its name, and
the lookup back from that name settled on whatever ref matched first, so
a branch spelled like the stem was archived in the tag's place.

2 files changed, 65 insertions, 31 deletions

diff --git a/source/ui-snapshot.c b/source/ui-snapshot.cindex 6a059a0..f4e79f7 100644--- a/source/ui-snapshot.c+++ b/source/ui-snapshot.c@@ -184,55 +184,80 @@ static const struct cgit_snapshot_format *find_format(const char *filename) 	return NULL; } -static int resolves(const char *rev)+/*+ * Whether rev names a ref, under refs/tags alone when tags_only is set, and+ * if it does, rev is replaced by the name to archive.+ */+static int resolves(struct strbuf *rev, int tags_only) { 	struct object_id oid;+	struct strbuf tag = STRBUF_INIT;+	int found = 0;  	// The name goes no further than this lookup, so a leading dash is 	// fine here, while the wider syntax is refused as in cgit_valid_rev.-	return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL) &&-		!repo_get_oid(the_repository, rev, &oid);+	if (check_refname_format(rev->buf, REFNAME_ALLOW_ONELEVEL))+		return 0;+	strbuf_addf(&tag, "refs/tags/%s", rev->buf);+	if (!repo_get_oid(the_repository, tag.buf, &oid)) {+		strbuf_swap(rev, &tag);+		found = 1;+	} else if (!tags_only && !repo_get_oid(the_repository, rev->buf, &oid)) {+		found = 1;+	}+	strbuf_release(&tag);+	return found; } -static const char *ref_from_filename(const struct cgit_repo *repo, const char *filename,-	const struct cgit_snapshot_format *format)+/*+ * The names a snapshot file could stand for, in the order the link builder+ * prefers them: the whole stem, the stem without the repository's prefix,+ * and that with a leading v or V, since a tag is often written with one the+ * file named after it is not.+ */+static int candidate_resolves(const struct cgit_repo *repo, const char *stem,+	struct strbuf *rev, int tags_only) {-	struct strbuf rev = STRBUF_INIT;-	const char *repo_prefix;-	int found = 1;--	strbuf_addstr(&rev, filename);-	strbuf_setlen(&rev, rev.len - strlen(format->suffix));+	const char *repo_prefix = cgit_snapshot_prefix(repo); -	if (resolves(rev.buf))-		goto out;+	strbuf_reset(rev);+	strbuf_addstr(rev, stem);+	if (resolves(rev, tags_only))+		return 1; -	repo_prefix = cgit_snapshot_prefix(repo);-	if (starts_with(rev.buf, repo_prefix)) {-		const char *rest = rev.buf + strlen(repo_prefix);+	if (starts_with(rev->buf, repo_prefix)) {+		const char *rest = rev->buf + strlen(repo_prefix);  		while (*rest == '-' || *rest == '_') 			rest++;-		strbuf_splice(&rev, 0, rest - rev.buf, "", 0);+		strbuf_splice(rev, 0, rest - rev->buf, "", 0); 	}+	if (resolves(rev, tags_only))+		return 1; -	if (resolves(rev.buf))-		goto out;--	// A tag is often written with a leading v while the file named after it-	// is not, so that is tried last.-	strbuf_insert(&rev, 0, "v", 1);-	if (resolves(rev.buf))-		goto out;+	strbuf_insert(rev, 0, "v", 1);+	if (resolves(rev, tags_only))+		return 1; -	strbuf_splice(&rev, 0, 1, "V", 1);-	if (resolves(rev.buf))-		goto out;+	strbuf_splice(rev, 0, 1, "V", 1);+	return resolves(rev, tags_only);+} -	found = 0;-	strbuf_release(&rev);+/*+ * The links are built from tags, so every candidate is tried as a tag before+ * any is tried as a branch, which a branch spelled like a tag's stem would+ * otherwise win.+ */+static const char *ref_from_filename(const struct cgit_repo *repo, const char *filename,+	const struct cgit_snapshot_format *format)+{+	struct strbuf rev = STRBUF_INIT;+	char *stem = xstrndup(filename, strlen(filename) - strlen(format->suffix));+	int found = candidate_resolves(repo, stem, &rev, 1) || candidate_resolves(repo, stem, &rev, 0); -out:+	free(stem);+	if (!found)+		strbuf_release(&rev); 	return found ? strbuf_detach(&rev, NULL) : NULL; } 
diff --git a/source/ui-snapshot.c b/source/ui-snapshot.c
index 6a059a0..f4e79f7 100644
--- a/source/ui-snapshot.c
+++ b/source/ui-snapshot.c
@@ -184,55 +184,80 @@ static const struct cgit_snapshot_format *find_format(const char *filename)
 	return NULL;
 }
 
-static int resolves(const char *rev)
+/*
+ * Whether rev names a ref, under refs/tags alone when tags_only is set, and
+ * if it does, rev is replaced by the name to archive.
+ */
+static int resolves(struct strbuf *rev, int tags_only)
 {
 	struct object_id oid;
+	struct strbuf tag = STRBUF_INIT;
+	int found = 0;
 
 	// The name goes no further than this lookup, so a leading dash is
 	// fine here, while the wider syntax is refused as in cgit_valid_rev.
-	return !check_refname_format(rev, REFNAME_ALLOW_ONELEVEL) &&
-		!repo_get_oid(the_repository, rev, &oid);
+	if (check_refname_format(rev->buf, REFNAME_ALLOW_ONELEVEL))
+		return 0;
+	strbuf_addf(&tag, "refs/tags/%s", rev->buf);
+	if (!repo_get_oid(the_repository, tag.buf, &oid)) {
+		strbuf_swap(rev, &tag);
+		found = 1;
+	} else if (!tags_only && !repo_get_oid(the_repository, rev->buf, &oid)) {
+		found = 1;
+	}
+	strbuf_release(&tag);
+	return found;
 }
 
-static const char *ref_from_filename(const struct cgit_repo *repo, const char *filename,
-	const struct cgit_snapshot_format *format)
+/*
+ * The names a snapshot file could stand for, in the order the link builder
+ * prefers them: the whole stem, the stem without the repository's prefix,
+ * and that with a leading v or V, since a tag is often written with one the
+ * file named after it is not.
+ */
+static int candidate_resolves(const struct cgit_repo *repo, const char *stem,
+	struct strbuf *rev, int tags_only)
 {
-	struct strbuf rev = STRBUF_INIT;
-	const char *repo_prefix;
-	int found = 1;
-
-	strbuf_addstr(&rev, filename);
-	strbuf_setlen(&rev, rev.len - strlen(format->suffix));
+	const char *repo_prefix = cgit_snapshot_prefix(repo);
 
-	if (resolves(rev.buf))
-		goto out;
+	strbuf_reset(rev);
+	strbuf_addstr(rev, stem);
+	if (resolves(rev, tags_only))
+		return 1;
 
-	repo_prefix = cgit_snapshot_prefix(repo);
-	if (starts_with(rev.buf, repo_prefix)) {
-		const char *rest = rev.buf + strlen(repo_prefix);
+	if (starts_with(rev->buf, repo_prefix)) {
+		const char *rest = rev->buf + strlen(repo_prefix);
 
 		while (*rest == '-' || *rest == '_')
 			rest++;
-		strbuf_splice(&rev, 0, rest - rev.buf, "", 0);
+		strbuf_splice(rev, 0, rest - rev->buf, "", 0);
 	}
+	if (resolves(rev, tags_only))
+		return 1;
 
-	if (resolves(rev.buf))
-		goto out;
-
-	// A tag is often written with a leading v while the file named after it
-	// is not, so that is tried last.
-	strbuf_insert(&rev, 0, "v", 1);
-	if (resolves(rev.buf))
-		goto out;
+	strbuf_insert(rev, 0, "v", 1);
+	if (resolves(rev, tags_only))
+		return 1;
 
-	strbuf_splice(&rev, 0, 1, "V", 1);
-	if (resolves(rev.buf))
-		goto out;
+	strbuf_splice(rev, 0, 1, "V", 1);
+	return resolves(rev, tags_only);
+}
 
-	found = 0;
-	strbuf_release(&rev);
+/*
+ * The links are built from tags, so every candidate is tried as a tag before
+ * any is tried as a branch, which a branch spelled like a tag's stem would
+ * otherwise win.
+ */
+static const char *ref_from_filename(const struct cgit_repo *repo, const char *filename,
+	const struct cgit_snapshot_format *format)
+{
+	struct strbuf rev = STRBUF_INIT;
+	char *stem = xstrndup(filename, strlen(filename) - strlen(format->suffix));
+	int found = candidate_resolves(repo, stem, &rev, 1) || candidate_resolves(repo, stem, &rev, 0);
 
-out:
+	free(stem);
+	if (!found)
+		strbuf_release(&rev);
 	return found ? strbuf_detach(&rev, NULL) : NULL;
 }
 
diff --git a/tests/t0110-snapshot.sh b/tests/t0110-snapshot.shindex 373a05d..2ce8316 100755--- a/tests/t0110-snapshot.sh+++ b/tests/t0110-snapshot.sh@@ -181,4 +181,13 @@ test_expect_success GZIP 'a compressed snapshot is made without a PATH' ' 	gzip -t nopath.tar.gz ' +# A link offers the snapshot of a tag, and the name on it has to lead back to+# that tag even when a branch is spelled like the tag's stem.+test_expect_success 'a snapshot name leads to the tag ahead of a branch' '+	git -C repos/vtags update-ref refs/heads/1.0 $(git -C repos/vtags commit-tree -m shadow HEAD^{tree}) &&+	CGIT_CONFIG="$PWD/vtagsrc" QUERY_STRING="url=vtags/snapshot/vtags-1.0.tar.gz" cgit >tmp &&+	strip_headers <tmp >tag.tar.gz &&+	test "$(gzip -dc tag.tar.gz | git -C repos/vtags get-tar-commit-id)" = "$(git -C repos/vtags rev-parse refs/tags/v1.0^{commit})"+'+ test_done
diff --git a/tests/t0110-snapshot.sh b/tests/t0110-snapshot.sh
index 373a05d..2ce8316 100755
--- a/tests/t0110-snapshot.sh
+++ b/tests/t0110-snapshot.sh
@@ -181,4 +181,13 @@ test_expect_success GZIP 'a compressed snapshot is made without a PATH' '
 	gzip -t nopath.tar.gz
 '
 
+# A link offers the snapshot of a tag, and the name on it has to lead back to
+# that tag even when a branch is spelled like the tag's stem.
+test_expect_success 'a snapshot name leads to the tag ahead of a branch' '
+	git -C repos/vtags update-ref refs/heads/1.0 $(git -C repos/vtags commit-tree -m shadow HEAD^{tree}) &&
+	CGIT_CONFIG="$PWD/vtagsrc" QUERY_STRING="url=vtags/snapshot/vtags-1.0.tar.gz" cgit >tmp &&
+	strip_headers <tmp >tag.tar.gz &&
+	test "$(gzip -dc tag.tar.gz | git -C repos/vtags get-tar-commit-id)" = "$(git -C repos/vtags rev-parse refs/tags/v1.0^{commit})"
+'
+
 test_done