[PATCH v2] rev-parse --namespace

[PATCH v2] rev-parse --namespace

[Ilari Liusvaara]

Add --namespace=<namespace> option to rev-parse and everything that
accepts its options. This option matches all refs in some subnamespace
of refs hierarchy.

Example:

'git log --branches --not --namespace=remotes/origin'

To show what you have that origin doesn't.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
Changes from v1:
- Get rid of global variable for namespace.
- Use strbuf API to build the namespace
- Test rev-list/rev-parse outputs up to permutation, not just line
  count.
- Add example to commit message and to git log manual page.

I left renaming the option (more opinions on that?) and changing the
description in manpage (the other similar entries should be fixed too
then) for later iteration (if these are to be done).

 Documentation/git-rev-list.txt     |    1 +
 Documentation/git-rev-parse.txt    |    4 +
 Documentation/rev-list-options.txt |    6 ++
 builtin-rev-parse.c                |    8 +++
 refs.c                             |   18 ++++++
 refs.h                             |   13 ++++
 revision.c                         |   20 ++++++-
 t/t6018-rev-list-namespace.sh      |  112 ++++++++++++++++++++++++++++++++++++
 8 files changed, 180 insertions(+), 2 deletions(-)
 create mode 100755 t/t6018-rev-list-namespace.sh

diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 3341d1b..a8f8f22 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 	     [ \--branches ]
 	     [ \--tags ]
 	     [ \--remotes ]
+	     [ \--namespace=namespace-prefix ]
 	     [ \--stdin ]
 	     [ \--quiet ]
 	     [ \--topo-order ]
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 82045a2..af4605a 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -112,6 +112,10 @@ OPTIONS
 --remotes::
 	Show tag refs found in `$GIT_DIR/refs/remotes`.
 
+--namespace=namespace-prefix::
+	Show refs found in `$GIT_DIR/namespace-prefix`. If namespace
+	specified lacks leading 'refs/', it is automatically prepended.
+
 --show-prefix::
 	When the command is invoked from a subdirectory, show the
 	path of the current directory relative to the top-level
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1f57aed..c824a7b 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -243,6 +243,12 @@ endif::git-rev-list[]
 	Pretend as if all the refs in `$GIT_DIR/refs/remotes` are listed
 	on the command line as '<commit>'.
 
+--namespace=namespace-prefix::
+	Pretend as if all the refs in `$GIT_DIR/namespace-prefix` are
+	listed on the command line as '<commit>'. Leading 'refs/', it
+	is automatically prepended if missing.
+
+
 ifndef::git-rev-list[]
 --bisect::
 
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 37d0233..255191d 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -52,6 +52,7 @@ static int is_rev_argument(const char *arg)
 		"--parents",
 		"--pretty",
 		"--remotes",
+		"--namespace=",
 		"--sparse",
 		"--tags",
 		"--topo-order",
@@ -577,6 +578,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				for_each_tag_ref(show_reference, NULL);
 				continue;
 			}
+			if (!prefixcmp(arg, "--namespace=")) {
+				struct for_each_namespace_request req;
+				req.req_namespace = arg + 12;
+				req.req_opaque = NULL;
+				for_each_namespace_ref(show_reference, &req);
+				continue;
+			}
 			if (!strcmp(arg, "--remotes")) {
 				for_each_remote_ref(show_reference, NULL);
 				continue;
diff --git a/refs.c b/refs.c
index 3e73a0a..c7162d1 100644
--- a/refs.c
+++ b/refs.c
@@ -7,6 +7,7 @@
 /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
 #define REF_KNOWS_PEELED 04
 
+/* Current prefix namespace in use. NULL means none. */
 struct ref_list {
 	struct ref_list *next;
 	unsigned char flag; /* ISSYMREF? ISPACKED? */
@@ -674,6 +675,23 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
 }
 
+int for_each_namespace_ref(each_ref_fn fn, void *cb_data)
+{
+	struct for_each_namespace_request *req = cb_data;
+	struct strbuf real_prefix = STRBUF_INIT;
+	int ret;
+
+	if (prefixcmp(req->req_namespace, "refs/"))
+		strbuf_addstr(&real_prefix, "refs/");
+	strbuf_addstr(&real_prefix, req->req_namespace);
+	if (real_prefix.buf[real_prefix.len - 1] != '/')
+		strbuf_addch(&real_prefix, '/');
+
+	ret = for_each_ref_in(real_prefix.buf, fn, req->req_opaque);
+	strbuf_release(&real_prefix);
+	return ret;
+}
+
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref("refs/", fn, 0,
diff --git a/refs.h b/refs.h
index e141991..4bb63b0 100644
--- a/refs.h
+++ b/refs.h
@@ -13,6 +13,18 @@ struct ref_lock {
 #define REF_ISSYMREF 01
 #define REF_ISPACKED 02
 
+/* Opaque request for for_each_namespace_ref */
+struct for_each_namespace_request
+{
+	/*
+	 * The limiting namespace. 'refs/' and '/' are autoprepended /
+	 * autoappended if missing.
+	 */
+	const char *req_namespace;
+	/* The real opaque data for callback function. */
+	void *req_opaque;
+};
+
 /*
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
@@ -25,6 +37,7 @@ extern int for_each_tag_ref(each_ref_fn, void *);
 extern int for_each_branch_ref(each_ref_fn, void *);
 extern int for_each_remote_ref(each_ref_fn, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_namespace_ref(each_ref_fn, void *);
 
 /* can be used to learn about broken ref and symref */
 extern int for_each_rawref(each_ref_fn, void *);
diff --git a/revision.c b/revision.c
index 25fa14d..ff9484f 100644
--- a/revision.c
+++ b/revision.c
@@ -699,12 +699,18 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	return 0;
 }
 
+static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
+	unsigned flags)
+{
+	cb->all_revs = revs;
+	cb->all_flags = flags;
+}
+
 static void handle_refs(struct rev_info *revs, unsigned flags,
 		int (*for_each)(each_ref_fn, void *))
 {
 	struct all_refs_cb cb;
-	cb.all_revs = revs;
-	cb.all_flags = flags;
+	init_all_refs_cb(&cb, revs, flags);
 	for_each(handle_one_ref, &cb);
 }
 
@@ -1352,6 +1358,16 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				handle_refs(revs, flags, for_each_remote_ref);
 				continue;
 			}
