[PATCH 00/16] [V2] Add namespace support to cgit

[PATCH 00/16] [V2] Add namespace support to cgit

[richard.maw]

This is a rebase of the previous patch series,
since it stopped applying and one of the commits was taken independently.

The following changes since commit ff9893ac8192579a00dd4c73ddff18ab232099a6:

  Fix qry.head leak on error (2016-07-12 01:06:04 +0200)

are available in the git repository at:

  git://git.gitano.org.uk/cgit.git 

for you to fetch changes up to 2a20425f4e76e378c138094c79b78aa476c019b5:

  Add documentation for repo.agefile (2016-07-13 20:09:37 +0100)

Richard Maw (16):
  Fix archive generation
  Add a wrapper for get_sha1 called cgit_get_sha1
  Parse repo.namespace
  Print out parsed namespace on request
  Set GIT_NAMESPACE when repo.namespace is provided
  Look up refs in namespace with cgit_get_sha1
  Guess the default branch based on current namespace
  Add cgit_for_each_namespaced_ref_in helper
  Find the default branch based on the contents of the namespace
  Only display refs in current namespace
  Add namespace support to dumb-clone
  Display notes from namespace
  Add documentation for repo.namespace
  Allow agefile to be set per-repository
  Update contrib script to update agefiles per namespace
  Add documentation for repo.agefile

 cgit.c                             | 63 +++++++++++++++++++++++++++++++++-----
 cgit.h                             |  6 ++++
 cgitrc.5.txt                       | 46 ++++++++++++++++++++++++++++
 contrib/hooks/post-receive.agefile | 15 ++++++++-
 shared.c                           | 60 ++++++++++++++++++++++++++++++++++++
 ui-blob.c                          |  6 ++--
 ui-clone.c                         | 23 +++++++++++++-
 ui-commit.c                        |  2 +-
 ui-diff.c                          |  4 +--
 ui-log.c                           |  2 +-
 ui-patch.c                         |  4 +--
 ui-plain.c                         |  2 +-
 ui-refs.c                          |  6 ++--
 ui-repolist.c                      |  3 +-
 ui-shared.c                        |  8 ++---
 ui-snapshot.c                      | 14 ++++-----
 ui-tag.c                           |  2 +-
 ui-tree.c                          |  2 +-
 18 files changed, 232 insertions(+), 36 deletions(-)

-- 
2.9.0

[PATCH 01/16] Fix archive generation

[richard.maw]

The get_ref_from_filename function is expected to return a sha1.
It didn't actually do this,
instead returning the ref that would under normal circumstances resolve to that.

Since we're going to resolve refs in a way that is namespace aware
we need to return the sha1 rather than the ref,
since the archive is created by libgit code that is not namespace aware,
and it would try to resolve the ref again.

This previously worked fine
because it would resolve the ref the same way both times.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 ui-snapshot.c | 4 ++--
 1 file changed, 2 insertions(+), 2 deletions(-)

diff --git a/ui-snapshot.c b/ui-snapshot.c
index f68e877..c6c3656 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -174,10 +174,10 @@ static const char *get_ref_from_filename(const char *url, const char *filename,
 		goto out;
 
 	result = 0;
-	strbuf_release(&snapshot);
 
 out:
-	return result ? strbuf_detach(&snapshot, NULL) : NULL;
+	strbuf_release(&snapshot);
+	return result ? xstrdup(sha1_to_hex(sha1)) : NULL;
 }
 
 void cgit_print_snapshot(const char *head, const char *hex,
-- 
2.9.0

[PATCH 02/16] Add a wrapper for get_sha1 called cgit_get_sha1

[richard.maw]

This will later be changed to include namespace resolution,
but the call sites are changed now to keep the changes small.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c        |  2 +-
 cgit.h        |  2 ++
 shared.c      |  5 +++++
 ui-blob.c     |  6 +++---
 ui-commit.c   |  2 +-
 ui-diff.c     |  4 ++--
 ui-log.c      |  2 +-
 ui-patch.c    |  4 ++--
 ui-plain.c    |  2 +-
 ui-shared.c   |  8 ++++----
 ui-snapshot.c | 10 +++++-----
 ui-tag.c      |  2 +-
 ui-tree.c     |  2 +-
 13 files changed, 29 insertions(+), 22 deletions(-)

diff --git a/cgit.c b/cgit.c
index 9427c4a..94d452c 100644
--- a/cgit.c
+++ b/cgit.c
@@ -615,7 +615,7 @@ static int prepare_repo_cmd(void)
 		return 1;
 	}
 
-	if (get_sha1(ctx.qry.head, sha1)) {
+	if (cgit_get_sha1(ctx.qry.head, sha1)) {
 		char *old_head = ctx.qry.head;
 		ctx.qry.head = xstrdup(ctx.repo->defbranch);
 		cgit_print_error_page(404, "Not found",
diff --git a/cgit.h b/cgit.h
index 325432b..49f0557 100644
--- a/cgit.h
+++ b/cgit.h
@@ -391,4 +391,6 @@ extern char *expand_macros(const char *txt);
 
 extern char *get_mimetype_for_filename(const char *filename);
 
+extern int cgit_get_sha1(const char *name, unsigned char *sha1);
+
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index a63633b..d82c07b 100644
--- a/shared.c
+++ b/shared.c
@@ -601,3 +601,8 @@ char *get_mimetype_for_filename(const char *filename)
 	fclose(file);
 	return NULL;
 }
+
+int cgit_get_sha1(const char *name, unsigned char *sha1)
+{
+	return get_sha1(name, sha1);
+}
diff --git a/ui-blob.c b/ui-blob.c
index d388489..21d21ed 100644
--- a/ui-blob.c
+++ b/ui-blob.c
@@ -52,7 +52,7 @@ int cgit_ref_path_exists(const char *path, const char *ref, int file_only)
 		.file_only = file_only
 	};
 
-	if (get_sha1(ref, sha1))
+	if (cgit_get_sha1(ref, sha1))
 		return 0;
 	if (sha1_object_info(sha1, &size) != OBJ_COMMIT)
 		return 0;
@@ -82,7 +82,7 @@ int cgit_print_file(char *path, const char *head, int file_only)
 		.file_only = file_only
 	};
 
