[PATCH 0/2] Add --expand to 'git notes get-ref'

From: "W. Trevor King" <redacted>

I was recently confused when

  $ git notes merge -v refs/remotes/origin/notes/commits

failed to do (or print) anything.  It turns out that note refs must
live under 'refs/notes/', so my command line ref was expanding to

  refs/notes/refs/remotes/origin/notes/commits

which wasn't matching anything.

The first of my commits here adds the '--expand' option, so you can
see how Git is expanding your ref internally:

  $ GIT_NOTES_REF=commits git notes get-ref --expand
  refs/notes/commits

The second commit makes the expansion less strict about the location
of note refs.  In his initial mail introducing 'git notes', Johan says
that note refs should live under 'refs/notes' [1].  This seems like a
good place for local notes, but note refs from remote repos should
probably live somewhere else (e.g. 'refs/remote-notes/' or
'refs/remotes/<name>/notes/'.  In the initial thread there are a few
messages talking about looking up reverse mappings under 'refs/notes/',
but this seems to all have been before the 'refs/notes/<namespace>/'
stage.  If I'm missing a good reason to keep everything under
'refs/notes/', feel free to ignore the second patch.

Cheers,
Trevor

[1]: http://permalink.gmane.org/gmane.comp.version-control.git/48540


W. Trevor King (2):
  notes get-ref: --expand expands the output notes ref.
  notes: don't alter refs starting with 'refs/' in expand_notes_ref

 Documentation/git-notes.txt |  6 +++++-
 builtin/notes.c             | 26 +++++++++++++++++++++-----
 notes.c                     |  2 +-
 t/t3301-notes.sh            | 12 ++++++++++++
 4 files changed, 39 insertions(+), 7 deletions(-)

-- 
1.7.12.176.g3fc0e4c.dirty

From: "W. Trevor King" <redacted>

This avoids surprising cases like:

  $ GIT_NOTES_REF=refs/remotes/origin/notes/commits git notes get-ref --expand
  refs/notes/refs/remotes/origin/notes/commits

With the old implementation, all note refs had to live under
'refs/notes/' which was not mentioned in the git-notes.txt.

Signed-off-by: W. Trevor King <redacted>
---
 notes.c          | 2 +-
 t/t3301-notes.sh | 4 ++++
 2 files changed, 5 insertions(+), 1 deletion(-)

diff --git a/notes.c b/notes.c
index 93e9868..60394c8 100644
--- a/notes.c
+++ b/notes.c

@@ -1289,7 +1289,7 @@ int copy_note(struct notes_tree *t,
 
 void expand_notes_ref(struct strbuf *sb)
 {
-	if (!prefixcmp(sb->buf, "refs/notes/"))
+	if (!prefixcmp(sb->buf, "refs/"))
 		return; /* we're happy */
 	else if (!prefixcmp(sb->buf, "notes/"))
 		strbuf_insert(sb, 0, "refs/", 5);

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index c0486a0..707d7e6 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh

@@ -1230,4 +1230,8 @@ test_expect_success 'git notes get-ref (--expand)' '
 	test "$(GIT_NOTES_REF=commits git notes get-ref --expand)" = "refs/notes/commits"
 '
 
+test_expect_success 'git notes get-ref (--expand)' '
+	test "$(GIT_NOTES_REF=refs/remotes/origin/notes/commits git notes get-ref --expand)" = "refs/remotes/origin/notes/commits"
+'
+
 test_done

-- 
1.7.12.176.g3fc0e4c.dirty

From: "W. Trevor King" <redacted>

Useful for debugging refs that don't seem to be expanding correctly.

Signed-off-by: W. Trevor King <redacted>
---
 Documentation/git-notes.txt |  6 +++++-
 builtin/notes.c             | 26 +++++++++++++++++++++-----
 t/t3301-notes.sh            |  8 ++++++++
 3 files changed, 34 insertions(+), 6 deletions(-)

diff --git a/Documentation/git-notes.txt b/Documentation/git-notes.txt
index b95aafa..a93d211 100644
--- a/Documentation/git-notes.txt
+++ b/Documentation/git-notes.txt

@@ -19,7 +19,7 @@ SYNOPSIS
 'git notes' merge --abort [-v | -q]
 'git notes' remove [--ignore-missing] [--stdin] [<object>...]
 'git notes' prune [-n | -v]
-'git notes' get-ref
+'git notes' get-ref [-e]
 
 
 DESCRIPTION

@@ -165,6 +165,10 @@ OPTIONS
 	input (there is no reason you cannot combine this with object
 	names from the command line).
 
+-e::
+--expand::
+  Expand the notes ref before printing it.
+
 -n::
 --dry-run::
 	Do not remove anything; just report the object names whose notes

diff --git a/builtin/notes.c b/builtin/notes.c
index 3644d14..17c6136 100644
--- a/builtin/notes.c
+++ b/builtin/notes.c

@@ -31,7 +31,7 @@ static const char * const git_notes_usage[] = {
 	"git notes merge --abort [-v | -q]",
 	"git notes [--ref <notes_ref>] remove [<object>...]",
 	"git notes [--ref <notes_ref>] prune [-n | -v]",
-	"git notes [--ref <notes_ref>] get-ref",
+	"git notes [--ref <notes_ref>] get-ref [-e]",
 	NULL
 };
 

@@ -84,7 +84,7 @@ static const char * const git_notes_prune_usage[] = {
 };
 
 static const char * const git_notes_get_ref_usage[] = {
-	"git notes get-ref",
+	"git notes get-ref [<options>]",
 	NULL
 };
 

@@ -1046,16 +1046,32 @@ static int prune(int argc, const char **argv, const char *prefix)
 
 static int get_ref(int argc, const char **argv, const char *prefix)
 {
-	struct option options[] = { OPT_END() };
+	const char *ref;
+	struct strbuf buf = STRBUF_INIT;
+	int expand = 0;
+	struct option options[] = {
+		OPT_BOOL('e', "expand", &expand,
+		         "expand the notes ref before printing it"),
+		OPT_END()
+	};
 	argc = parse_options(argc, argv, prefix, options,
 			     git_notes_get_ref_usage, 0);
 
-	if (argc) {
+	if (argc > 1) {
 		error("too many parameters");
 		usage_with_options(git_notes_get_ref_usage, options);
 	}
 
-	puts(default_notes_ref());
+	ref = default_notes_ref();
+	if (expand) {
+		strbuf_insert(&buf, 0, ref, strlen(ref));
+		expand_notes_ref(&buf);
+		puts(buf.buf);
+		strbuf_release(&buf);
+	} else {
+		puts(ref);
+	}	
+
 	return 0;
 }
 

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 16de05a..c0486a0 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh

@@ -1222,4 +1222,12 @@ test_expect_success 'git notes get-ref (--ref)' '
 	test "$(GIT_NOTES_REF=refs/notes/bar git notes --ref=baz get-ref)" = "refs/notes/baz"
 '
 
+test_expect_success 'git notes get-ref (no expand)' '
+	test "$(GIT_NOTES_REF=commits git notes get-ref)" = "commits"
+'
+
+test_expect_success 'git notes get-ref (--expand)' '
+	test "$(GIT_NOTES_REF=commits git notes get-ref --expand)" = "refs/notes/commits"
+'
+
 test_done

-- 
1.7.12.176.g3fc0e4c.dirty

On Wed, Sep 5, 2012 at 2:48 PM, W. Trevor King [off-list ref] wrote:

The second commit makes the expansion less strict about the location
of note refs.  In his initial mail introducing 'git notes', Johan says
that note refs should live under 'refs/notes' [1].  This seems like a
good place for local notes, but note refs from remote repos should
probably live somewhere else (e.g. 'refs/remote-notes/' or
'refs/remotes/<name>/notes/'.  In the initial thread there are a few
messages talking about looking up reverse mappings under 'refs/notes/',
but this seems to all have been before the 'refs/notes/<namespace>/'
stage.  If I'm missing a good reason to keep everything under
'refs/notes/', feel free to ignore the second patch.

This has been discussed a couple of times on this list, but it never
resulted in any actual changes. Read up on this thread to get some
context:

http://thread.gmane.org/gmane.comp.version-control.git/160503


...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net

On Wed, Sep 5, 2012 at 2:52 PM, W. Trevor King [off-list ref] wrote:

From: "W. Trevor King" <redacted>

Useful for debugging refs that don't seem to be expanding correctly.

Signed-off-by: W. Trevor King <redacted>

Acked-by: Johan Herland <redacted>


-- 
Johan Herland, [off-list ref]
www.herland.net

On Wed, Sep 05, 2012 at 05:58:37PM +0200, Johan Herland wrote:

On Wed, Sep 5, 2012 at 2:48 PM, W. Trevor King [off-list ref] wrote:

quoted

If I'm missing a good reason to keep everything under
'refs/notes/', feel free to ignore the second patch.

This has been discussed a couple of times on this list, but it never
resulted in any actual changes. Read up on this thread to get some
context:

http://thread.gmane.org/gmane.comp.version-control.git/160503

Thanks for the pointer, it looks like there are a bunch of good ideas.
I assume the lack of changes was due to nobody having the
time/inclination to implement

  http://article.gmane.org/gmane.comp.version-control.git/160655

Trevor

-- 
As a side note, does anyone know of a thread-specific version of

  http://download.gmane.org/<xyz>/<start>/<stop>

It would be easier for me (and less load for them) if I could run

  http://download.gmane.org/gmane.comp.version-control.git/thread/160503

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

On Wed, Sep 5, 2012 at 7:53 PM, W. Trevor King [off-list ref] wrote:

On Wed, Sep 05, 2012 at 05:58:37PM +0200, Johan Herland wrote:

quoted

On Wed, Sep 5, 2012 at 2:48 PM, W. Trevor King [off-list ref] wrote:

quoted

If I'm missing a good reason to keep everything under
'refs/notes/', feel free to ignore the second patch.

This has been discussed a couple of times on this list, but it never
resulted in any actual changes. Read up on this thread to get some
context:

http://thread.gmane.org/gmane.comp.version-control.git/160503

Thanks for the pointer, it looks like there are a bunch of good ideas.
I assume the lack of changes was due to nobody having the
time/inclination to implement

  http://article.gmane.org/gmane.comp.version-control.git/160655

Yes, I think time/inclination probably has a lot to do with it. Also,
I forgot to link to the largest mailing list thread that discusses
these changes in _much_ more detail:

http://thread.gmane.org/gmane.comp.version-control.git/165799/focus=165885

As you can see, the discussion quickly balloons into something much
larger than a simple discussion of where remote notes should be
located...

That said, your patch 2/2 might be an acceptable stopgap measure for
the time being, and then we can reevaluate it if/when we implement a
larger ref restructuring.


...Johan

-- 
Johan Herland, [off-list ref]
www.herland.net