+			if (!prefixcmp(arg, "--namespace=")) {
+				struct for_each_namespace_request req;
+				struct all_refs_cb cb;
+
+				init_all_refs_cb(&cb, revs, flags);
+				req.req_namespace = arg + 12;
+				req.req_opaque = &cb;
+				for_each_namespace_ref(handle_one_ref, &req);
+				continue;
+			}
 			if (!strcmp(arg, "--reflog")) {
 				handle_reflog(revs, flags);
 				continue;
diff --git a/t/t6018-rev-list-namespace.sh b/t/t6018-rev-list-namespace.sh
new file mode 100755
index 0000000..6bb562a
--- /dev/null
+++ b/t/t6018-rev-list-namespace.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+
+test_description='rev-list/rev-parse --namespace'
+
+. ./test-lib.sh
+
+
+commit () {
+	test_tick &&
+	echo $1 > foo &&
+	git add foo &&
+	git commit -m "$1"
+}
+
+compare () {
+	# Split arguments on whitespace.
+	git $1 $2 | sort >expected &&
+	git $1 $3 | sort >actual &&
+	cmp expected actual
+}
+
+test_expect_success 'setup' '
+
+	commit master &&
+	git checkout -b subspace/one master
+	commit one &&
+	git checkout -b subspace/two master
+	commit two &&
+	git checkout -b subspace-x master
+	commit subspace-x &&
+	git checkout -b other/three master
+	commit three &&
+	git checkout -b someref master
+	commit some &&
+	git checkout master &&
+	commit master2
+'
+
+test_expect_success 'rev-parse --namespace=refs/heads/subspace/' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=refs/heads/subspace/"
+
+'
+
+test_expect_success 'rev-parse --namespace=refs/heads/subspace' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=refs/heads/subspace"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/subspace' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=heads/subspace"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/subspace --namespace=heads/other' '
+
+	compare rev-parse "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/someref master' '
+
+	compare rev-parse "master" "--namespace=heads/someref master"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads' '
+
+	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+
+'
+
+test_expect_success 'rev-list --namespace=refs/heads/subspace/' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=refs/heads/subspace/"
+
+'
+
+test_expect_success 'rev-list --namespace=refs/heads/subspace' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=refs/heads/subspace"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/subspace' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=heads/subspace"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/someref master' '
+
+	compare rev-parse "master" "--namespace=heads/someref master"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/subspace --namespace=heads/other' '
+
+	compare rev-parse "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
+
+'
+
+test_expect_success 'rev-list --namespace=heads' '
+
+	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+
+'
+
+
+test_done
-- 
1.6.6.199.gff4b0

Re: [PATCH v2] rev-parse --namespace

[Junio C Hamano]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> Add --namespace=<namespace> option to rev-parse and everything that
> accepts its options. This option matches all refs in some subnamespace
> of refs hierarchy.
>
> Example:
>
> 'git log --branches --not --namespace=remotes/origin'
>
> To show what you have that origin doesn't.
>
> Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>

Thanks.

> @@ -577,6 +578,13 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
>  				for_each_tag_ref(show_reference, NULL);
>  				continue;
>  			}
> +			if (!prefixcmp(arg, "--namespace=")) {
> +				struct for_each_namespace_request req;
> +				req.req_namespace = arg + 12;
> +				req.req_opaque = NULL;

If the structure variable is often named after "request", there is not
much point naming its fields (which are most likely all related to
request) with "req_" prefix.  IOW, wouldn't this be much easier to read?

	struct for_each_namespace_request req;
	memset(&req, 0, sizeof(req));
        req.namespace = arg + 12;

But please don't worry about this part too much, as I'll suggest getting
rid of the struct altogether shortly (read on).

> diff --git a/refs.c b/refs.c
> index 3e73a0a..c7162d1 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -7,6 +7,7 @@
>  /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
>  #define REF_KNOWS_PEELED 04
>  
> +/* Current prefix namespace in use. NULL means none. */
>  struct ref_list {
>  	struct ref_list *next;
>  	unsigned char flag; /* ISSYMREF? ISPACKED? */

Is the above new comment really about what this structure type is?

> diff --git a/refs.h b/refs.h
> index e141991..4bb63b0 100644
> --- a/refs.h
> +++ b/refs.h
> @@ -25,6 +37,7 @@ extern int for_each_tag_ref(each_ref_fn, void *);
>  extern int for_each_branch_ref(each_ref_fn, void *);
>  extern int for_each_remote_ref(each_ref_fn, void *);
>  extern int for_each_replace_ref(each_ref_fn, void *);
> +extern int for_each_namespace_ref(each_ref_fn, void *);

I know somebody in the review discussion thought that the second parameter
for this should be void * for whatever reason, but I don't see the point.

Why cannot this be

	for_each_namespace_ref(each_ref_fn, const char *, void *)

where the second one specifies the "namespace prefix"?

The caller of for_each_namespace_ref() with v2 interface needs to be aware
of that for_each_namespace_request structure anyway, so there is nothing
"void *" about it.  You can have a function pointer that can point at
for_each_{branch,remote,replace}_ref and indirectly call one of the
functions via that pointer to grab different set of refs, but this new
function for_each_namespace_ref cannot play together with them, as its
cb_data must be of different shape for the request to make sense; having
the same function signature as others wouldn't help.

If the callback "each_ref_fn" function learns what ns-prefix was given for
it to be called (iow, it gets the whole for_each_namespace_request,
instead of just its req_opaque member), then I might be talked into
believing that it _could_ be useful in some situations, but the way the
callback is called is just the usual "this ref points at this object name,
by the way here is the message my caller told me to give it to you".  And
if the caller really wanted to tell the namespace prefix to the callback
function, it can be made part of the application specific cbdata.

So in short, I simply don't see the point of using this new "request"
structure.  Here is a suggested update that can be squashed on top of v2.

Am I missing some subtle issues?

 builtin-rev-parse.c |    5 +----
 refs.c              |   10 ++++------
 refs.h              |   14 +-------------
 revision.c          |    5 +----
 4 files changed, 7 insertions(+), 27 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 255191d..34af347 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -579,10 +579,7 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				continue;
 			}
 			if (!prefixcmp(arg, "--namespace=")) {
-				struct for_each_namespace_request req;
-				req.req_namespace = arg + 12;
-				req.req_opaque = NULL;
-				for_each_namespace_ref(show_reference, &req);
+				for_each_namespace_ref(show_reference, arg + 12, NULL);
 				continue;
 			}
 			if (!strcmp(arg, "--remotes")) {
diff --git a/refs.c b/refs.c
index c7162d1..2bf8755 100644
--- a/refs.c
+++ b/refs.c
@@ -7,7 +7,6 @@
 /* ISSYMREF=01 and ISPACKED=02 are public interfaces */
 #define REF_KNOWS_PEELED 04
 
-/* Current prefix namespace in use. NULL means none. */
 struct ref_list {
 	struct ref_list *next;
 	unsigned char flag; /* ISSYMREF? ISPACKED? */
@@ -675,19 +674,18 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
 }
 
-int for_each_namespace_ref(each_ref_fn fn, void *cb_data)
+int for_each_namespace_ref(each_ref_fn fn, const char *ns_prefix, void *cb_data)
 {
-	struct for_each_namespace_request *req = cb_data;
 	struct strbuf real_prefix = STRBUF_INIT;
 	int ret;
 
-	if (prefixcmp(req->req_namespace, "refs/"))
+	if (prefixcmp(ns_prefix, "refs/"))
 		strbuf_addstr(&real_prefix, "refs/");
-	strbuf_addstr(&real_prefix, req->req_namespace);
+	strbuf_addstr(&real_prefix, ns_prefix);
 	if (real_prefix.buf[real_prefix.len - 1] != '/')
 		strbuf_addch(&real_prefix, '/');
 
-	ret = for_each_ref_in(real_prefix.buf, fn, req->req_opaque);
+	ret = for_each_ref_in(real_prefix.buf, fn, cb_data);
 	strbuf_release(&real_prefix);
 	return ret;
 }
diff --git a/refs.h b/refs.h
index 4bb63b0..a1d7378 100644
--- a/refs.h
+++ b/refs.h
@@ -13,18 +13,6 @@ struct ref_lock {
 #define REF_ISSYMREF 01
 #define REF_ISPACKED 02
 
-/* Opaque request for for_each_namespace_ref */
-struct for_each_namespace_request
-{
-	/*
-	 * The limiting namespace. 'refs/' and '/' are autoprepended /
-	 * autoappended if missing.
-	 */
-	const char *req_namespace;
-	/* The real opaque data for callback function. */
-	void *req_opaque;
-};
-
 /*
  * Calls the specified function for each ref file until it returns nonzero,
  * and returns the value
@@ -37,7 +25,7 @@ extern int for_each_tag_ref(each_ref_fn, void *);
 extern int for_each_branch_ref(each_ref_fn, void *);
 extern int for_each_remote_ref(each_ref_fn, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
-extern int for_each_namespace_ref(each_ref_fn, void *);
+extern int for_each_namespace_ref(each_ref_fn, const char *, void *);
 
 /* can be used to learn about broken ref and symref */
 extern int for_each_rawref(each_ref_fn, void *);
diff --git a/revision.c b/revision.c
index ff9484f..ec63fa0 100644
--- a/revision.c
+++ b/revision.c
@@ -1359,13 +1359,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				continue;
 			}
 			if (!prefixcmp(arg, "--namespace=")) {
-				struct for_each_namespace_request req;
 				struct all_refs_cb cb;
 
 				init_all_refs_cb(&cb, revs, flags);
-				req.req_namespace = arg + 12;
-				req.req_opaque = &cb;
-				for_each_namespace_ref(handle_one_ref, &req);
+				for_each_namespace_ref(handle_one_ref, arg + 12, &cb);
 				continue;
 			}
 			if (!strcmp(arg, "--reflog")) {

[PATCH v3] rev-parse --namespace

[Ilari Liusvaara]

Add --namespace=<namespace> option to rev-parse and everything that
accepts its options. This option matches all refs in some subnamespace
of refs hierarchy.

Example:

'git log --branches --not --namespace=remotes/origin'

To show what you have that origin doesn't.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
Changes from v2:
- Actually include updated git log examples (oops).
- Get rid of for_each_namespace_request, use parameters instead.
- Get rid of extraneous comment.

 Documentation/git-log.txt          |    6 ++
 Documentation/git-rev-list.txt     |    1 +
 Documentation/git-rev-parse.txt    |    4 +
 Documentation/rev-list-options.txt |    6 ++
 builtin-rev-parse.c                |    5 ++
 refs.c                             |   16 +++++
 refs.h                             |    1 +
 revision.c                         |   16 +++++-
 t/t6018-rev-list-namespace.sh      |  112 ++++++++++++++++++++++++++++++++++++
 9 files changed, 165 insertions(+), 2 deletions(-)
 create mode 100755 t/t6018-rev-list-namespace.sh

diff --git a/Documentation/git-log.txt b/Documentation/git-log.txt
index 3d79de1..d84660c 100644
--- a/Documentation/git-log.txt
+++ b/Documentation/git-log.txt
@@ -107,6 +107,12 @@ git log --follow builtin-rev-list.c::
 	those commits that occurred before the file was given its
 	present name.
 
+git log --branches --not --namespace=remotes/origin::
+
+	Shows all commits that are in any of local branches but not in
+	any of remote tracking branches for 'origin' (what you have that
+	origin doesn't).
+
 Discussion
 ----------
 
diff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt
index 3341d1b..a8f8f22 100644
--- a/Documentation/git-rev-list.txt
+++ b/Documentation/git-rev-list.txt
@@ -24,6 +24,7 @@ SYNOPSIS
 	     [ \--branches ]
 	     [ \--tags ]
 	     [ \--remotes ]
+	     [ \--namespace=namespace-prefix ]
 	     [ \--stdin ]
 	     [ \--quiet ]
 	     [ \--topo-order ]
diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt
index 82045a2..af4605a 100644
--- a/Documentation/git-rev-parse.txt
+++ b/Documentation/git-rev-parse.txt
@@ -112,6 +112,10 @@ OPTIONS
 --remotes::
 	Show tag refs found in `$GIT_DIR/refs/remotes`.
 
+--namespace=namespace-prefix::
+	Show refs found in `$GIT_DIR/namespace-prefix`. If namespace
+	specified lacks leading 'refs/', it is automatically prepended.
+
 --show-prefix::
 	When the command is invoked from a subdirectory, show the
 	path of the current directory relative to the top-level
diff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt
index 1f57aed..c824a7b 100644
--- a/Documentation/rev-list-options.txt
+++ b/Documentation/rev-list-options.txt
@@ -243,6 +243,12 @@ endif::git-rev-list[]
 	Pretend as if all the refs in `$GIT_DIR/refs/remotes` are listed
 	on the command line as '<commit>'.
 
+--namespace=namespace-prefix::
+	Pretend as if all the refs in `$GIT_DIR/namespace-prefix` are
+	listed on the command line as '<commit>'. Leading 'refs/', it
+	is automatically prepended if missing.
+
+
 ifndef::git-rev-list[]
 --bisect::
 
diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 37d0233..34af347 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -52,6 +52,7 @@ static int is_rev_argument(const char *arg)
 		"--parents",
 		"--pretty",
 		"--remotes",
+		"--namespace=",
 		"--sparse",
 		"--tags",
 		"--topo-order",
@@ -577,6 +578,10 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)
 				for_each_tag_ref(show_reference, NULL);
 				continue;
 			}
+			if (!prefixcmp(arg, "--namespace=")) {
+				for_each_namespace_ref(show_reference, arg + 12, NULL);
+				continue;
+			}
 			if (!strcmp(arg, "--remotes")) {
 				for_each_remote_ref(show_reference, NULL);
 				continue;
diff --git a/refs.c b/refs.c
index 3e73a0a..5583f4b 100644
--- a/refs.c
+++ b/refs.c
@@ -674,6 +674,22 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
 }
 
+int for_each_namespace_ref(each_ref_fn fn, const char *ns_name, void *cb_data)
+{
+	struct strbuf real_prefix = STRBUF_INIT;
+	int ret;
+
+	if (prefixcmp(ns_name, "refs/"))
+		strbuf_addstr(&real_prefix, "refs/");
+	strbuf_addstr(&real_prefix, ns_name);
+	if (real_prefix.buf[real_prefix.len - 1] != '/')
+		strbuf_addch(&real_prefix, '/');
+
+	ret = for_each_ref_in(real_prefix.buf, fn, cb_data);
+	strbuf_release(&real_prefix);
+	return ret;
+}
+
 int for_each_rawref(each_ref_fn fn, void *cb_data)
 {
 	return do_for_each_ref("refs/", fn, 0,
diff --git a/refs.h b/refs.h
index e141991..b26c3a8 100644
--- a/refs.h
+++ b/refs.h
@@ -25,6 +25,7 @@ extern int for_each_tag_ref(each_ref_fn, void *);
 extern int for_each_branch_ref(each_ref_fn, void *);
 extern int for_each_remote_ref(each_ref_fn, void *);
 extern int for_each_replace_ref(each_ref_fn, void *);
+extern int for_each_namespace_ref(each_ref_fn, const char* ns_name, void *);
 
 /* can be used to learn about broken ref and symref */
 extern int for_each_rawref(each_ref_fn, void *);
diff --git a/revision.c b/revision.c
index 25fa14d..7328201 100644
--- a/revision.c
+++ b/revision.c
@@ -699,12 +699,18 @@ static int handle_one_ref(const char *path, const unsigned char *sha1, int flag,
 	return 0;
 }
 
+static void init_all_refs_cb(struct all_refs_cb *cb, struct rev_info *revs,
+	unsigned flags)
+{
+	cb->all_revs = revs;
+	cb->all_flags = flags;
+}
+
 static void handle_refs(struct rev_info *revs, unsigned flags,
 		int (*for_each)(each_ref_fn, void *))
 {
 	struct all_refs_cb cb;
-	cb.all_revs = revs;
-	cb.all_flags = flags;
+	init_all_refs_cb(&cb, revs, flags);
 	for_each(handle_one_ref, &cb);
 }
 
@@ -1352,6 +1358,12 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				handle_refs(revs, flags, for_each_remote_ref);
 				continue;
 			}
+			if (!prefixcmp(arg, "--namespace=")) {
+				struct all_refs_cb cb;
+				init_all_refs_cb(&cb, revs, flags);
+				for_each_namespace_ref(handle_one_ref, arg + 12, &cb);
+				continue;
+			}
 			if (!strcmp(arg, "--reflog")) {
 				handle_reflog(revs, flags);
 				continue;
diff --git a/t/t6018-rev-list-namespace.sh b/t/t6018-rev-list-namespace.sh
new file mode 100755
index 0000000..6bb562a
--- /dev/null
+++ b/t/t6018-rev-list-namespace.sh
@@ -0,0 +1,112 @@
+#!/bin/sh
+
+test_description='rev-list/rev-parse --namespace'
+
+. ./test-lib.sh
+
+
+commit () {
+	test_tick &&
+	echo $1 > foo &&
+	git add foo &&
+	git commit -m "$1"
+}
+
+compare () {
+	# Split arguments on whitespace.
+	git $1 $2 | sort >expected &&
+	git $1 $3 | sort >actual &&
+	cmp expected actual
+}
+
+test_expect_success 'setup' '
+
+	commit master &&
+	git checkout -b subspace/one master
+	commit one &&
+	git checkout -b subspace/two master
+	commit two &&
+	git checkout -b subspace-x master
+	commit subspace-x &&
+	git checkout -b other/three master
+	commit three &&
+	git checkout -b someref master
+	commit some &&
+	git checkout master &&
+	commit master2
+'
+
+test_expect_success 'rev-parse --namespace=refs/heads/subspace/' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=refs/heads/subspace/"
+
+'
+
+test_expect_success 'rev-parse --namespace=refs/heads/subspace' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=refs/heads/subspace"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/subspace' '
+
+	compare rev-parse "subspace/one subspace/two" "--namespace=heads/subspace"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/subspace --namespace=heads/other' '
+
+	compare rev-parse "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads/someref master' '
+
+	compare rev-parse "master" "--namespace=heads/someref master"
+
+'
+
+test_expect_success 'rev-parse --namespace=heads' '
+
+	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+
+'
+
+test_expect_success 'rev-list --namespace=refs/heads/subspace/' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=refs/heads/subspace/"
+
+'
+
+test_expect_success 'rev-list --namespace=refs/heads/subspace' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=refs/heads/subspace"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/subspace' '
+
+	compare rev-list "subspace/one subspace/two" "--namespace=heads/subspace"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/someref master' '
+
+	compare rev-parse "master" "--namespace=heads/someref master"
+
+'
+
+test_expect_success 'rev-list --namespace=heads/subspace --namespace=heads/other' '
+
+	compare rev-parse "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
+
+'
+
+test_expect_success 'rev-list --namespace=heads' '
+
+	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+
+'
+
+
+test_done
-- 
1.6.6.199.gff4b0

Re: [PATCH v3] rev-parse --namespace

[Jeff King]

On Mon, Jan 18, 2010 at 11:51:36AM +0200, Ilari Liusvaara wrote:

> Add --namespace=<namespace> option to rev-parse and everything that
> accepts its options. This option matches all refs in some subnamespace
> of refs hierarchy.

Thanks, I think the code in this version looks good, but:

> --- /dev/null
> +++ b/t/t6018-rev-list-namespace.sh
> [...]
> +compare () {
> +	# Split arguments on whitespace.
> +	git $1 $2 | sort >expected &&
> +	git $1 $3 | sort >actual &&
> +	cmp expected actual
> +}

Please use test_cmp instead of regular cmp.

Also, do you really need to sort? The internal ref code keeps the ref
lists sorted by name, and we merge-sort the packed and loose lists in
do_for_each_ref. So your --namespace should always operate on the refs
in sorted order, producing predictable results.

> +test_expect_success 'setup' '
> +
> +	commit master &&
> +	git checkout -b subspace/one master
> +	commit one &&
> +	git checkout -b subspace/two master
> +	commit two &&
> +	git checkout -b subspace-x master
> +	commit subspace-x &&
> +	git checkout -b other/three master
> +	commit three &&
> +	git checkout -b someref master
> +	commit some &&
> +	git checkout master &&
> +	commit master2
> +'

You are still missing some '&&' here to detect setup failures.

-Peff

[PATCH] Fix errors in t6018

[Ilari Liusvaara]

Fix errors and shortcomings in t6018 (the rev-parse --namespace test):
1) Some tests were missing &&'s, leading too possibly missing failures
2) Some tests used incorrect commands (didn't test what they were
suppoed to)
3) Some test used 'cmp' instead of 'test_cmp'
4) Tests only tested output up to permutation

