{"thread":{"id":"23230","subject":"[PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","startedAt":"2010-03-29T13:05:57Z","lastAt":"2010-04-02T00:16:41Z","messageCount":7,"participants":["Michael J Gruber","Johan Herland","Jeff King","Jakub Narebski"],"isPatch":true,"patchVersion":1,"patchTotal":2},"messages":[{"id":"138084","messageId":"d6c334ec855bf04d9edb432b9cdc3590ab96d6e9.1269867675.git.git@drmicha.warpmail.net","threadId":"23230","inReplyTo":null,"subject":"[PATCH 1/2] t3301-notes: Test the creation of reflog entries","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-03-29T13:05:57Z","receivedAt":"2010-03-29T13:05:57Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"Test whether the notes code writes reflog entries. It intends to\n(setting up the reflog messages) but currently does not.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\n t/t3301-notes.sh |    9 +++++++++\n 1 files changed, 9 insertions(+), 0 deletions(-)\n\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 1d6cd45..5410a6d 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -65,6 +65,15 @@ test_expect_success 'create notes' '\n \ttest_must_fail git notes show HEAD^\n '\n \n+cat >expect <<EOF\n+d423f8c refs/notes/commits@{0}: notes: Notes added by 'git notes add'\n+EOF\n+\n+test_expect_failure 'create reflog entry' '\n+\tgit reflog show refs/notes/commits >output &&\n+\ttest_cmp expect output\n+'\n+\n test_expect_success 'edit existing notes' '\n \tMSG=b3 git notes edit &&\n \ttest ! -f .git/NOTES_EDITMSG &&\n-- \n1.7.0.3.448.g82eeb\n"},{"id":"138083","messageId":"0f06a30f395c14307066d0e4365fc61a294997db.1269867675.git.git@drmicha.warpmail.net","threadId":"23230","inReplyTo":"d6c334ec855bf04d9edb432b9cdc3590ab96d6e9.1269867675.git.git@drmicha.warpmail.net","subject":"[PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Michael J Gruber","fromEmail":"git@drmicha.warpmail.net","sentAt":"2010-03-29T13:05:58Z","receivedAt":"2010-03-29T13:05:58Z","isPatch":true,"sender":{"key":"git@grubix.eu","avatar":"https://avatars.githubusercontent.com/u/233215?v=4"},"body":"The notes code intends to write reflog entries, but currently they are\nnot written because log_ref_write() checks for the refname path\nexplicitly.\n\nAdd refs/notes to the list of allowed paths so that notes references are\ntreated just like branch heads, i.e. according to core.logAllRefUpdates\nand core.bare.\n\nSigned-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n---\nThis is actually inspired by Jeff's novel notes use. I think there are\nuse cases where a notes log makes sense (notes on commits) and those\nwhere it does not (metadata/textconv). In both cases having a reflog is\nuseful. So, the next step is really to allow notes trees without\nhistory, which also takes care of the pruning issue. I know how to do this,\nI just have to decide about the configuration options.\n\n refs.c           |    1 +\n t/t3301-notes.sh |    2 +-\n 2 files changed, 2 insertions(+), 1 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex 0f24c8d..d3db15a 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -1276,6 +1276,7 @@ static int log_ref_write(const char *ref_name, const unsigned char *old_sha1,\n \tif (log_all_ref_updates &&\n \t    (!prefixcmp(ref_name, \"refs/heads/\") ||\n \t     !prefixcmp(ref_name, \"refs/remotes/\") ||\n+\t     !prefixcmp(ref_name, \"refs/notes/\") ||\n \t     !strcmp(ref_name, \"HEAD\"))) {\n \t\tif (safe_create_leading_directories(log_file) < 0)\n \t\t\treturn error(\"unable to create directory for %s\",\ndiff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\nindex 5410a6d..b2e7b07 100755\n--- a/t/t3301-notes.sh\n+++ b/t/t3301-notes.sh\n@@ -69,7 +69,7 @@ cat >expect <<EOF\n d423f8c refs/notes/commits@{0}: notes: Notes added by 'git notes add'\n EOF\n \n-test_expect_failure 'create reflog entry' '\n+test_expect_success 'create reflog entry' '\n \tgit reflog show refs/notes/commits >output &&\n \ttest_cmp expect output\n '\n-- \n1.7.0.3.448.g82eeb\n"},{"id":"138091","messageId":"201003291625.22977.johan@herland.net","threadId":"23230","inReplyTo":"0f06a30f395c14307066d0e4365fc61a294997db.1269867675.git.git@drmicha.warpmail.net","subject":"Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-03-29T14:25:22Z","receivedAt":"2010-03-29T14:25:22Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Monday 29 March 2010, Michael J Gruber wrote:\n> The notes code intends to write reflog entries, but currently they\n> are not written because log_ref_write() checks for the refname path\n> explicitly.\n>\n> Add refs/notes to the list of allowed paths so that notes references\n> are treated just like branch heads, i.e. according to\n> core.logAllRefUpdates and core.bare.\n>\n> Signed-off-by: Michael J Gruber <git@drmicha.warpmail.net>\n\nBoth patches are\n\nAcked-by: Johan Herland <johan@herland.net>\n\n> ---\n> This is actually inspired by Jeff's novel notes use. I think there\n> are use cases where a notes log makes sense (notes on commits) and\n> those where it does not (metadata/textconv). In both cases having a\n> reflog is useful. So, the next step is really to allow notes trees\n> without history, which also takes care of the pruning issue. I know\n> how to do this, I just have to decide about the configuration\n> options.\n\nI noticed that Jeff's proof-of-concept wrote notes trees without making \nnotes commits, and although it seemed like a bug at first, it does - as \nyou say - provide a rather nice way to store notes trees without \nhistory.\n\nNote that I haven't explicitly designed the notes feature with this in \nmind, so it's wise to add testcases for expected behaviour once we \nstart use history-less notes trees.\n\nThinking about it, the notes code itself (notes.h/.c) only wants a notes \n_tree_ object, so will probably work fine with history-less notes \ntrees. But builtin/notes.c with its public commit_notes() function may \nbe another story...\n\n\n...Johan\n\n\n>  refs.c           |    1 +\n>  t/t3301-notes.sh |    2 +-\n>  2 files changed, 2 insertions(+), 1 deletions(-)\n>\n> diff --git a/refs.c b/refs.c\n> index 0f24c8d..d3db15a 100644\n> --- a/refs.c\n> +++ b/refs.c\n> @@ -1276,6 +1276,7 @@ static int log_ref_write(const char *ref_name,\n> const unsigned char *old_sha1, if (log_all_ref_updates &&\n>  \t    (!prefixcmp(ref_name, \"refs/heads/\") ||\n>  \t     !prefixcmp(ref_name, \"refs/remotes/\") ||\n> +\t     !prefixcmp(ref_name, \"refs/notes/\") ||\n>  \t     !strcmp(ref_name, \"HEAD\"))) {\n>  \t\tif (safe_create_leading_directories(log_file) < 0)\n>  \t\t\treturn error(\"unable to create directory for %s\",\n> diff --git a/t/t3301-notes.sh b/t/t3301-notes.sh\n> index 5410a6d..b2e7b07 100755\n> --- a/t/t3301-notes.sh\n> +++ b/t/t3301-notes.sh\n> @@ -69,7 +69,7 @@ cat >expect <<EOF\n>  d423f8c refs/notes/commits@{0}: notes: Notes added by 'git notes\n> add' EOF\n>\n> -test_expect_failure 'create reflog entry' '\n> +test_expect_success 'create reflog entry' '\n>  \tgit reflog show refs/notes/commits >output &&\n>  \ttest_cmp expect output\n>  '\n\n\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"138193","messageId":"20100330171932.GE17763@coredump.intra.peff.net","threadId":"23230","inReplyTo":"201003291625.22977.johan@herland.net","subject":"Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-03-30T17:19:32Z","receivedAt":"2010-03-30T17:19:32Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Mon, Mar 29, 2010 at 04:25:22PM +0200, Johan Herland wrote:\n\n> > This is actually inspired by Jeff's novel notes use. I think there\n> > are use cases where a notes log makes sense (notes on commits) and\n> > those where it does not (metadata/textconv). In both cases having a\n> > reflog is useful. So, the next step is really to allow notes trees\n> > without history, which also takes care of the pruning issue. I know\n> > how to do this, I just have to decide about the configuration\n> > options.\n> \n> I noticed that Jeff's proof-of-concept wrote notes trees without making \n> notes commits, and although it seemed like a bug at first, it does - as \n> you say - provide a rather nice way to store notes trees without \n> history.\n\nNo, it was very much intentional.\n\nHowever, I think the next iteration will wrap the tree in an actual\ncommit, but just keep each commit parentless. That will provide a nice\nspot for metadata like the cache validity information.\n\nI like the idea of having a reflog, just because you could use it to\nsalvage an old cache if you were playing around with your helper's\noptions (or debugging your helper :) ). The usual 90-day expiration\ntime is perhaps too long, though.\n\n> Note that I haven't explicitly designed the notes feature with this in \n> mind, so it's wise to add testcases for expected behaviour once we \n> start use history-less notes trees.\n> \n> Thinking about it, the notes code itself (notes.h/.c) only wants a notes \n> _tree_ object, so will probably work fine with history-less notes \n> trees. But builtin/notes.c with its public commit_notes() function may \n> be another story...\n\nI was planning on using my own cache-specific helper instead of\ncommit_notes() anyway, so that shouldn't be a problem. By using a commit\nwrapper, I don't think any of the display code should be confused (since\nthey need to handle the case of a root note commit anyway). Once I have\nsome example trees, I can poke at them with the existing notes code and\nsee how they behave (and how we _want_ them to behave, since I'm not\nsure yet what sort of cache introspection, if any, would be useful).\n\n-Peff\n"},{"id":"138198","messageId":"201003302000.35616.johan@herland.net","threadId":"23230","inReplyTo":"20100330171932.GE17763@coredump.intra.peff.net","subject":"Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2010-03-30T18:00:35Z","receivedAt":"2010-03-30T18:00:35Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Tuesday 30 March 2010, Jeff King wrote:\n> On Mon, Mar 29, 2010 at 04:25:22PM +0200, Johan Herland wrote:\n> > > This is actually inspired by Jeff's novel notes use. I think\n> > > there are use cases where a notes log makes sense (notes on\n> > > commits) and those where it does not (metadata/textconv). In both\n> > > cases having a reflog is useful. So, the next step is really to\n> > > allow notes trees without history, which also takes care of the\n> > > pruning issue. I know how to do this, I just have to decide about\n> > > the configuration options.\n> >\n> > I noticed that Jeff's proof-of-concept wrote notes trees without\n> > making notes commits, and although it seemed like a bug at first,\n> > it does - as you say - provide a rather nice way to store notes\n> > trees without history.\n>\n> No, it was very much intentional.\n>\n> However, I think the next iteration will wrap the tree in an actual\n> commit, but just keep each commit parentless. That will provide a\n> nice spot for metadata like the cache validity information.\n\nAgreed.\n\n> I like the idea of having a reflog, just because you could use it to\n> salvage an old cache if you were playing around with your helper's\n> options (or debugging your helper :) ). The usual 90-day expiration\n> time is perhaps too long, though.\n\nYes, 90 days as a default might be excessive, but you can always \noverride it with a \"git gc --prune=now\"...\n\n> > Note that I haven't explicitly designed the notes feature with this\n> > in mind, so it's wise to add testcases for expected behaviour once\n> > we start use history-less notes trees.\n> >\n> > Thinking about it, the notes code itself (notes.h/.c) only wants a\n> > notes _tree_ object, so will probably work fine with history-less\n> > notes trees. But builtin/notes.c with its public commit_notes()\n> > function may be another story...\n>\n> I was planning on using my own cache-specific helper instead of\n> commit_notes() anyway, so that shouldn't be a problem. By using a\n> commit wrapper, I don't think any of the display code should be\n> confused (since they need to handle the case of a root note commit\n> anyway). Once I have some example trees, I can poke at them with the\n> existing notes code and see how they behave (and how we _want_ them\n> to behave, since I'm not sure yet what sort of cache introspection,\n> if any, would be useful).\n\nLooking forward to your patches. :)\n\n\n...Johan\n\n-- \nJohan Herland, <johan@herland.net>\nwww.herland.net\n"},{"id":"138204","messageId":"m3mxxpipqw.fsf@localhost.localdomain","threadId":"23230","inReplyTo":"201003302000.35616.johan@herland.net","subject":"Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Jakub Narebski","fromEmail":"jnareb@gmail.com","sentAt":"2010-03-30T19:18:16Z","receivedAt":"2010-03-30T19:18:16Z","isPatch":true,"sender":{"key":"jnareb@gmail.com","avatar":"https://avatars.githubusercontent.com/u/2706?v=4"},"body":"Johan Herland <johan@herland.net> writes:\n> On Tuesday 30 March 2010, Jeff King wrote:\n\n> > I like the idea of having a reflog, just because you could use it to\n> > salvage an old cache if you were playing around with your helper's\n> > options (or debugging your helper :) ). The usual 90-day expiration\n> > time is perhaps too long, though.\n> \n> Yes, 90 days as a default might be excessive, but you can always \n> override it with a \"git gc --prune=now\"...\n\nYou can always set different expire time for notes by using\n\n  [gc \"refs/notes\"]\n        reflogExpire = 7 # days, I suppose\n\nWhich is not documented (I have found it in RelNotes-1.6.0.txt).  \nOh well...\n-- \nJakub Narebski\nPoland\nShadeHawk on #git\n"},{"id":"138380","messageId":"20100402001640.GH16462@coredump.intra.peff.net","threadId":"23230","inReplyTo":"m3mxxpipqw.fsf@localhost.localdomain","subject":"Re: [PATCH 2/2] refs.c: Write reflogs for notes just like for branch heads","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2010-04-02T00:16:41Z","receivedAt":"2010-04-02T00:16:41Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, Mar 30, 2010 at 12:18:16PM -0700, Jakub Narebski wrote:\n\n> > > I like the idea of having a reflog, just because you could use it to\n> > > salvage an old cache if you were playing around with your helper's\n> > > options (or debugging your helper :) ). The usual 90-day expiration\n> > > time is perhaps too long, though.\n> > \n> > Yes, 90 days as a default might be excessive, but you can always \n> > override it with a \"git gc --prune=now\"...\n> \n> You can always set different expire time for notes by using\n> \n>   [gc \"refs/notes\"]\n>         reflogExpire = 7 # days, I suppose\n> \n> Which is not documented (I have found it in RelNotes-1.6.0.txt).  \n> Oh well...\n\nThanks, I didn't know about that feature. I just posted my series\nwithout dealing with the reflogs at all, but I think it may be sensible\nto drop the default for \"refs/notes/textconv\" in a followup patch.\n\n-Peff\n"}]}