[PATCH 1/2] t3301-notes: Test the creation of reflog entries

Test whether the notes code writes reflog entries. It intends to
(setting up the reflog messages) but currently does not.

Signed-off-by: Michael J Gruber <redacted>
---
 t/t3301-notes.sh |    9 +++++++++
 1 files changed, 9 insertions(+), 0 deletions(-)

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 1d6cd45..5410a6d 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh

@@ -65,6 +65,15 @@ test_expect_success 'create notes' '
 	test_must_fail git notes show HEAD^
 '
 
+cat >expect <<EOF
+d423f8c refs/notes/commits@{0}: notes: Notes added by 'git notes add'
+EOF
+
+test_expect_failure 'create reflog entry' '
+	git reflog show refs/notes/commits >output &&
+	test_cmp expect output
+'
+
 test_expect_success 'edit existing notes' '
 	MSG=b3 git notes edit &&
 	test ! -f .git/NOTES_EDITMSG &&

-- 
1.7.0.3.448.g82eeb

The notes code intends to write reflog entries, but currently they are
not written because log_ref_write() checks for the refname path
explicitly.

Add refs/notes to the list of allowed paths so that notes references are
treated just like branch heads, i.e. according to core.logAllRefUpdates
and core.bare.

Signed-off-by: Michael J Gruber <redacted>
---
This is actually inspired by Jeff's novel notes use. I think there are
use cases where a notes log makes sense (notes on commits) and those
where it does not (metadata/textconv). In both cases having a reflog is
useful. So, the next step is really to allow notes trees without
history, which also takes care of the pruning issue. I know how to do this,
I just have to decide about the configuration options.

 refs.c           |    1 +
 t/t3301-notes.sh |    2 +-
 2 files changed, 2 insertions(+), 1 deletions(-)

diff --git a/refs.c b/refs.c
index 0f24c8d..d3db15a 100644
--- a/refs.c
+++ b/refs.c

@@ -1276,6 +1276,7 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,
 	if (log_all_ref_updates &&
 	    (!prefixcmp(ref_name, "refs/heads/") ||
 	     !prefixcmp(ref_name, "refs/remotes/") ||
+	     !prefixcmp(ref_name, "refs/notes/") ||
 	     !strcmp(ref_name, "HEAD"))) {
 		if (safe_create_leading_directories(log_file) < 0)
 			return error("unable to create directory for %s",

diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh
index 5410a6d..b2e7b07 100755
--- a/t/t3301-notes.sh
+++ b/t/t3301-notes.sh

@@ -69,7 +69,7 @@ cat >expect <<EOF
 d423f8c refs/notes/commits@{0}: notes: Notes added by 'git notes add'
 EOF
 
-test_expect_failure 'create reflog entry' '
+test_expect_success 'create reflog entry' '
 	git reflog show refs/notes/commits >output &&
 	test_cmp expect output
 '

-- 
1.7.0.3.448.g82eeb

On Monday 29 March 2010, Michael J Gruber wrote:

The notes code intends to write reflog entries, but currently they
are not written because log_ref_write() checks for the refname path
explicitly.

Add refs/notes to the list of allowed paths so that notes references
are treated just like branch heads, i.e. according to
core.logAllRefUpdates and core.bare.

Signed-off-by: Michael J Gruber <redacted>

Both patches are

Acked-by: Johan Herland <redacted>

---
This is actually inspired by Jeff's novel notes use. I think there
are use cases where a notes log makes sense (notes on commits) and
those where it does not (metadata/textconv). In both cases having a
reflog is useful. So, the next step is really to allow notes trees
without history, which also takes care of the pruning issue. I know
how to do this, I just have to decide about the configuration
options.

I noticed that Jeff's proof-of-concept wrote notes trees without making 
notes commits, and although it seemed like a bug at first, it does - as 
you say - provide a rather nice way to store notes trees without 
history.

Note that I haven't explicitly designed the notes feature with this in 
mind, so it's wise to add testcases for expected behaviour once we 
start use history-less notes trees.

Thinking about it, the notes code itself (notes.h/.c) only wants a notes 
_tree_ object, so will probably work fine with history-less notes 
trees. But builtin/notes.c with its public commit_notes() function may 
be another story...


...Johan

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

On Mon, Mar 29, 2010 at 04:25:22PM +0200, Johan Herland wrote:

quoted

This is actually inspired by Jeff's novel notes use. I think there
are use cases where a notes log makes sense (notes on commits) and
those where it does not (metadata/textconv). In both cases having a
reflog is useful. So, the next step is really to allow notes trees
without history, which also takes care of the pruning issue. I know
how to do this, I just have to decide about the configuration
options.

I noticed that Jeff's proof-of-concept wrote notes trees without making 
notes commits, and although it seemed like a bug at first, it does - as 
you say - provide a rather nice way to store notes trees without 
history.

No, it was very much intentional.

However, I think the next iteration will wrap the tree in an actual
commit, but just keep each commit parentless. That will provide a nice
spot for metadata like the cache validity information.

I like the idea of having a reflog, just because you could use it to
salvage an old cache if you were playing around with your helper's
options (or debugging your helper :) ). The usual 90-day expiration
time is perhaps too long, though.

Note that I haven't explicitly designed the notes feature with this in 
mind, so it's wise to add testcases for expected behaviour once we 
start use history-less notes trees.

Thinking about it, the notes code itself (notes.h/.c) only wants a notes 
_tree_ object, so will probably work fine with history-less notes 
trees. But builtin/notes.c with its public commit_notes() function may 
be another story...

I was planning on using my own cache-specific helper instead of
commit_notes() anyway, so that shouldn't be a problem. By using a commit
wrapper, I don't think any of the display code should be confused (since
they need to handle the case of a root note commit anyway). Once I have
some example trees, I can poke at them with the existing notes code and
see how they behave (and how we _want_ them to behave, since I'm not
sure yet what sort of cache introspection, if any, would be useful).