Fix these errors in t6018.

Signed-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>
---
This can be appiled on top of rev-parse --namespace patch v3 or this
can be squashed to that patch.

 t/t6018-rev-list-namespace.sh |   24 ++++++++++++------------
 1 files changed, 12 insertions(+), 12 deletions(-)

diff --git a/t/t6018-rev-list-namespace.sh b/t/t6018-rev-list-namespace.sh
index 6bb562a..70c8a83 100755
--- a/t/t6018-rev-list-namespace.sh
+++ b/t/t6018-rev-list-namespace.sh
@@ -14,23 +14,23 @@ commit () {
 
 compare () {
 	# Split arguments on whitespace.
-	git $1 $2 | sort >expected &&
-	git $1 $3 | sort >actual &&
-	cmp expected actual
+	git $1 $2 >expected &&
+	git $1 $3 >actual &&
+	test_cmp expected actual
 }
 
 test_expect_success 'setup' '
 
 	commit master &&
-	git checkout -b subspace/one master
+	git checkout -b subspace/one master &&
 	commit one &&
-	git checkout -b subspace/two master
+	git checkout -b subspace/two master &&
 	commit two &&
-	git checkout -b subspace-x master
+	git checkout -b subspace-x master &&
 	commit subspace-x &&
-	git checkout -b other/three master
+	git checkout -b other/three master &&
 	commit three &&
-	git checkout -b someref master
+	git checkout -b someref master &&
 	commit some &&
 	git checkout master &&
 	commit master2
@@ -68,7 +68,7 @@ test_expect_success 'rev-parse --namespace=heads/someref master' '
 
 test_expect_success 'rev-parse --namespace=heads' '
 
-	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+	compare rev-parse "master other/three someref subspace-x subspace/one subspace/two" "--namespace=heads"
 
 '
 
@@ -92,19 +92,19 @@ test_expect_success 'rev-list --namespace=heads/subspace' '
 
 test_expect_success 'rev-list --namespace=heads/someref master' '
 
-	compare rev-parse "master" "--namespace=heads/someref master"
+	compare rev-list "master" "--namespace=heads/someref master"
 
 '
 
 test_expect_success 'rev-list --namespace=heads/subspace --namespace=heads/other' '
 
-	compare rev-parse "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
+	compare rev-list "subspace/one subspace/two other/three" "--namespace=heads/subspace --namespace=heads/other"
 
 '
 
 test_expect_success 'rev-list --namespace=heads' '
 
-	compare rev-parse "subspace/one subspace/two other/three subspace-x master someref" "--namespace=heads"
+	compare rev-list "master other/three someref subspace-x subspace/one subspace/two" "--namespace=heads"
 
 '
 
-- 
1.6.6.199.gff4b0

Re: [PATCH v2] rev-parse --namespace

[Thomas Rast]

Ilari Liusvaara wrote:
> Add --namespace=<namespace> option to rev-parse and everything that
> accepts its options. This option matches all refs in some subnamespace
> of refs hierarchy.
> 
> Example:
> 
> 'git log --branches --not --namespace=remotes/origin'
> 
> To show what you have that origin doesn't.

Sorry for being so late to this discussion, but... wouldn't it be
nicer to give it some globbing powers and the same semantics as
'fetch' lines?  That way spelling "all master branches of my remotes"
and other such things would be easy.

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH v2] rev-parse --namespace

