{"thread":{"id":"22258","subject":"[PATCH] rev-parse --namespace","startedAt":"2010-01-17T13:45:31Z","lastAt":"2010-01-17T19:28:06Z","messageCount":5,"participants":["Ilari Liusvaara","Jeff King","Junio C Hamano"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"131943","messageId":"1263735931-20227-1-git-send-email-ilari.liusvaara@elisanet.fi","threadId":"22258","inReplyTo":null,"subject":"[PATCH] rev-parse --namespace","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-01-17T13:45:31Z","receivedAt":"2010-01-17T13:45:31Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"Add --namespace=<namespace> option to rev-parse and everything that\naccepts its options. This option matches all refs in some subnamespace\nof refs hierarchy, and is useful for selecting everything reachable from\none or few, but not all remotes (--namespace=remotes/foo).\n\nSigned-off-by: Ilari Liusvaara <ilari.liusvaara@elisanet.fi>\n---\nAnother one from my todo list. Based on master branch.\n\n Documentation/git-rev-list.txt     |    1 +\n Documentation/git-rev-parse.txt    |    4 ++\n Documentation/rev-list-options.txt |    6 ++\n builtin-rev-parse.c                |    6 ++\n refs.c                             |   33 +++++++++++\n refs.h                             |    9 +++\n revision.c                         |    5 ++\n t/t6018-rev-list-namespace.sh      |  103 ++++++++++++++++++++++++++++++++++++\n 8 files changed, 167 insertions(+), 0 deletions(-)\n create mode 100755 t/t6018-rev-list-namespace.sh\n\ndiff --git a/Documentation/git-rev-list.txt b/Documentation/git-rev-list.txt\nindex 3341d1b..a8f8f22 100644\n--- a/Documentation/git-rev-list.txt\n+++ b/Documentation/git-rev-list.txt\n@@ -24,6 +24,7 @@ SYNOPSIS\n \t     [ \\--branches ]\n \t     [ \\--tags ]\n \t     [ \\--remotes ]\n+\t     [ \\--namespace=namespace-prefix ]\n \t     [ \\--stdin ]\n \t     [ \\--quiet ]\n \t     [ \\--topo-order ]\ndiff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\nindex 82045a2..af4605a 100644\n--- a/Documentation/git-rev-parse.txt\n+++ b/Documentation/git-rev-parse.txt\n@@ -112,6 +112,10 @@ OPTIONS\n --remotes::\n \tShow tag refs found in `$GIT_DIR/refs/remotes`.\n \n+--namespace=namespace-prefix::\n+\tShow refs found in `$GIT_DIR/namespace-prefix`. If namespace\n+\tspecified lacks leading 'refs/', it is automatically prepended.\n+\n --show-prefix::\n \tWhen the command is invoked from a subdirectory, show the\n \tpath of the current directory relative to the top-level\ndiff --git a/Documentation/rev-list-options.txt b/Documentation/rev-list-options.txt\nindex 1f57aed..c824a7b 100644\n--- a/Documentation/rev-list-options.txt\n+++ b/Documentation/rev-list-options.txt\n@@ -243,6 +243,12 @@ endif::git-rev-list[]\n \tPretend as if all the refs in `$GIT_DIR/refs/remotes` are listed\n \ton the command line as '<commit>'.\n \n+--namespace=namespace-prefix::\n+\tPretend as if all the refs in `$GIT_DIR/namespace-prefix` are\n+\tlisted on the command line as '<commit>'. Leading 'refs/', it\n+\tis automatically prepended if missing.\n+\n+\n ifndef::git-rev-list[]\n --bisect::\n \ndiff --git a/builtin-rev-parse.c b/builtin-rev-parse.c\nindex 37d0233..d7cb3c1 100644\n--- a/builtin-rev-parse.c\n+++ b/builtin-rev-parse.c\n@@ -52,6 +52,7 @@ static int is_rev_argument(const char *arg)\n \t\t\"--parents\",\n \t\t\"--pretty\",\n \t\t\"--remotes\",\n+\t\t\"--namespace=\",\n \t\t\"--sparse\",\n \t\t\"--tags\",\n \t\t\"--topo-order\",\n@@ -577,6 +578,11 @@ int cmd_rev_parse(int argc, const char **argv, const char *prefix)\n \t\t\t\tfor_each_tag_ref(show_reference, NULL);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!prefixcmp(arg, \"--namespace=\")) {\n+\t\t\t\tset_for_each_namespace(arg + 12);\n+\t\t\t\tfor_each_namespace_ref(show_reference, NULL);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--remotes\")) {\n \t\t\t\tfor_each_remote_ref(show_reference, NULL);\n \t\t\t\tcontinue;\ndiff --git a/refs.c b/refs.c\nindex 3e73a0a..ae3f1a4 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -7,6 +7,9 @@\n /* ISSYMREF=01 and ISPACKED=02 are public interfaces */\n #define REF_KNOWS_PEELED 04\n \n+/* Current prefix namespace in use. NULL means none. */\n+static char* prefix_namespace = NULL;\n+\n struct ref_list {\n \tstruct ref_list *next;\n \tunsigned char flag; /* ISSYMREF? ISPACKED? */\n@@ -674,6 +677,36 @@ int for_each_replace_ref(each_ref_fn fn, void *cb_data)\n \treturn do_for_each_ref(\"refs/replace/\", fn, 13, 0, cb_data);\n }\n \n+void set_for_each_namespace(const char *ref_namespace)\n+{\n+\tsize_t alloclen, origlen;\n+\tif (prefix_namespace)\n+\t\tfree(prefix_namespace);\n+\n+\t/* Compute the length of true prefix. */\n+\toriglen = alloclen = strlen(ref_namespace);\n+\tif (*ref_namespace && ref_namespace[origlen - 1] != '/')\n+\t\talloclen++;\n+\tif (prefixcmp(ref_namespace, \"refs/\"))\n+\t\talloclen += 5;\t\t/* 'refs/' */\n+\n+\t/* Allocate and build it (assume alloclen is \"small\") */\n+\tprefix_namespace = xmalloc(alloclen + 1);\n+\t*prefix_namespace = 0;\n+\tif (prefixcmp(ref_namespace, \"refs/\"))\n+\t\tstrcat(prefix_namespace, \"refs/\");\n+\tstrcat(prefix_namespace, ref_namespace);\n+\tif (*ref_namespace && ref_namespace[origlen - 1] != '/')\n+\t\tstrcat(prefix_namespace, \"/\");\n+}\n+\n+int for_each_namespace_ref(each_ref_fn fn, void *cb_data)\n+{\n+\tif (!prefix_namespace)\n+\t\tdie(\"BUG: Call set_for_each_namespace() before for_each_namespace_ref()!\");\n+\treturn for_each_ref_in(prefix_namespace, fn, cb_data);\n+}\n+\n int for_each_rawref(each_ref_fn fn, void *cb_data)\n {\n \treturn do_for_each_ref(\"refs/\", fn, 0,\ndiff --git a/refs.h b/refs.h\nindex e141991..be493ca 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -25,6 +25,15 @@ extern int for_each_tag_ref(each_ref_fn, void *);\n extern int for_each_branch_ref(each_ref_fn, void *);\n extern int for_each_remote_ref(each_ref_fn, void *);\n extern int for_each_replace_ref(each_ref_fn, void *);\n+extern int for_each_namespace_ref(each_ref_fn, void *);\n+\n+/*\n+ * Set namespace for_each_namespace_ref() operates in. Must be called before\n+ * calling that function. Autoprepends 'refs/' if it is missing. Autoappends\n+ * '/' if it is missing.\n+ */\n+void set_for_each_namespace(const char *ref_namespace);\n+\n \n /* can be used to learn about broken ref and symref */\n extern int for_each_rawref(each_ref_fn, void *);\ndiff --git a/revision.c b/revision.c\nindex 25fa14d..9e1d960 100644\n--- a/revision.c\n+++ b/revision.c\n@@ -1352,6 +1352,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n \t\t\t\thandle_refs(revs, flags, for_each_remote_ref);\n \t\t\t\tcontinue;\n \t\t\t}\n+\t\t\tif (!prefixcmp(arg, \"--namespace=\")) {\n+\t\t\t\tset_for_each_namespace(arg + 12);\n+\t\t\t\thandle_refs(revs, flags, for_each_namespace_ref);\n+\t\t\t\tcontinue;\n+\t\t\t}\n \t\t\tif (!strcmp(arg, \"--reflog\")) {\n \t\t\t\thandle_reflog(revs, flags);\n \t\t\t\tcontinue;\ndiff --git a/t/t6018-rev-list-namespace.sh b/t/t6018-rev-list-namespace.sh\nnew file mode 100755\nindex 0000000..f04bde1\n--- /dev/null\n+++ b/t/t6018-rev-list-namespace.sh\n@@ -0,0 +1,103 @@\n+#!/bin/sh\n+\n+test_description='rev-list/rev-parse --namespace'\n+\n+. ./test-lib.sh\n+\n+\n+commit () {\n+\ttest_tick &&\n+\techo $1 > foo &&\n+\tgit add foo &&\n+\tgit commit -m \"$1\"\n+}\n+\n+test_expect_success 'setup' '\n+\n+\tcommit master &&\n+\tgit checkout -b subspace/one master\n+\tcommit one &&\n+\tgit checkout -b subspace/two master\n+\tcommit two &&\n+\tgit checkout -b other/three master\n+\tcommit three &&\n+\tgit checkout -b someref master\n+\tcommit some &&\n+\tgit checkout master\n+\tcommit master2\n+'\n+\n+test_expect_success 'rev-parse --namespace=refs/heads/subspace/' '\n+\n+\ttest 2 = $(git rev-parse --namespace=refs/heads/subspace/ | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-parse --namespace=refs/heads/subspace' '\n+\n+\ttest 2 = $(git rev-parse --namespace=refs/heads/subspace | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-parse --namespace=heads/subspace' '\n+\n+\ttest 2 = $(git rev-parse --namespace=heads/subspace | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-parse --namespace=heads/subspace --namespace=heads/other' '\n+\n+\ttest 3 = $(git rev-parse --namespace=heads/subspace --namespace=heads/other | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-parse --namespace=heads/someref master' '\n+\n+\ttest 1 = $(git rev-parse --namespace=heads/someref master | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-parse --namespace=heads' '\n+\n+\ttest 4 = $(git rev-parse --namespace=heads | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=refs/heads/subspace/' '\n+\n+\ttest 3 = $(git rev-list --namespace=refs/heads/subspace/ | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=refs/heads/subspace' '\n+\n+\ttest 3 = $(git rev-list --namespace=refs/heads/subspace | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=heads/subspace' '\n+\n+\ttest 3 = $(git rev-list --namespace=heads/subspace | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=heads/someref master' '\n+\n+\ttest 2 = $(git rev-list --namespace=heads/someref master | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=heads/subspace --namespace=heads/other' '\n+\n+\ttest 4 = $(git rev-list --namespace=heads/subspace --namespace=heads/other | wc -l)\n+\n+'\n+\n+test_expect_success 'rev-list --namespace=heads' '\n+\n+\ttest 5 = $(git rev-list --namespace=heads | wc -l)\n+\n+'\n+\n+\n+test_done\n-- \n1.6.6.199.gff4b0\n"},{"id":"131951","messageId":"20100117162712.GB7153@sigill.intra.peff.net","threadId":"22258","inReplyTo":"1263735931-20227-1-git-send-email-ilari.liusvaara@elisanet.fi","subject":"Re: [PATCH] rev-parse --namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-17T16:27:12Z","receivedAt":"2010-01-17T16:27:12Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 17, 2010 at 03:45:31PM +0200, Ilari Liusvaara wrote:\n\n> Add --namespace=<namespace> option to rev-parse and everything that\n> accepts its options. This option matches all refs in some subnamespace\n> of refs hierarchy, and is useful for selecting everything reachable from\n> one or few, but not all remotes (--namespace=remotes/foo).\n\nIf I understand it correctly, isn't the same as\n\n  git for-each-ref refs/remotes/foo\n\n?\n\n-Peff\n"},{"id":"131952","messageId":"20100117164057.GA20554@Knoppix","threadId":"22258","inReplyTo":"20100117162712.GB7153@sigill.intra.peff.net","subject":"Re: [PATCH] rev-parse --namespace","fromName":"Ilari Liusvaara","fromEmail":"ilari.liusvaara@elisanet.fi","sentAt":"2010-01-17T16:40:57Z","receivedAt":"2010-01-17T16:40:57Z","isPatch":true,"sender":{"key":"ilari.liusvaara@elisanet.fi","avatar":null},"body":"On Sun, Jan 17, 2010 at 11:27:12AM -0500, Jeff King wrote:\n> On Sun, Jan 17, 2010 at 03:45:31PM +0200, Ilari Liusvaara wrote:\n> \n> > Add --namespace=<namespace> option to rev-parse and everything that\n> > accepts its options. This option matches all refs in some subnamespace\n> > of refs hierarchy, and is useful for selecting everything reachable from\n> > one or few, but not all remotes (--namespace=remotes/foo).\n> \n> If I understand it correctly, isn't the same as\n> \n>   git for-each-ref refs/remotes/foo\n\nNope. Compare:\n\n'git log --branches --not --namespace=remotes/origin' \n\nwith whatever you would have to currently type...\n\n-Ilari\n"},{"id":"131955","messageId":"20100117190827.GA31536@coredump.intra.peff.net","threadId":"22258","inReplyTo":"20100117164057.GA20554@Knoppix","subject":"Re: [PATCH] rev-parse --namespace","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-01-17T19:08:27Z","receivedAt":"2010-01-17T19:08:27Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Sun, Jan 17, 2010 at 06:40:57PM +0200, Ilari Liusvaara wrote:\n\n> On Sun, Jan 17, 2010 at 11:27:12AM -0500, Jeff King wrote:\n> > On Sun, Jan 17, 2010 at 03:45:31PM +0200, Ilari Liusvaara wrote:\n> > \n> > > Add --namespace=<namespace> option to rev-parse and everything that\n> > > accepts its options. This option matches all refs in some subnamespace\n> > > of refs hierarchy, and is useful for selecting everything reachable from\n> > > one or few, but not all remotes (--namespace=remotes/foo).\n> > \n> > If I understand it correctly, isn't the same as\n> > \n> >   git for-each-ref refs/remotes/foo\n> \n> Nope. Compare:\n> \n> 'git log --branches --not --namespace=remotes/origin' \n> \n> with whatever you would have to currently type...\n\nOK, that makes sense and is actually useful. That is the sort of\nmotivation that should go in the commit message. Mentioning \"rev-parse\"\nis a bit of a red herring.\n\nAs for the patch text itself, it looks correct, though I have a few\nnits:\n\n> diff --git a/Documentation/git-rev-parse.txt b/Documentation/git-rev-parse.txt\n> index 82045a2..af4605a 100644\n> --- a/Documentation/git-rev-parse.txt\n> +++ b/Documentation/git-rev-parse.txt\n> @@ -112,6 +112,10 @@ OPTIONS\n>  --remotes::\n>  \tShow tag refs found in `$GIT_DIR/refs/remotes`.\n>  \n> +--namespace=namespace-prefix::\n> +\tShow refs found in `$GIT_DIR/namespace-prefix`. If namespace\n> +\tspecified lacks leading 'refs/', it is automatically prepended.\n> +\n\nThis is not a new problem you are introducing, as you are just following\nthe template of of --remotes and others above, but is \"show\" really the\nright word? In the rev-list page these entries talk about \"pretend as if\nthese refs were given on the command line\". Isn't that also the case\nhere? If I say \"git rev-parse --not --remotes\" (or --namespace=), I will\nget \"^\"-lines.\n\nAlso, I notice in the context that the remotes entry says \"Show tag\nrefs\" which is obviously wrong. Again, not your problem, but something\nto clean up while in the area.\n\n> +\t\t\tif (!prefixcmp(arg, \"--namespace=\")) {\n> +\t\t\t\tset_for_each_namespace(arg + 12);\n> +\t\t\t\tfor_each_namespace_ref(show_reference, NULL);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n\nI was going to complain about the use of a global variable here when we\ncould simply pass the namespace into the function, but I see that in the\nother call here:\n\n> diff --git a/revision.c b/revision.c\n> index 25fa14d..9e1d960 100644\n> --- a/revision.c\n> +++ b/revision.c\n> @@ -1352,6 +1352,11 @@ int setup_revisions(int argc, const char **argv, struct rev_info *revs, const ch\n>  \t\t\t\thandle_refs(revs, flags, for_each_remote_ref);\n>  \t\t\t\tcontinue;\n>  \t\t\t}\n> +\t\t\tif (!prefixcmp(arg, \"--namespace=\")) {\n> +\t\t\t\tset_for_each_namespace(arg + 12);\n> +\t\t\t\thandle_refs(revs, flags, for_each_namespace_ref);\n> +\t\t\t\tcontinue;\n> +\t\t\t}\n\n...that giving the for_each_namespace_ref function a different signature\nthan the other for_each_* functions would complicate things. OTOH,\nhandle_refs is only a 4 line helper to set up the callback struct, so\nperhaps it is better to simply duplicate it rather than introduce a new\nglobal.\n\nToo bad we can't do currying in C. :)\n\n> +void set_for_each_namespace(const char *ref_namespace)\n> +{\n> +\tsize_t alloclen, origlen;\n> +\tif (prefix_namespace)\n> +\t\tfree(prefix_namespace);\n> +\n> +\t/* Compute the length of true prefix. */\n> +\toriglen = alloclen = strlen(ref_namespace);\n> +\tif (*ref_namespace && ref_namespace[origlen - 1] != '/')\n> +\t\talloclen++;\n> +\tif (prefixcmp(ref_namespace, \"refs/\"))\n> +\t\talloclen += 5;\t\t/* 'refs/' */\n> +\n> +\t/* Allocate and build it (assume alloclen is \"small\") */\n> +\tprefix_namespace = xmalloc(alloclen + 1);\n> +\t*prefix_namespace = 0;\n> +\tif (prefixcmp(ref_namespace, \"refs/\"))\n> +\t\tstrcat(prefix_namespace, \"refs/\");\n> +\tstrcat(prefix_namespace, ref_namespace);\n> +\tif (*ref_namespace && ref_namespace[origlen - 1] != '/')\n> +\t\tstrcat(prefix_namespace, \"/\");\n> +}\n\nWouldn't this be much simpler and more readable using a strbuf? As in:\n\n  strbuf_reset(&prefix_namespace);\n  if (prefixcmp(ref_namespace, \"refs/\"))\n    strbuf_addstr(&prefix_namespace, \"refs/\");\n  strbuf_addstr(&prefix_namespace, ref_namespace);\n  if (prefix_namespace.buf[prefix_namespace.len-1] != '/')\n    strbuf_addch(&prefix_namespace, '/');\n\n?\n\n> +test_expect_success 'setup' '\n> +\n> +\tcommit master &&\n> +\tgit checkout -b subspace/one master\n> +\tcommit one &&\n> +\tgit checkout -b subspace/two master\n> +\tcommit two &&\n> +\tgit checkout -b other/three master\n> +\tcommit three &&\n> +\tgit checkout -b someref master\n> +\tcommit some &&\n> +\tgit checkout master\n> +\tcommit master2\n> +'\n\nMissing '&&' on the checkout lines?\n\n> +test_expect_success 'rev-parse --namespace=refs/heads/subspace/' '\n> +\n> +\ttest 2 = $(git rev-parse --namespace=refs/heads/subspace/ | wc -l)\n> +\n> +'\n\nThis is perhaps a minor nit, but I really prefer to write this by\nputting the expected output in a file and using test_cmp, because:\n\n  1. You actually test that rev-parse exits correctly.\n\n  2. You test that it actually produced the correct output, and not\n     simply two lines which are presumably correct.\n\n  3. If it _does_ fail, the output from test_cmp is a nicely readable\n     diff between expected and actual output. Your test produces no\n     output.\n\nWe also had some problems with different implementations of \"wc\"\nproducing different amounts of whitespace, but I don't think that is a\nproblem here since the shell should eat any whitespace outside of\nquotation marks.\n\n-Peff\n"},{"id":"131958","messageId":"7v4omkzg3d.fsf@alter.siamese.dyndns.org","threadId":"22258","inReplyTo":"20100117164057.GA20554@Knoppix","subject":"Re: [PATCH] rev-parse --namespace","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2010-01-17T19:28:06Z","receivedAt":"2010-01-17T19:28:06Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Ilari Liusvaara <ilari.liusvaara@elisanet.fi> writes:\n\n> 'git log --branches --not --namespace=remotes/origin' \n\nThis sounds sensible; it essentially is a natural extention of the\nexisting --branches, --tags and --remotes options.  While I don't care too\nmuch, people may want to have an even shorter name (perhaps --under=heads\nor even --in=tags.\n\nIn any case, we would want to have your \"log\" command line above both in\nthe proposed commit log message and the documentation in the Examples\nsection, given that this is not immediately obvious to even veterans like\nPeff and I; it didn't \"click\" for me until I saw the follow-up exchange\nbetween you two.\n"}]}