-Peff

On Tuesday 30 March 2010, Jeff King wrote:

On Mon, Mar 29, 2010 at 04:25:22PM +0200, Johan Herland wrote:

quoted

quoted

This is actually inspired by Jeff's novel notes use. I think
there are use cases where a notes log makes sense (notes on
commits) and those where it does not (metadata/textconv). In both
cases having a reflog is useful. So, the next step is really to
allow notes trees without history, which also takes care of the
pruning issue. I know how to do this, I just have to decide about
the configuration options.

I noticed that Jeff's proof-of-concept wrote notes trees without
making notes commits, and although it seemed like a bug at first,
it does - as you say - provide a rather nice way to store notes
trees without history.

No, it was very much intentional.

However, I think the next iteration will wrap the tree in an actual
commit, but just keep each commit parentless. That will provide a
nice spot for metadata like the cache validity information.

Agreed.

I like the idea of having a reflog, just because you could use it to
salvage an old cache if you were playing around with your helper's
options (or debugging your helper :) ). The usual 90-day expiration
time is perhaps too long, though.

Yes, 90 days as a default might be excessive, but you can always 
override it with a "git gc --prune=now"...

quoted

Note that I haven't explicitly designed the notes feature with this
in mind, so it's wise to add testcases for expected behaviour once
we start use history-less notes trees.

Thinking about it, the notes code itself (notes.h/.c) only wants a
notes _tree_ object, so will probably work fine with history-less
notes trees. But builtin/notes.c with its public commit_notes()
function may be another story...

I was planning on using my own cache-specific helper instead of
commit_notes() anyway, so that shouldn't be a problem. By using a
commit wrapper, I don't think any of the display code should be
confused (since they need to handle the case of a root note commit
anyway). Once I have some example trees, I can poke at them with the
existing notes code and see how they behave (and how we _want_ them
to behave, since I'm not sure yet what sort of cache introspection,
if any, would be useful).

Looking forward to your patches. :)


...Johan

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

Johan Herland [off-list ref] writes:

On Tuesday 30 March 2010, Jeff King wrote:

quoted

I like the idea of having a reflog, just because you could use it to
salvage an old cache if you were playing around with your helper's
options (or debugging your helper :) ). The usual 90-day expiration
time is perhaps too long, though.

Yes, 90 days as a default might be excessive, but you can always 
override it with a "git gc --prune=now"...

You can always set different expire time for notes by using

  [gc "refs/notes"]
        reflogExpire = 7 # days, I suppose

Which is not documented (I have found it in RelNotes-1.6.0.txt).  
Oh well...
-- 
Jakub Narebski
Poland
ShadeHawk on #git

On Tue, Mar 30, 2010 at 12:18:16PM -0700, Jakub Narebski wrote:

quoted

quoted

I like the idea of having a reflog, just because you could use it to
salvage an old cache if you were playing around with your helper's
options (or debugging your helper :) ). The usual 90-day expiration
time is perhaps too long, though.

Yes, 90 days as a default might be excessive, but you can always 
override it with a "git gc --prune=now"...

You can always set different expire time for notes by using

  [gc "refs/notes"]
        reflogExpire = 7 # days, I suppose

Which is not documented (I have found it in RelNotes-1.6.0.txt).  
Oh well...

Thanks, I didn't know about that feature. I just posted my series
without dealing with the reflogs at all, but I think it may be sensible
to drop the default for "refs/notes/textconv" in a followup patch.

-Peff