[Ilari Liusvaara]

On Tue, Jan 19, 2010 at 06:39:25PM +0100, Thomas Rast wrote:
> Ilari Liusvaara wrote:
> 
> Sorry for being so late to this discussion, but... wouldn't it be
> nicer to give it some globbing powers and the same semantics as
> 'fetch' lines?  That way spelling "all master branches of my remotes"
> and other such things would be easy.

Unfortunately doesn't seem to be easy to do, as push and fetch have
their own refspec logic. In fact, if matching power would be extended,
probably the easiest extension would be full-blown extended regular
expressions.

-Ilari

Re: [PATCH v2] rev-parse --namespace

[Junio C Hamano]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> ..., if matching power would be extended,
> probably the easiest extension would be full-blown extended regular
> expressions.

As refs behave like a filesystem path and we try to use fnmatch() for
anything that behave like a filesystem path, that would break consistency.

But a patch to add fnmatch() shouldn't be too bad; something like this
(not even compile tested)?

 refs.c |   45 +++++++++++++++++++++++++++++++++++++--------
 1 files changed, 37 insertions(+), 8 deletions(-)

diff --git a/refs.c b/refs.c
index 5583f4b..0494c75 100644
--- a/refs.c
+++ b/refs.c
@@ -674,19 +674,48 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)
 	return do_for_each_ref("refs/replace/", fn, 13, 0, cb_data);
 }
 