-	if (get_sha1(head, sha1))
+	if (cgit_get_sha1(head, sha1))
 		return -1;
 	type = sha1_object_info(sha1, &size);
 	if (type == OBJ_COMMIT) {
@@ -132,7 +132,7 @@ void cgit_print_blob(const char *hex, char *path, const char *head, int file_onl
 			return;
 		}
 	} else {
-		if (get_sha1(head, sha1)) {
+		if (cgit_get_sha1(head, sha1)) {
 			cgit_print_error_page(404, "Not found",
 					"Bad ref: %s", head);
 			return;
diff --git a/ui-commit.c b/ui-commit.c
index 099d294..5ba0791 100644
--- a/ui-commit.c
+++ b/ui-commit.c
@@ -26,7 +26,7 @@ void cgit_print_commit(char *hex, const char *prefix)
 	if (!hex)
 		hex = ctx.qry.head;
 
-	if (get_sha1(hex, sha1)) {
+	if (cgit_get_sha1(hex, sha1)) {
 		cgit_print_error_page(400, "Bad request",
 				"Bad object id: %s", hex);
 		return;
diff --git a/ui-diff.c b/ui-diff.c
index edee793..9256d12 100644
--- a/ui-diff.c
+++ b/ui-diff.c
@@ -402,7 +402,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 
 	if (!new_rev)
 		new_rev = ctx.qry.head;
-	if (get_sha1(new_rev, new_rev_sha1)) {
+	if (cgit_get_sha1(new_rev, new_rev_sha1)) {
 		cgit_print_error_page(404, "Not found",
 			"Bad object name: %s", new_rev);
 		return;
@@ -416,7 +416,7 @@ void cgit_print_diff(const char *new_rev, const char *old_rev,
 	new_tree_sha1 = commit->tree->object.oid.hash;
 
 	if (old_rev) {
-		if (get_sha1(old_rev, old_rev_sha1)) {
+		if (cgit_get_sha1(old_rev, old_rev_sha1)) {
 			cgit_print_error_page(404, "Not found",
 				"Bad object name: %s", old_rev);
 			return;
diff --git a/ui-log.c b/ui-log.c
index c97b8e0..28ef3c0 100644
--- a/ui-log.c
+++ b/ui-log.c
@@ -325,7 +325,7 @@ static const char *disambiguate_ref(const char *ref, int *must_free_result)
 	struct strbuf longref = STRBUF_INIT;
 
 	strbuf_addf(&longref, "refs/heads/%s", ref);
-	if (get_sha1(longref.buf, oid.hash) == 0) {
+	if (cgit_get_sha1(longref.buf, oid.hash) == 0) {
 		*must_free_result = 1;
 		return strbuf_detach(&longref, NULL);
 	}
diff --git a/ui-patch.c b/ui-patch.c
index 4c051e8..ff09a8e 100644
--- a/ui-patch.c
+++ b/ui-patch.c
@@ -24,7 +24,7 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
 	if (!new_rev)
 		new_rev = ctx.qry.head;
 
-	if (get_sha1(new_rev, new_rev_sha1)) {
+	if (cgit_get_sha1(new_rev, new_rev_sha1)) {
 		cgit_print_error_page(404, "Not found",
 				"Bad object id: %s", new_rev);
 		return;
@@ -37,7 +37,7 @@ void cgit_print_patch(const char *new_rev, const char *old_rev,
 	}
 
 	if (old_rev) {
-		if (get_sha1(old_rev, old_rev_sha1)) {
+		if (cgit_get_sha1(old_rev, old_rev_sha1)) {
 			cgit_print_error_page(404, "Not found",
 					"Bad object id: %s", old_rev);
 			return;
diff --git a/ui-plain.c b/ui-plain.c
index 97cf639..88dd1a8 100644
--- a/ui-plain.c
+++ b/ui-plain.c
@@ -181,7 +181,7 @@ void cgit_print_plain(void)
 	if (!rev)
 		rev = ctx.qry.head;
 
-	if (get_sha1(rev, sha1)) {
+	if (cgit_get_sha1(rev, sha1)) {
 		cgit_print_error_page(404, "Not found", "Not found");
 		return;
 	}
diff --git a/ui-shared.c b/ui-shared.c
index e39d004..ca40d42 100644
--- a/ui-shared.c
+++ b/ui-shared.c
@@ -1079,11 +1079,11 @@ void cgit_compose_snapshot_prefix(struct strbuf *filename, const char *base,
 	 * name starts with {v,V}[0-9] and the prettify mapping is injective,
 	 * i.e. each stripped tag can be inverted without ambiguities.
 	 */
-	if (get_sha1(fmt("refs/tags/%s", ref), sha1) == 0 &&
+	if (cgit_get_sha1(fmt("refs/tags/%s", ref), sha1) == 0 &&
 	    (ref[0] == 'v' || ref[0] == 'V') && isdigit(ref[1]) &&
-	    ((get_sha1(fmt("refs/tags/%s", ref + 1), sha1) == 0) +
-	     (get_sha1(fmt("refs/tags/v%s", ref + 1), sha1) == 0) +
-	     (get_sha1(fmt("refs/tags/V%s", ref + 1), sha1) == 0) == 1))
+	    ((cgit_get_sha1(fmt("refs/tags/%s", ref + 1), sha1) == 0) +
+	     (cgit_get_sha1(fmt("refs/tags/v%s", ref + 1), sha1) == 0) +
+	     (cgit_get_sha1(fmt("refs/tags/V%s", ref + 1), sha1) == 0) == 1))
 		ref++;
 
 	strbuf_addf(filename, "%s-%s", base, ref);
diff --git a/ui-snapshot.c b/ui-snapshot.c
index c6c3656..93af28e 100644
--- a/ui-snapshot.c
+++ b/ui-snapshot.c
@@ -111,7 +111,7 @@ static int make_snapshot(const struct cgit_snapshot_format *format,
 {
 	unsigned char sha1[20];
 
-	if (get_sha1(hex, sha1)) {
+	if (cgit_get_sha1(hex, sha1)) {
 		cgit_print_error_page(404, "Not found",
 				"Bad object id: %s", hex);
 		return 1;
@@ -150,7 +150,7 @@ static const char *get_ref_from_filename(const char *url, const char *filename,
 	strbuf_addstr(&snapshot, filename);
 	strbuf_setlen(&snapshot, snapshot.len - strlen(format->suffix));
 
-	if (get_sha1(snapshot.buf, sha1) == 0)
+	if (cgit_get_sha1(snapshot.buf, sha1) == 0)
 		goto out;
 
 	reponame = cgit_repobasename(url);
@@ -162,15 +162,15 @@ static const char *get_ref_from_filename(const char *url, const char *filename,
 		strbuf_splice(&snapshot, 0, new_start - snapshot.buf, "", 0);
 	}
 
-	if (get_sha1(snapshot.buf, sha1) == 0)
+	if (cgit_get_sha1(snapshot.buf, sha1) == 0)
 		goto out;
 
 	strbuf_insert(&snapshot, 0, "v", 1);
-	if (get_sha1(snapshot.buf, sha1) == 0)
+	if (cgit_get_sha1(snapshot.buf, sha1) == 0)
 		goto out;
 
 	strbuf_splice(&snapshot, 0, 1, "V", 1);
-	if (get_sha1(snapshot.buf, sha1) == 0)
+	if (cgit_get_sha1(snapshot.buf, sha1) == 0)
 		goto out;
 
 	result = 0;
diff --git a/ui-tag.c b/ui-tag.c
index 6b838cb..7372770 100644
--- a/ui-tag.c
+++ b/ui-tag.c
@@ -51,7 +51,7 @@ void cgit_print_tag(char *revname)
 		revname = ctx.qry.head;
 
 	strbuf_addf(&fullref, "refs/tags/%s", revname);
-	if (get_sha1(fullref.buf, sha1)) {
+	if (cgit_get_sha1(fullref.buf, sha1)) {
 		cgit_print_error_page(404, "Not found",
 			"Bad tag reference: %s", revname);
 		goto cleanup;
diff --git a/ui-tree.c b/ui-tree.c
index 120066c..ae2acce 100644
--- a/ui-tree.c
+++ b/ui-tree.c
@@ -306,7 +306,7 @@ void cgit_print_tree(const char *rev, char *path)
 	if (!rev)
 		rev = ctx.qry.head;
 
-	if (get_sha1(rev, sha1)) {
+	if (cgit_get_sha1(rev, sha1)) {
 		cgit_print_error_page(404, "Not found",
 			"Invalid revision name: %s", rev);
 		return;
-- 
2.9.0

[PATCH 03/16] Parse repo.namespace

[richard.maw]

This contains the unexpanded name of the namespace
rather than the base ref of the namespace,
since the git namespace mechanism works by setting GIT_NAMESPACE
and on the first call to get_git_namespace() it gets expanded.

We need to save this for a later call to prepare_repo_cmd,
rather than trying to process it here,
since we can only do it once,
and we have other uses for the unexpanded name.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 2 ++
 cgit.h | 1 +
 2 files changed, 3 insertions(+)

diff --git a/cgit.c b/cgit.c
index 94d452c..e525264 100644
--- a/cgit.c
+++ b/cgit.c
@@ -45,6 +45,8 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 		repo->homepage = xstrdup(value);
 	else if (!strcmp(name, "defbranch"))
 		repo->defbranch = xstrdup(value);
+	else if (!strcmp(name, "namespace"))
+		repo->namespace = xstrdup(value);
 	else if (!strcmp(name, "snapshots"))
 		repo->snapshots = ctx.cfg.snapshots & cgit_parse_snapshots_mask(value);
 	else if (!strcmp(name, "enable-commit-graph"))
diff --git a/cgit.h b/cgit.h
index 49f0557..93de0ea 100644
--- a/cgit.h
+++ b/cgit.h
@@ -87,6 +87,7 @@ struct cgit_repo {
 	struct string_list readme;
 	char *section;
 	char *clone_url;
+	char *namespace;
 	char *logo;
 	char *logo_link;
 	int snapshots;
-- 
2.9.0

[PATCH 04/16] Print out parsed namespace on request

[richard.maw]

This is not strictly necessary,
as we do not have any way to generate namespace entries from a scan-path,
but I'd rather not leave this as a surprise
to someone who comes up with a good namespace discovery mechanism.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 2 ++
 1 file changed, 2 insertions(+)

diff --git a/cgit.c b/cgit.c
index e525264..9b2733b 100644
--- a/cgit.c
+++ b/cgit.c
@@ -778,6 +778,8 @@ static void print_repo(FILE *f, struct cgit_repo *repo)
 	fprintf(f, "repo.url=%s\n", repo->url);
 	fprintf(f, "repo.name=%s\n", repo->name);
 	fprintf(f, "repo.path=%s\n", repo->path);
+	if (repo->namespace)
+		fprintf(f, "repo.namespace=%s\n", repo->namespace);
 	if (repo->owner)
 		fprintf(f, "repo.owner=%s\n", repo->owner);
 	if (repo->desc) {
-- 
2.9.0

[PATCH 05/16] Set GIT_NAMESPACE when repo.namespace is provided

[richard.maw]

This causes any namespace-aware code
to only handle refs under that namespace.

Currently this doesn't do much
as the only namespace aware code is in recieve-pack and upload-pack,
which are not handled by CGit.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 5 +++++
 1 file changed, 5 insertions(+)

diff --git a/cgit.c b/cgit.c
index 9b2733b..eae2f34 100644
--- a/cgit.c
+++ b/cgit.c
@@ -566,6 +566,11 @@ static int prepare_repo_cmd(void)
 	/* The path to the git repository. */
 	setenv("GIT_DIR", ctx.repo->path, 1);
 
+	/*  Set the namespace in the environment,
+	 *  so it gets loaded on the first get_git_namespace() */
+	if (ctx.repo->namespace)
+		setenv("GIT_NAMESPACE", ctx.repo->namespace, 1);
+
 	/* Do not look in /etc/ for gitconfig and gitattributes. */
 	setenv("GIT_CONFIG_NOSYSTEM", "1", 1);
 	setenv("GIT_ATTR_NOSYSTEM", "1", 1);
-- 
2.9.0

[PATCH 06/16] Look up refs in namespace with cgit_get_sha1

[richard.maw]

This causes all ref resolving to look for the requested branch
inside the current namespace.

Previously any form of git revision would be accepted,
but ref resolving isn't namespace aware
and it would be infeasible to replicate all its behaviour,
so we stick to providing the most common cases
of a sha1, an absolute ref, or a partial ref.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 shared.c | 47 ++++++++++++++++++++++++++++++++++++++++++++++-
 1 file changed, 46 insertions(+), 1 deletion(-)

diff --git a/shared.c b/shared.c
index d82c07b..81a5cd8 100644
--- a/shared.c
+++ b/shared.c
@@ -602,7 +602,52 @@ char *get_mimetype_for_filename(const char *filename)
 	return NULL;
 }
 
+static int namespaced_dwim_ref_get_sha1(const char *name, unsigned char *sha1)
+{
+	/* The standard git name disambiguation order is:
+	     $name
+	     refs/$name
+	     refs/tags/$name
+	     refs/heads/$name
+	     refs/remotes/$name (not sure why)
+	     refs/remotes/$name/HEAD
+	   we don't care about remotes, so we can skip those,
+	   and we can't specify a prefix for dwm_ref,
+	   so we have to do this ourselves */
+	static const char *namespaced_ref_patterns[] = {
+		"%s%s",
+		"%srefs/%s",
+		"%srefs/tags/%s",
+		"%srefs/heads/%s",
+		NULL,
+	};
+	const char **p;
+
+	for (p = namespaced_ref_patterns; *p; p++) {
+		char *fullref = NULL;
+		const char *r;
+		fullref = mkpathdup(*p, get_git_namespace(), name);
+		r = resolve_ref_unsafe(fullref, RESOLVE_REF_READING, sha1, NULL);
+		free(fullref);
+		if (r)
+			return 0;
+	}
+	return 1;
+}
+
 int cgit_get_sha1(const char *name, unsigned char *sha1)
 {
-	return get_sha1(name, sha1);
+	if (ctx.repo->namespace) {
+		/* If we have a namespace, we can get either a sha1,
+		   or a possibly abbreviated ref.
+		   Advanced ref forms are not supported at this time
+		   as this would require reimplementing all of ref parsing.
+		   If get_sha1_with_context grows support for a namespaced flag
+		   then this code may go away. */
+		if (get_sha1_hex(name, sha1) == 0)
+			return 0;
+		return namespaced_dwim_ref_get_sha1(name, sha1);
+	} else {
+		return get_sha1(name, sha1);
+	}
 }
-- 
2.9.0

[PATCH 07/16] Guess the default branch based on current namespace

[richard.maw]

resolve_ref_unsafe() can't be told to be namespace aware,
so we need to prepend the namespace beforehand.

Additionally, we need to add the RESOLVE_REF_NO_RECURSE flag,
since otherwise if the commit that is pointed to exists in the root namespace,
it will opt to return that rather than the value in the namespace,
presumably preferring shorter ref names to longer ones.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 14 ++++++++++----
 1 file changed, 10 insertions(+), 4 deletions(-)

diff --git a/cgit.c b/cgit.c
index eae2f34..d9fe2b9 100644
--- a/cgit.c
+++ b/cgit.c
@@ -474,11 +474,17 @@ static char *guess_defbranch(void)
 {
 	const char *ref;
 	unsigned char sha1[20];
-
-	ref = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
-	if (!ref || !starts_with(ref, "refs/heads/"))
+	char *namespaced_head = NULL;
+
+	if (get_git_namespace())
+		namespaced_head = mkpathdup("%sHEAD", get_git_namespace());
+	/* NOTE: RESOLVE_REF_NO_RECURSE is required to prevent it resolving HEAD
+	   into a ref outside of the namespace. */
+	ref = resolve_ref_unsafe(namespaced_head ?: "HEAD", RESOLVE_REF_NO_RECURSE, sha1, NULL);
+	free(namespaced_head);
+	if (!ref || !starts_with(strip_namespace(ref), "refs/heads/"))
 		return "master";
-	return xstrdup(ref + 11);
+	return xstrdup(strip_namespace(ref) + 11);
 }
 /* The caller must free filename and ref after calling this. */
 static inline void parse_readme(const char *readme, char **filename, char **ref, struct cgit_repo *repo)
-- 
2.9.0

[PATCH 08/16] Add cgit_for_each_namespaced_ref_in helper

[richard.maw]

libgit has a for_each_namespaced_ref,
but lacks equivalents for just branches, tags or remotes.

Rather than implementing all of those helpers,
it's more convenient to implement just one
that prepends the namespace to the provided ref base.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.h   |  2 ++
 shared.c | 10 ++++++++++
 2 files changed, 12 insertions(+)

diff --git a/cgit.h b/cgit.h
index 93de0ea..f0d25d6 100644
--- a/cgit.h
+++ b/cgit.h
@@ -394,4 +394,6 @@ extern char *get_mimetype_for_filename(const char *filename);
 
 extern int cgit_get_sha1(const char *name, unsigned char *sha1);
 
+extern int cgit_for_each_namespaced_ref_in(const char *prefix, each_ref_fn fn, void *cb_data);
+
 #endif /* CGIT_H */
diff --git a/shared.c b/shared.c
index 81a5cd8..54940b3 100644
--- a/shared.c
+++ b/shared.c
@@ -651,3 +651,13 @@ int cgit_get_sha1(const char *name, unsigned char *sha1)
 		return get_sha1(name, sha1);
 	}
 }
+
+int cgit_for_each_namespaced_ref_in(const char *prefix, each_ref_fn fn, void *cb_data)
+{
+	struct strbuf strbuf = STRBUF_INIT;
+	int ret;
+	strbuf_addf(&strbuf, "%s%s", get_git_namespace(), prefix);
+	ret = for_each_ref_in(strbuf.buf, fn, cb_data);
+	strbuf_release(&strbuf);
+	return ret;
+}
-- 
2.9.0

[PATCH 09/16] Find the default branch based on the contents of the namespace

[richard.maw]

The find_current_ref callback does not need to be modified
to strip off the namespace prefix,
since the for_each_ref functions don't include the base ref prefix.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/cgit.c b/cgit.c
index d9fe2b9..540200e 100644
--- a/cgit.c
+++ b/cgit.c
@@ -458,7 +458,7 @@ static char *find_default_branch(struct cgit_repo *repo)
 	info.req_ref = repo->defbranch;
 	info.first_ref = NULL;
 	info.match = 0;
-	for_each_branch_ref(find_current_ref, &info);
+	cgit_for_each_namespaced_ref_in("refs/heads/", find_current_ref, &info);
 	if (info.match)
 		ref = info.req_ref;
 	else
-- 
2.9.0

[PATCH 10/16] Only display refs in current namespace

[richard.maw]

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 ui-refs.c | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)

diff --git a/ui-refs.c b/ui-refs.c
index 75f2789..3e4edad 100644
--- a/ui-refs.c
+++ b/ui-refs.c
@@ -183,9 +183,9 @@ void cgit_print_branches(int maxcount)
 
 	list.refs = NULL;
 	list.alloc = list.count = 0;
-	for_each_branch_ref(cgit_refs_cb, &list);
+	cgit_for_each_namespaced_ref_in("refs/heads/", cgit_refs_cb, &list);
 	if (ctx.repo->enable_remote_branches)
-		for_each_remote_ref(cgit_refs_cb, &list);
+		cgit_for_each_namespaced_ref_in("refs/remotes/", cgit_refs_cb, &list);
 
 	if (maxcount == 0 || maxcount > list.count)
 		maxcount = list.count;
@@ -210,7 +210,7 @@ void cgit_print_tags(int maxcount)
 
 	list.refs = NULL;
 	list.alloc = list.count = 0;
-	for_each_tag_ref(cgit_refs_cb, &list);
+	cgit_for_each_namespaced_ref_in("refs/tags/", cgit_refs_cb, &list);
 	if (list.count == 0)
 		return;
 	qsort(list.refs, list.count, sizeof(*list.refs), cmp_tag_age);
-- 
2.9.0

[PATCH 11/16] Add namespace support to dumb-clone

[richard.maw]

This requires namespacing the HEAD symbolic ref
and the list of refs.

Sending HEAD required some tweaking,
since the file itself refers to a namespaced ref,
but we want to provide the ref with its namespace stripped off.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 ui-clone.c | 23 ++++++++++++++++++++++-
 1 file changed, 22 insertions(+), 1 deletion(-)

diff --git a/ui-clone.c b/ui-clone.c
index 5f6606a..c610ed2 100644
--- a/ui-clone.c
+++ b/ui-clone.c
@@ -86,6 +86,9 @@ void cgit_clone_info(void)
 	ctx.page.filename = "info/refs";
 	cgit_print_http_headers();
 	for_each_ref(print_ref_info, NULL);
+	/* NOTE: we pass an empty prefix because we want to enumerate everything
+	   not just all refs under $namespace/refs/ */
+	cgit_for_each_namespaced_ref_in("", print_ref_info, NULL);
 }
 
 void cgit_clone_objects(void)
@@ -105,5 +108,23 @@ void cgit_clone_objects(void)
 
 void cgit_clone_head(void)
 {
-	send_file(git_path("%s", "HEAD"));
+	if (get_git_namespace()) {
+		unsigned char unused[20];
+		char *namespaced_head = NULL;
+		const char *ref;
+		namespaced_head = mkpathdup("%sHEAD", get_git_namespace());
+		/* NOTE: RESOLVE_REF_NO_RECURSE is required to prevent it resolving HEAD
+		   into a ref outside of the namespace. */
+		ref = resolve_ref_unsafe(namespaced_head, RESOLVE_REF_NO_RECURSE, unused, NULL);
+		if (!ref) {
+			cgit_print_error_page(404, "Not found", "Not found");
+			free(namespaced_head);
+			return;
+		}
+		cgit_print_http_headers();
+		htmlf("ref: %s\n", strip_namespace(ref));
+		free(namespaced_head);
+	} else {
+		send_file(git_path("%s", "HEAD"));
+	}
 }
-- 
2.9.0

[PATCH 12/16] Display notes from namespace

[richard.maw]

We must not leave it at its default behaviour,
as that results in notes from the root namespace being used
and ignoring notes from the current namespace,
since it is not namespace aware.

We can handle this by instructing it to not load the default refs,
and providing a set of extra refs to use.

The provided extra refs are globs rather than ref names,
so we should escape them to be sure.

We get an annoying warning if the provided ref does not exist,
so we check whether the ref exists before attempting to provide it.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c | 34 +++++++++++++++++++++++++++++++++-
 1 file changed, 33 insertions(+), 1 deletion(-)

diff --git a/cgit.c b/cgit.c
index 540200e..d428019 100644
--- a/cgit.c
+++ b/cgit.c
@@ -587,7 +587,39 @@ static int prepare_repo_cmd(void)
 	 * load local configuration from the git repository, so we do them both while
 	 * the HOME variables are unset. */
 	setup_git_directory_gently(&nongit);
-	init_display_notes(NULL);
+	if (get_git_namespace()) {
+		struct display_notes_opt opt = {
+			.use_default_notes = 0,
+			.extra_notes_refs = STRING_LIST_INIT_NODUP,
+		};
+		struct strbuf namespaced_note_ref = STRBUF_INIT;
+		struct strbuf glob_escaped = STRBUF_INIT;
+
+		strbuf_add(&namespaced_note_ref, get_git_namespace(),
+		           strlen(get_git_namespace()));
+		strbuf_add(&namespaced_note_ref, "refs/notes/commits",
+		           strlen("refs/notes/commits"));
+
+		if (ref_exists(namespaced_note_ref.buf)) {
+			if (has_glob_specials(namespaced_note_ref.buf)) {
+				for (const char *c = namespaced_note_ref.buf; *c; c++) {
+					if (is_glob_special(*c))
+						strbuf_addchars(&glob_escaped, '\\', 1);
+					strbuf_addchars(&glob_escaped, *c, 1);
+				}
+				string_list_append(&opt.extra_notes_refs, glob_escaped.buf);
+			} else {
+				string_list_append(&opt.extra_notes_refs, namespaced_note_ref.buf);
+			}
+		}
+
+		init_display_notes(&opt);
+		string_list_clear(&opt.extra_notes_refs, 1);
+		strbuf_release(&glob_escaped);
+		strbuf_release(&namespaced_note_ref);
+	} else {
+		init_display_notes(NULL);
+	}
 
 	if (nongit) {
 		const char *name = ctx.repo->name;
-- 
2.9.0

[PATCH 13/16] Add documentation for repo.namespace

[richard.maw]

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgitrc.5.txt | 32 ++++++++++++++++++++++++++++++++
 1 file changed, 32 insertions(+)

diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 9fcf445..6116d1b 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -570,6 +570,10 @@ repo.max-stats::
 repo.name::
 	The value to show as repository name. Default value: <repo.url>.
 
+repo.namespace::
+	Set the git namespace, so that fetching and cloning only provide refs
+        under refs/namespaces. Default value: none.
+
 repo.owner::
 	A value used to identify the owner of the repository. Default value:
 	none.
@@ -772,6 +776,34 @@ Conversely, when a ttl value is zero, the cache is disabled for that
 particular page type, and the page type is never cached.
 
 
+NAMESPACES
+----------
+
+A namespace may be created by creating a "HEAD" symbolic ref for the namespace,
+which may be done with the following command:
+
+....
+git symbolic-ref $namespace_ref_base/HEAD $namespace_ref_base/refs/heads/master
+....
+
+and adding "repo.namespace" for the repository.
+
+For a namespace called "foo/bar", the base ref would be
+"refs/namespaces/foo/refs/namespaces/bar/",
+so the command to create the HEAD ref is:
+
+....
+git symbolic-ref refs/namespaces/foo/refs/namespaces/bar/HEAD \
+                 refs/namespaces/foo/refs/namespaces/bar/refs/heads/master
+....
+
+The config setting for this repository would be:
+
+....
+repo.namespace = foo/bar
+....
+
+
 EXAMPLE CGITRC FILE
 -------------------
 
-- 
2.9.0

[PATCH 14/16] Allow agefile to be set per-repository

[richard.maw]

This adds "repo.agefile", since namespaced repositories share the same files,
and we'd like to be able to see the ages of the refs for each namespace.

Whatever the git server uses for updating the age file must be namespace aware
and must write the age file to a path consistent with "repo.agefile".

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgit.c        | 2 ++
 cgit.h        | 1 +
 ui-repolist.c | 3 ++-
 3 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/cgit.c b/cgit.c
index d428019..a87f245 100644
--- a/cgit.c
+++ b/cgit.c
@@ -88,6 +88,8 @@ static void repo_config(struct cgit_repo *repo, const char *name, const char *va
 		repo->logo = xstrdup(value);
 	else if (!strcmp(name, "logo-link") && value != NULL)
 		repo->logo_link = xstrdup(value);
+	else if (!strcmp(name, "agefile") && value != NULL)
+		repo->agefile = xstrdup(value);
 	else if (!strcmp(name, "hide"))
 		repo->hide = atoi(value);
 	else if (!strcmp(name, "ignore"))
diff --git a/cgit.h b/cgit.h
index f0d25d6..b6994a3 100644
--- a/cgit.h
+++ b/cgit.h
@@ -88,6 +88,7 @@ struct cgit_repo {
 	char *section;
 	char *clone_url;
 	char *namespace;
+	char *agefile;
 	char *logo;
 	char *logo_link;
 	int snapshots;
diff --git a/ui-repolist.c b/ui-repolist.c
index 30915df..6b1afb6 100644
--- a/ui-repolist.c
+++ b/ui-repolist.c
@@ -42,7 +42,8 @@ static int get_repo_modtime(const struct cgit_repo *repo, time_t *mtime)
 		*mtime = repo->mtime;
 		return 1;
 	}
-	strbuf_addf(&path, "%s/%s", repo->path, ctx.cfg.agefile);
+	strbuf_addf(&path, "%s/%s", repo->path,
+	            repo->agefile ? repo->agefile : ctx.cfg.agefile);
 	if (stat(path.buf, &s) == 0) {
 		*mtime = read_agefile(path.buf);
 		if (*mtime) {
-- 
2.9.0

[PATCH 15/16] Update contrib script to update agefiles per namespace

[richard.maw]

This uses the same age file as before if it's in the root namespace,
but puts the agefile in info/web/namespaced/$GIT_NAMESPACE/last-modified.

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 contrib/hooks/post-receive.agefile | 15 ++++++++++++++-
 1 file changed, 14 insertions(+), 1 deletion(-)

diff --git a/contrib/hooks/post-receive.agefile b/contrib/hooks/post-receive.agefile
index 2f72ae9..d0be639 100755
--- a/contrib/hooks/post-receive.agefile
+++ b/contrib/hooks/post-receive.agefile
@@ -10,10 +10,23 @@
 # each of your repositories.
 #
 
-agefile="$(git rev-parse --git-dir)"/info/web/last-modified
+agefile="$(git rev-parse --git-dir)"/info/web
+if [ -z "$GIT_NAMESPACE" ]; then
+	agefile="$agefile/last-modified"
+else
+	agefile="$agefile/namespaced/$GIT_NAMESPACE/last-modified"
+fi
+
+old_IFS="$IFS"
+IFS=/
+for comp in $GIT_NAMESPACE; do
+	nsref="${nsref}refs/namespaces/$comp/"
+done
+IFS="$old_IFS"
 
 mkdir -p "$(dirname "$agefile")" &&
 git for-each-ref \
 	--sort=-authordate --count=1 \
 	--format='%(authordate:iso8601)' \
+	$nsref \
 	>"$agefile"
-- 
2.9.0

[PATCH 16/16] Add documentation for repo.agefile

[richard.maw]

Signed-off-by: Richard Maw <richard.maw at gmail.com>
---
 cgitrc.5.txt | 14 ++++++++++++++
 1 file changed, 14 insertions(+)

diff --git a/cgitrc.5.txt b/cgitrc.5.txt
index 6116d1b..db22269 100644
--- a/cgitrc.5.txt
+++ b/cgitrc.5.txt
@@ -468,6 +468,10 @@ repo.about-filter::
 	Override the default about-filter. Default value: none. See also:
 	"enable-filter-overrides". See also: "FILTER API".
 
+repo.agefile::
+	Override the default agefile. Default value: none. See also:
+	"agefile". See also: "NAMESPACES".
+
 repo.branch-sort::
 	Flag which, when set to "age", enables date ordering in the branch ref
 	list, and when set to "name" enables ordering by branch name. Default
@@ -803,6 +807,16 @@ The config setting for this repository would be:
 repo.namespace = foo/bar
 ....
 
+It's also recommended to set a different age file for the namespace.
+The cgit repository contains a contrib script for generating age-files
+for namespaces in a subdirectory.
+
+The corresponding config for the "foo/bar" namespace would be:
+
+....
+repo.agefile = info/web/namespaced/foo/bar/last-modified
+....
+
 
 EXAMPLE CGITRC FILE
 -------------------
-- 
2.9.0

[PATCH 00/16] [V2] Add namespace support to cgit

[richard.maw]

Also, I wrote this script to generate an environment where it serves
CGit and smart-http over the same server, since I thought it might be
useful to illustrate a sample usage for this.

The delay in resending after rebasing was that I went through 3
different web servers before settling on learning how to configure
apache.

On 15 July 2016 at 23:59, Richard Maw <richard.maw at gmail.com> wrote:
> This is a rebase of the previous patch series,
> since it stopped applying and one of the commits was taken independently.
>
> The following changes since commit ff9893ac8192579a00dd4c73ddff18ab232099a6:
>
>   Fix qry.head leak on error (2016-07-12 01:06:04 +0200)
>
> are available in the git repository at:
>
>   git://git.gitano.org.uk/cgit.git
>
> for you to fetch changes up to 2a20425f4e76e378c138094c79b78aa476c019b5:
>
>   Add documentation for repo.agefile (2016-07-13 20:09:37 +0100)
>
> Richard Maw (16):
>   Fix archive generation
>   Add a wrapper for get_sha1 called cgit_get_sha1
>   Parse repo.namespace
>   Print out parsed namespace on request
>   Set GIT_NAMESPACE when repo.namespace is provided
>   Look up refs in namespace with cgit_get_sha1
>   Guess the default branch based on current namespace
>   Add cgit_for_each_namespaced_ref_in helper
>   Find the default branch based on the contents of the namespace
>   Only display refs in current namespace
>   Add namespace support to dumb-clone
>   Display notes from namespace
>   Add documentation for repo.namespace
>   Allow agefile to be set per-repository
>   Update contrib script to update agefiles per namespace
>   Add documentation for repo.agefile
>
>  cgit.c                             | 63 +++++++++++++++++++++++++++++++++-----
>  cgit.h                             |  6 ++++
>  cgitrc.5.txt                       | 46 ++++++++++++++++++++++++++++
>  contrib/hooks/post-receive.agefile | 15 ++++++++-
>  shared.c                           | 60 ++++++++++++++++++++++++++++++++++++
>  ui-blob.c                          |  6 ++--
>  ui-clone.c                         | 23 +++++++++++++-
>  ui-commit.c                        |  2 +-
>  ui-diff.c                          |  4 +--
>  ui-log.c                           |  2 +-
>  ui-patch.c                         |  4 +--
>  ui-plain.c                         |  2 +-
>  ui-refs.c                          |  6 ++--
>  ui-repolist.c                      |  3 +-
>  ui-shared.c                        |  8 ++---
>  ui-snapshot.c                      | 14 ++++-----
>  ui-tag.c                           |  2 +-
>  ui-tree.c                          |  2 +-
>  18 files changed, 232 insertions(+), 36 deletions(-)
>
> --
> 2.9.0
>
-------------- next part --------------
A non-text attachment was scrubbed...
Name: gentest.sh
Type: application/x-sh
Size: 3681 bytes
Desc: not available
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160716/47414ddd/attachment.sh>

[PATCH 00/16] [V2] Add namespace support to cgit

[richard.maw]

Hi.

We're wondering when you'd get time to look at this, as we're hoping
to make a Gitano release, and were hoping to be able to provide
support for using cgit to display namespaced repositories created with
Gitano.

If you're not likely to get around to it soon, then we'd appreciate
knowing, so we can prioritise in time for the next Debian freeze.

On 16 July 2016 at 00:10, Richard <richard.maw at gmail.com> wrote:
> Also, I wrote this script to generate an environment where it serves
> CGit and smart-http over the same server, since I thought it might be
> useful to illustrate a sample usage for this.
>
> The delay in resending after rebasing was that I went through 3
> different web servers before settling on learning how to configure
> apache.
>
> On 15 July 2016 at 23:59, Richard Maw <richard.maw at gmail.com> wrote:
>> This is a rebase of the previous patch series,
>> since it stopped applying and one of the commits was taken independently.
>>
>> The following changes since commit ff9893ac8192579a00dd4c73ddff18ab232099a6:
>>
>>   Fix qry.head leak on error (2016-07-12 01:06:04 +0200)
>>
>> are available in the git repository at:
>>
>>   git://git.gitano.org.uk/cgit.git
>>
>> for you to fetch changes up to 2a20425f4e76e378c138094c79b78aa476c019b5:
>>
>>   Add documentation for repo.agefile (2016-07-13 20:09:37 +0100)
>>
>> Richard Maw (16):
>>   Fix archive generation
>>   Add a wrapper for get_sha1 called cgit_get_sha1
>>   Parse repo.namespace
>>   Print out parsed namespace on request
>>   Set GIT_NAMESPACE when repo.namespace is provided
>>   Look up refs in namespace with cgit_get_sha1
>>   Guess the default branch based on current namespace
>>   Add cgit_for_each_namespaced_ref_in helper
>>   Find the default branch based on the contents of the namespace
>>   Only display refs in current namespace
>>   Add namespace support to dumb-clone
>>   Display notes from namespace
>>   Add documentation for repo.namespace
>>   Allow agefile to be set per-repository
>>   Update contrib script to update agefiles per namespace
>>   Add documentation for repo.agefile
>>
>>  cgit.c                             | 63 +++++++++++++++++++++++++++++++++-----
>>  cgit.h                             |  6 ++++
>>  cgitrc.5.txt                       | 46 ++++++++++++++++++++++++++++
>>  contrib/hooks/post-receive.agefile | 15 ++++++++-
>>  shared.c                           | 60 ++++++++++++++++++++++++++++++++++++
>>  ui-blob.c                          |  6 ++--
>>  ui-clone.c                         | 23 +++++++++++++-
>>  ui-commit.c                        |  2 +-
>>  ui-diff.c                          |  4 +--
>>  ui-log.c                           |  2 +-
>>  ui-patch.c                         |  4 +--
>>  ui-plain.c                         |  2 +-
>>  ui-refs.c                          |  6 ++--
>>  ui-repolist.c                      |  3 +-
>>  ui-shared.c                        |  8 ++---
>>  ui-snapshot.c                      | 14 ++++-----
>>  ui-tag.c                           |  2 +-
>>  ui-tree.c                          |  2 +-
>>  18 files changed, 232 insertions(+), 36 deletions(-)
>>
>> --
>> 2.9.0
>>

[PATCH 00/16] [V2] Add namespace support to cgit

[Jason]

Thanks for the reminder! I'll take a look tomorrow morning.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.zx2c4.com/pipermail/cgit/attachments/20160728/613dc591/attachment.html>

[PATCH 06/16] Look up refs in namespace with cgit_get_sha1

[Jason]

On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
> +static int namespaced_dwim_ref_get_sha1(const char *name, unsigned char *sha1)
> +{
> +       /* The standard git name disambiguation order is:
> +            $name
> +            refs/$name
> +            refs/tags/$name
> +            refs/heads/$name
> +            refs/remotes/$name (not sure why)
> +            refs/remotes/$name/HEAD
> +          we don't care about remotes, so we can skip those,
> +          and we can't specify a prefix for dwm_ref,
> +          so we have to do this ourselves */
> +       static const char *namespaced_ref_patterns[] = {
> +               "%s%s",
> +               "%srefs/%s",
> +               "%srefs/tags/%s",
> +               "%srefs/heads/%s",
> +               NULL,
> +       };
> +       const char **p;
> +
> +       for (p = namespaced_ref_patterns; *p; p++) {
> +               char *fullref = NULL;
> +               const char *r;
> +               fullref = mkpathdup(*p, get_git_namespace(), name);
> +               r = resolve_ref_unsafe(fullref, RESOLVE_REF_READING, sha1, NULL);
> +               free(fullref);
> +               if (r)
> +                       return 0;
> +       }
> +       return 1;
> +}
> +
>  int cgit_get_sha1(const char *name, unsigned char *sha1)
>  {
> -       return get_sha1(name, sha1);
> +       if (ctx.repo->namespace) {
> +               /* If we have a namespace, we can get either a sha1,
> +                  or a possibly abbreviated ref.
> +                  Advanced ref forms are not supported at this time
> +                  as this would require reimplementing all of ref parsing.
> +                  If get_sha1_with_context grows support for a namespaced flag
> +                  then this code may go away. */
> +               if (get_sha1_hex(name, sha1) == 0)
> +                       return 0;
> +               return namespaced_dwim_ref_get_sha1(name, sha1);

Ugh. This manual ref lookup is really ugly and unfortunate. I don't
like having to duplicate the code like this. What would you think of
adding the flag to upstream, so that the functions are namespace
aware, and then this becomes unnecessary?



> +       } else {
> +               return get_sha1(name, sha1);
> +       }


Get rid of the else clause, and just put "return get_sha1(name,
sha1);" unintended as the last line of the function.

[PATCH 07/16] Guess the default branch based on current namespace

[Jason]

On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
> -
> -       ref = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
> -       if (!ref || !starts_with(ref, "refs/heads/"))
> +       char *namespaced_head = NULL;
> +
> +       if (get_git_namespace())
> +               namespaced_head = mkpathdup("%sHEAD", get_git_namespace());
> +       /* NOTE: RESOLVE_REF_NO_RECURSE is required to prevent it resolving HEAD
> +          into a ref outside of the namespace. */
> +       ref = resolve_ref_unsafe(namespaced_head ?: "HEAD", RESOLVE_REF_NO_RECURSE, sha1, NULL);

Except now, for the non-namespaced case, we don't recurse on symbolic
entries, which might not be desirable.

Rather than this, would it be possible to make the underlying upstream
function namespace aware? By the environment variable perhaps?

> +       free(namespaced_head);
> +       if (!ref || !starts_with(strip_namespace(ref), "refs/heads/"))
>                 return "master";
> -       return xstrdup(ref + 11);
> +       return xstrdup(strip_namespace(ref) + 11);
>  }
>  /* The caller must free filename and ref after calling this. */
>  static inline void parse_readme(const char *readme, char **filename, char **ref, struct cgit_repo *repo)

[PATCH 11/16] Add namespace support to dumb-clone

[Jason]

On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
> +               unsigned char unused[20];
> +               char *namespaced_head = NULL;
> +               const char *ref;
> +               namespaced_head = mkpathdup("%sHEAD", get_git_namespace());
> +               /* NOTE: RESOLVE_REF_NO_RECURSE is required to prevent it resolving HEAD
> +                  into a ref outside of the namespace. */
> +               ref = resolve_ref_unsafe(namespaced_head, RESOLVE_REF_NO_RECURSE, unused, NULL);

This funny business is duplicated from a previous patch.

[PATCH 12/16] Display notes from namespace

[Jason]

On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
> +               if (ref_exists(namespaced_note_ref.buf)) {
> +                       if (has_glob_specials(namespaced_note_ref.buf)) {
> +                               for (const char *c = namespaced_note_ref.buf; *c; c++) {
> +                                       if (is_glob_special(*c))
> +                                               strbuf_addchars(&glob_escaped, '\\', 1);
> +                                       strbuf_addchars(&glob_escaped, *c, 1);

Could you explain why this is necessary? Why are the extra refs globs
exactly? Does git core have a convenience function for this?

> +                               }
> +                               string_list_append(&opt.extra_notes_refs, glob_escaped.buf);
> +                       } else {
> +                               string_list_append(&opt.extra_notes_refs, namespaced_note_ref.buf);
> +                       }

else
        single-statement;


> +               }
> +
> +               init_display_notes(&opt);
> +               string_list_clear(&opt.extra_notes_refs, 1);
> +               strbuf_release(&glob_escaped);
> +               strbuf_release(&namespaced_note_ref);
> +       } else {
> +               init_display_notes(NULL);
> +       }

else
        init_display_notes(NULL);

[PATCH 13/16] Add documentation for repo.namespace

[Jason]

On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
> Signed-off-by: Richard Maw <richard.maw at gmail.com>
> ---
>  cgitrc.5.txt | 32 ++++++++++++++++++++++++++++++++
>  1 file changed, 32 insertions(+)
>
> diff --git a/cgitrc.5.txt b/cgitrc.5.txt
> index 9fcf445..6116d1b 100644
> --- a/cgitrc.5.txt
> +++ b/cgitrc.5.txt
> @@ -570,6 +570,10 @@ repo.max-stats::
>  repo.name::
>         The value to show as repository name. Default value: <repo.url>.
>
> +repo.namespace::
> +       Set the git namespace, so that fetching and cloning only provide refs
> +        under refs/namespaces. Default value: none.

Mixed tabs and spaces it appears.

This does more than just fetching and cloning, right? It's supposed to
affect the whole displayed view of cgit?

[PATCH 15/16] Update contrib script to update agefiles per namespace

[Jason]

On Sat, Jul 16, 2016 at 1:00 AM, Richard Maw <richard.maw at gmail.com> wrote:
> -agefile="$(git rev-parse --git-dir)"/info/web/last-modified
> +agefile="$(git rev-parse --git-dir)"/info/web
> +if [ -z "$GIT_NAMESPACE" ]; then
> +       agefile="$agefile/last-modified"
> +else
> +       agefile="$agefile/namespaced/$GIT_NAMESPACE/last-modified"
> +fi

namespaced --> namespaces?

[PATCH 07/16] Guess the default branch based on current namespace

[richard.maw]

On 29 July 2016 at 15:36, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
>> -
>> -       ref = resolve_ref_unsafe("HEAD", 0, sha1, NULL);
>> -       if (!ref || !starts_with(ref, "refs/heads/"))
>> +       char *namespaced_head = NULL;
>> +
>> +       if (get_git_namespace())
>> +               namespaced_head = mkpathdup("%sHEAD", get_git_namespace());
>> +       /* NOTE: RESOLVE_REF_NO_RECURSE is required to prevent it resolving HEAD
>> +          into a ref outside of the namespace. */
>> +       ref = resolve_ref_unsafe(namespaced_head ?: "HEAD", RESOLVE_REF_NO_RECURSE, sha1, NULL);
>
> Except now, for the non-namespaced case, we don't recurse on symbolic
> entries, which might not be desirable.

I've never seen anyone use doubly indirect symbolic refs for HEAD before,
but I can't say that doesn't happen.

I'll conditionalise the flag if I can't make resolve_ref_unsafe namespace aware.

> Rather than this, would it be possible to make the underlying upstream
> function namespace aware? By the environment variable perhaps?

I'm not sure how well git would cope with resolve_ref_unsafe being
suddenly namespace aware,
so I'd probably gate it behind a flag as well.

[PATCH 12/16] Display notes from namespace

[richard.maw]

On 29 July 2016 at 15:44, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
>> +               if (ref_exists(namespaced_note_ref.buf)) {
>> +                       if (has_glob_specials(namespaced_note_ref.buf)) {
>> +                               for (const char *c = namespaced_note_ref.buf; *c; c++) {
>> +                                       if (is_glob_special(*c))
>> +                                               strbuf_addchars(&glob_escaped, '\\', 1);
>> +                                       strbuf_addchars(&glob_escaped, *c, 1);
>
> Could you explain why this is necessary?

The git note loading logic isn't namespace aware,
so rather than reimplementing it all
this just adds the default note file in the current namespace.

> Why are the extra refs globs exactly?

I'm not sure.

I thought it was just that its intended user was glob expressions
provided by the command line or config,
and that it was just a leaky abstraction,
but it looks like there might be validation code preventing refs from
including glob characters.

I'll take a look at the git code to see if I can prove this hypothesis.

> Does git core have a convenience function for this?

Not that I could find, but if my hypothesis proves correct it can go
away entirely.

[PATCH 13/16] Add documentation for repo.namespace

[richard.maw]

On 29 July 2016 at 15:48, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
>> +repo.namespace::
>> +       Set the git namespace, so that fetching and cloning only provide refs
>> +        under refs/namespaces. Default value: none.
>
> Mixed tabs and spaces it appears.

Bah! Sorry, not sure how that crept in.

> This does more than just fetching and cloning, right? It's supposed to
> affect the whole displayed view of cgit?

Yeah. I can't remember if I was intentionally being over-brief there.
I'll list everything that the setting changes.

[PATCH 15/16] Update contrib script to update agefiles per namespace

[richard.maw]

On 29 July 2016 at 15:51, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
> On Sat, Jul 16, 2016 at 1:00 AM, Richard Maw <richard.maw at gmail.com> wrote:
>> +       agefile="$agefile/namespaced/$GIT_NAMESPACE/last-modified"
>> +fi
>
> namespaced --> namespaces?

Cheers, I'm bad at names and "namespaced" was the best I could think of,
but "namespaces" provides a nice symmetry with how the refs are organised.

[PATCH 06/16] Look up refs in namespace with cgit_get_sha1

[richard.maw]

Adding CC to the list, since I forgot it, and have more to add.

On 29 July 2016 at 16:34, Richard <richard.maw at gmail.com> wrote:
> Apologies in advance if Gmail mangles this, I only have access to
> webmail at the moment.
>
> On 29 July 2016 at 15:32, Jason A. Donenfeld <Jason at zx2c4.com> wrote:
>> On Sat, Jul 16, 2016 at 12:59 AM, Richard Maw <richard.maw at gmail.com> wrote:
>>> +static int namespaced_dwim_ref_get_sha1(const char *name, unsigned char *sha1)
> <snip>
>>> +               return namespaced_dwim_ref_get_sha1(name, sha1);
>>
>> Ugh. This manual ref lookup is really ugly and unfortunate. I don't
>> like having to duplicate the code like this. What would you think of
>> adding the flag to upstream, so that the functions are namespace
>> aware, and then this becomes unnecessary?
>
> I'll have a go. I don't know how successful adding a feature for a
> different project would be.

We're hoping to have this in the next Debian release.
Git releases look to have a few months between them,
so if it takes another few months for a Git release then it could miss
the deadline.

After I've worked out how to add the support to libgit I'll know better,
but it might be a good idea to:

1.  Make a patch to add support to libgit
2.  Port that patch into cgit,
    so when the libgit patch is submitted I can point to cgit
    to say that we're using cgit as a proving ground for the change.
3.  Submit namespace support patch to libgit.
4.  When there's a release with namespace support,
    update to it and remove the patch from cgit.

I'm not sure whether it would be better to transplant that part of
libgit into cgit,
or to patch libgit at build time.

>>> +       } else {
>>> +               return get_sha1(name, sha1);
>>> +       }
>>
>>
>> Get rid of the else clause, and just put "return get_sha1(name,
>> sha1);" unintended as the last line of the function.
>
> Sure, I failed to spot the coding style you preferred, I don't
> remember seeing it documented anywhere.

[PATCH 06/16] Look up refs in namespace with cgit_get_sha1

[richard.maw]

On 29 July 2016 at 17:54, Richard <richard.maw at gmail.com> wrote:
> On 29 July 2016 at 16:34, Richard <richard.maw at gmail.com> wrote:
>> I'll have a go. I don't know how successful adding a feature for a
>> different project would be.
>
> After I've worked out how to add the support to libgit I'll know better,
> but it might be a good idea to:
>
> 1.  Make a patch to add support to libgit
> 2.  Port that patch into cgit,
>     so when the libgit patch is submitted I can point to cgit
>     to say that we're using cgit as a proving ground for the change.
> 3.  Submit namespace support patch to libgit.
> 4.  When there's a release with namespace support,
>     update to it and remove the patch from cgit.
>
> I'm not sure whether it would be better to transplant that part of
> libgit into cgit,
> or to patch libgit at build time.

At a first approximation, the change needed is to make everything that
interacts with the files backend have a namespace aware variant,
then make these variants the ones that get called when we are using a namespace.

There is prior-art for this in the initial namespaces implementation,
which only added a for_each_ref equivalent,
which calls do_for_each_ref with a namespaced ref prefix.

The first issue with this is that the full set of operations is:

pack_refs,
resolve_gitlink_ref, read_raw_ref, peel_ref, verify_refname_available,
do_for_each_ref
rename_ref, delete_refs, ref_transaction_commit, initial_ref_transaction_commit,
create_symref, set_worktree_head_symref,
reflog_exists, for_each_reflog_ent_reverse, for_each_reflog_ent,
for_each_reflog,
safe_create_reflog, files_log_ref_write, delete_reflog and reflog_expire.

pack_refs is the only one of those functions that wouldn't need to be
made namespace aware.
So to start with there's approximately 90 call-sites that would need
to be made namespace aware.

We can cull that further by only considering the interfaces for reading refs:

resolve_gitlink_ref, read_raw_ref, peel_ref, verify_refname_available,
do_for_each_ref
reflog_exists, for_each_reflog_ent_reverse, for_each_reflog_ent,
for_each_reflog,

This makes it about 50 call sites.

We would then need to work backwards from these functions to the APIs
we want to call (which is likely to be a significant fraction of
libgit) and add a way to thread through the call chains that we want
to opt into using the current namespace.

We would need to make some new command-line programs namespace aware,
otherwise git upstream would have no reason to accept the patches,
and we'd need to thread the flag for using the current namespace all
the way to the command-line,
rather than just detecting whether GIT_NAMESPACE was set,
since hook scripts are called with GIT_NAMESPACE set if it was set for
git-http-backend.

The age file script that was updated in this patch series would stop working
if git for-each-ref suddenly started to only handle refs in
GIT_NAMESPACE without an opt-in.


If we were to instead drop support for short branch names in the CGit API,
or continue to use our simpler dwim_ref implementation,
the only function that needs to be made namespace aware is resolve_ref_unsafe,
which we are currently handling by wrapping it with something that
translates the ref names back and forth.


In summary, I failed to find a nice way to add namespace support,
looking both top-down from the API and bottom-up from how it reads refs.
It would be a huge undertaking which reduces the odds of it being
accepted upstream,
but in doing this analysis I'm more sure that the work that's already
been submitted to cgit
is the smallest change necessary to support namespaced ref lookups.

Therefore I suggest it is best to stick with the current approach,
and the next version I submit would mark which functions should be revisited
if git gained greater support for namespaced ref resolution.
If this is a sticking point then I will attempt to make the changes in libgit,
but am uncertain whether it would be successful.