# [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads

7 messages from 2010-03-29 to 2010-04-02. Participants: Michael J Gruber, Johan Herland, Jeff King, Jakub Narebski.
Thread: https://gitlist.dev/t/23230

## Michael J Gruber, 2010-03-29 13:05

Subject: [PATCH 1/2] t3301-notes: Test the creation of reflog entries
Message-ID: <d6c334ec855bf04d9edb432b9cdc3590ab96d6e9.1269867675.git.git@drmicha.warpmail.net>
URL: https://gitlist.dev/e/d6c334ec855bf04d9edb432b9cdc3590ab96d6e9.1269867675.git.git%40drmicha.warpmail.net

```
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 <git@drmicha.warpmail.net>
---
 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

```

## Michael J Gruber, 2010-03-29 13:05

Subject: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <0f06a30f395c14307066d0e4365fc61a294997db.1269867675.git.git@drmicha.warpmail.net>
URL: https://gitlist.dev/e/0f06a30f395c14307066d0e4365fc61a294997db.1269867675.git.git%40drmicha.warpmail.net
In-Reply-To: <d6c334ec855bf04d9edb432b9cdc3590ab96d6e9.1269867675.git.git@drmicha.warpmail.net>

```
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 <git@drmicha.warpmail.net>
---
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

```

## Johan Herland, 2010-03-29 14:25

Subject: Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <201003291625.22977.johan@herland.net>
URL: https://gitlist.dev/e/201003291625.22977.johan%40herland.net
In-Reply-To: <0f06a30f395c14307066d0e4365fc61a294997db.1269867675.git.git@drmicha.warpmail.net>

```
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 <git@drmicha.warpmail.net>

Both patches are

Acked-by: Johan Herland <johan@herland.net>

> ---
> 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


>  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
>  '



-- 
Johan Herland, <johan@herland.net>
www.herland.net

```

## Jeff King, 2010-03-30 17:19

Subject: Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <20100330171932.GE17763@coredump.intra.peff.net>
URL: https://gitlist.dev/e/20100330171932.GE17763%40coredump.intra.peff.net
In-Reply-To: <201003291625.22977.johan@herland.net>

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

> > 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

```

## Johan Herland, 2010-03-30 18:00

Subject: Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <201003302000.35616.johan@herland.net>
URL: https://gitlist.dev/e/201003302000.35616.johan%40herland.net
In-Reply-To: <20100330171932.GE17763@coredump.intra.peff.net>

```
On Tuesday 30 March 2010, Jeff King wrote:
> On Mon, Mar 29, 2010 at 04:25:22PM +0200, Johan Herland wrote:
> > > 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"...

> > 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, <johan@herland.net>
www.herland.net

```

## Jakub Narebski, 2010-03-30 19:18

Subject: Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <m3mxxpipqw.fsf@localhost.localdomain>
URL: https://gitlist.dev/e/m3mxxpipqw.fsf%40localhost.localdomain
In-Reply-To: <201003302000.35616.johan@herland.net>

```
Johan Herland <johan@herland.net> writes:
> On Tuesday 30 March 2010, Jeff King wrote:

> > 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

```

## Jeff King, 2010-04-02 00:16

Subject: Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads
Message-ID: <20100402001640.GH16462@coredump.intra.peff.net>
URL: https://gitlist.dev/e/20100402001640.GH16462%40coredump.intra.peff.net
In-Reply-To: <m3mxxpipqw.fsf@localhost.localdomain>

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

> > > 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

```