+struct ref_glob_filter {
+	each_ref_fn *user_callback;
+	void *user_cb_data;
+	const char *pattern;
+	int pattern_len;
+};
+
+static int ref_glob_filter_cb(const char *refname,
+			      const unsigned char *sha1,
+			      int flags, void *cb_data)
+{
+	struct ref_glob_filter *cb = cb_data;
+
+	/*
+	 * Reject if it does not produce a prefix match and
+	 * it doesn't pass fnmatch().
+	 */
+	if (!(cb->pattern_len <= strlen(refname)
+	      && !memcmp(cb->pattern, refname, cb->pattern_len)) &&
+	    fnmatch(cb->pattern, refname, 0))
+		return 0;
+	return cb->user_callback(refname, sha1, flags, cb->user_cb_data);
+}
+
 int for_each_namespace_ref(each_ref_fn fn, const char *ns_name, void *cb_data)
 {
-	struct strbuf real_prefix = STRBUF_INIT;
+	struct ref_glob_filter filter;
+	struct strbuf pattern = STRBUF_INIT;
 	int ret;
 
 	if (prefixcmp(ns_name, "refs/"))
-		strbuf_addstr(&real_prefix, "refs/");
-	strbuf_addstr(&real_prefix, ns_name);
-	if (real_prefix.buf[real_prefix.len - 1] != '/')
-		strbuf_addch(&real_prefix, '/');
-
-	ret = for_each_ref_in(real_prefix.buf, fn, cb_data);
-	strbuf_release(&real_prefix);
+		strbuf_addstr(&pattern, "refs/");
+	strbuf_addstr(&pattern, ns_name);
+	if (pattern.buf[pattern.len - 1] != '/')
+		strbuf_addch(&pattern, '/');
+	filter.pattern = pattern.buf;
+	filter.pattern_len = pattern.len;
+	filter.user_callback = fn;
+	filter.user_cb_data = cb_data;
+
+	ret = for_each_ref(ref_glob_filter_cb, &filter);
+	strbuf_release(&pattern);
 	return ret;
 }
 

Re: [PATCH] Fix errors in t6018

[Junio C Hamano]

Thanks, both of you, for being careful.

Re: [PATCH v2] rev-parse --namespace

[Ilari Liusvaara]

On Tue, Jan 19, 2010 at 12:06:12PM -0800, Junio C Hamano wrote:
> Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:
> 
> > ..., if matching power would be extended,
> > probably the easiest extension would be full-blown extended regular
> > expressions.
> 
> As refs behave like a filesystem path and we try to use fnmatch() for
> anything that behave like a filesystem path, that would break consistency.

Eh, remind me what commands take refs and shell-glob them? The only
'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
shell globbing...

-Ilari

Re: [PATCH v2] rev-parse --namespace

[Thomas Rast]

Ilari Liusvaara wrote:
> On Tue, Jan 19, 2010 at 12:06:12PM -0800, Junio C Hamano wrote:
> > As refs behave like a filesystem path and we try to use fnmatch() for
> > anything that behave like a filesystem path, that would break consistency.
> 
> Eh, remind me what commands take refs and shell-glob them? The only
> 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
> shell globbing...

fetchspecs?

-- 
Thomas Rast
trast@{inf,student}.ethz.ch

Re: [PATCH v2] rev-parse --namespace

[Ilari Liusvaara]

On Tue, Jan 19, 2010 at 10:46:51PM +0100, Thomas Rast wrote:
> Ilari Liusvaara wrote:
> > 
> > Eh, remind me what commands take refs and shell-glob them? The only
> > 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
> > shell globbing...
> 
> fetchspecs?

Ah, found one: 'ls-remote'. Documentation of that doesn't say what format
patterns are in... 

Any others? 

-Ilari

Re: [PATCH v2] rev-parse --namespace

[Junio C Hamano]

Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:

> On Tue, Jan 19, 2010 at 10:46:51PM +0100, Thomas Rast wrote:
>> Ilari Liusvaara wrote:
>> > 
>> > Eh, remind me what commands take refs and shell-glob them? The only
>> > 'globbing' of refs I'm aware of is in refspecs, and that definitely isn't
>> > shell globbing...
>> 
>> fetchspecs?
>
> Ah, found one: 'ls-remote'. Documentation of that doesn't say what format
> patterns are in... 
>
> Any others? 

What I gave you was "Please don't use regexp, because matching refs with
fnmatch() is the design guideline we follow".  It was not "please follow
the precedence of existing commands".  IOW, even if there was no command
that matched refs with globs, it is not an excuse to use regexp.  You
didn't even have to find any single example.  But here are some others, if
you are interested.

$ git grep fnmatch builtin-{for-each-ref,reflog,name-rev,show-branch,tag}.c

You find the design guideline to make refs behave as paths in a lot more
fundamental places, such as "if you have 'master' branch, you don't have
master/foo branch", and "you cannot use '?' in ref name because it is a
special character in globbing".

Re: [PATCH v2] rev-parse --namespace

[Johannes Sixt]

I've implemented --remotes=foo on top of your patch because I needed it,
and --namespace=remotes/foo was too much to type. Are you interested in
integrating this in your series in some way?

I only needed the --remotes=foo part; the --branches=foo is only
"because we can". Note that 'foo' is always a complete path component,
because it is the name of a remote (the trailing slash is implied), e.g.,
'--remotes=origin'.

--- 8< ---
From: Johannes Sixt <j6t@kdbg.org>
Subject: [PATCH] revision options --remotes=prefix and --branches=prefix

Signed-off-by: Johannes Sixt <j6t@kdbg.org>
---
 builtin-rev-parse.c |    2 ++
 revision.c          |   23 +++++++++++++++++++++++
 2 files changed, 25 insertions(+), 0 deletions(-)

diff --git a/builtin-rev-parse.c b/builtin-rev-parse.c
index 34af347..08b0555 100644
--- a/builtin-rev-parse.c
+++ b/builtin-rev-parse.c
@@ -42,6 +42,7 @@ static int is_rev_argument(const char *arg)
 		"--bisect",
 		"--dense",
 		"--branches",
+		"--branches=",
 		"--header",
 		"--max-age=",
 		"--max-count=",
@@ -52,6 +53,7 @@ static int is_rev_argument(const char *arg)
 		"--parents",
 		"--pretty",
 		"--remotes",
+		"--remotes=",
 		"--namespace=",
 		"--sparse",
 		"--tags",
diff --git a/revision.c b/revision.c
index ec63fa0..68e251b 100644
--- a/revision.c
+++ b/revision.c
@@ -714,6 +714,21 @@ static void handle_refs(struct rev_info *revs, unsigned flags,
 	for_each(handle_one_ref, &cb);
 }

+static void handle_ref_subset(struct rev_info *revs, unsigned flags,
+			      const char *prefix, const char *postfix)
+{
+	struct strbuf name = STRBUF_INIT;
+	struct all_refs_cb cb;
+
+	strbuf_addstr(&name, prefix);
+	strbuf_addstr(&name, postfix);
+	if (name.buf[name.len - 1] != '/')
+		strbuf_addch(&name, '/');
+	init_all_refs_cb(&cb, revs, flags);
+	for_each_ref_in(name.buf, handle_one_ref, &cb);
+	strbuf_release(&name);
+}
+
 static void handle_one_reflog_commit(unsigned char *sha1, void *cb_data)
 {
 	struct all_refs_cb *cb = cb_data;
@@ -1344,6 +1359,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				handle_refs(revs, flags, for_each_branch_ref);
 				continue;
 			}
+			if (!prefixcmp(arg, "--branches=")) {
+				handle_ref_subset(revs, flags, "refs/heads/", arg + 11);
+				continue;
+			}
 			if (!strcmp(arg, "--bisect")) {
 				handle_refs(revs, flags, for_each_bad_bisect_ref);
 				handle_refs(revs, flags ^ UNINTERESTING, for_each_good_bisect_ref);
@@ -1358,6 +1377,10 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch
 				handle_refs(revs, flags, for_each_remote_ref);
 				continue;
 			}
+			if (!prefixcmp(arg, "--remotes=")) {
+				handle_ref_subset(revs, flags, "refs/remotes/", arg + 10);
+				continue;
+			}
 			if (!prefixcmp(arg, "--namespace=")) {
 				struct all_refs_cb cb;

-- 
1.6.6.1301.g354d