# [PATCH] sha1_name: support sha1^{note} to return note sha-1 5 messages from 2012-05-08 to 2012-05-09. Participants: Nguyễn Thái Ngọc Duy, Jeff King, Nguyen Thai Ngoc Duy, Johan Herland. Thread: https://gitlist.dev/t/30469 ## Nguyễn Thái Ngọc Duy, 2012-05-08 13:14 Subject: [PATCH] sha1_name: support sha1^{note} to return note sha-1 Message-ID: <1336482870-30842-1-git-send-email-pclouds@gmail.com> URL: https://gitlist.dev/e/1336482870-30842-1-git-send-email-pclouds%40gmail.com ``` Signed-off-by: Nguyễn Thái Ngọc Duy --- It may be useful for scripting, and looks nice. Though may be not worth adding if there are no actual users. Documentation/revisions.txt | 1 + sha1_name.c | 15 ++++++++++++++- 2 files changed, 15 insertions(+), 1 deletions(-) diff --git a/Documentation/revisions.txt b/Documentation/revisions.txt index 1725661..5a671fe 100644 --- a/Documentation/revisions.txt +++ b/Documentation/revisions.txt @@ -114,6 +114,7 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file. object of that type is found or the object cannot be dereferenced anymore (in which case, barf). '{caret}0' is a short-hand for '{caret}\{commit\}'. + A special type "note" can be used to return the note object. '{caret}\{\}', e.g. 'v0.99.8{caret}\{\}':: A suffix '{caret}' followed by an empty brace pair diff --git a/sha1_name.c b/sha1_name.c index c633113..02d28df 100644 --- a/sha1_name.c +++ b/sha1_name.c @@ -6,6 +6,7 @@ #include "tree-walk.h" #include "refs.h" #include "remote.h" +#include "notes.h" static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *); @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) expected_type = OBJ_NONE; else if (sp[0] == '/') expected_type = OBJ_COMMIT; - else + else if (!strncmp("note}", sp, 5)) { + const unsigned char *note; + struct notes_tree t; + if (get_sha1_1(name, sp - name - 2, outer)) + return -1; + memset(&t, 0, sizeof(t)); + init_notes(&t, NULL, NULL, 0); + note = get_note(&t, outer); + if (note) + hashcpy(sha1, note); + free_notes(&t); + return note ? 0 : -1; + } else return -1; if (get_sha1_1(name, sp - name - 2, outer)) -- 1.7.8.36.g69ee2 ``` ## Jeff King, 2012-05-08 16:11 Subject: Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1 Message-ID: <20120508161126.GB26838@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120508161126.GB26838%40sigill.intra.peff.net In-Reply-To: <1336482870-30842-1-git-send-email-pclouds@gmail.com> ``` On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote: > It may be useful for scripting, and looks nice. Though may be not > worth adding if there are no actual users. There can be many notes refs. So I think to do this right, you would want something like: foo^{note:bar} which would look in refs/notes/bar (this logic is handled by expand_notes_ref). And "foo^{note}" would be a synonym for the default note ref. > @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) > expected_type = OBJ_NONE; > else if (sp[0] == '/') > expected_type = OBJ_COMMIT; > - else > + else if (!strncmp("note}", sp, 5)) { > + const unsigned char *note; > + struct notes_tree t; > + if (get_sha1_1(name, sp - name - 2, outer)) > + return -1; > + memset(&t, 0, sizeof(t)); > + init_notes(&t, NULL, NULL, 0); > + note = get_note(&t, outer); > + if (note) > + hashcpy(sha1, note); > + free_notes(&t); > + return note ? 0 : -1; > + } else The notes code is relatively expensive to initialize, with the assumption that the effort will be amortized across multiple lookups (which are made faster). Is it possible to cache this initialized notes structure in case many lookups are done? It may not be as important as it used to be, either. I think more recent versions of the code will progressively load the notes tree rather than filling it all in at initialization time. But it has been a while since I've done anything with notes. -Peff ``` ## Nguyen Thai Ngoc Duy, 2012-05-09 08:25 Subject: Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1 Message-ID: URL: https://gitlist.dev/e/CACsJy8AfD-N6L6nZNDG3uw4s6jAdYqkuQ8Jt2XRXRGzRwt1ncQ%40mail.gmail.com In-Reply-To: <20120508161126.GB26838@sigill.intra.peff.net> ``` On Tue, May 8, 2012 at 11:11 PM, Jeff King wrote: > On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote: > >>  It may be useful for scripting, and looks nice. Though may be not >>  worth adding if there are no actual users. > > There can be many notes refs. So I think to do this right, you would > want something like: > >  foo^{note:bar} > > which would look in refs/notes/bar (this logic is handled by > expand_notes_ref). And "foo^{note}" would be a synonym for the default > note ref. Right. Thanks. >> @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) >>               expected_type = OBJ_NONE; >>       else if (sp[0] == '/') >>               expected_type = OBJ_COMMIT; >> -     else >> +     else if (!strncmp("note}", sp, 5)) { >> +             const unsigned char *note; >> +             struct notes_tree t; >> +             if (get_sha1_1(name, sp - name - 2, outer)) >> +                     return -1; >> +             memset(&t, 0, sizeof(t)); >> +             init_notes(&t, NULL, NULL, 0); >> +             note = get_note(&t, outer); >> +             if (note) >> +                     hashcpy(sha1, note); >> +             free_notes(&t); >> +             return note ? 0 : -1; >> +     } else > > The notes code is relatively expensive to initialize, with the > assumption that the effort will be amortized across multiple lookups > (which are made faster). Is it possible to cache this initialized notes > structure in case many lookups are done? No idea. I have never worked/used notes until yesterday (and these patches were the result). I will look into it. > It may not be as important as it used to be, either. I think more recent > versions of the code will progressively load the notes tree rather than > filling it all in at initialization time. But it has been a while since > I've done anything with notes. -- Duy ``` ## Johan Herland, 2012-05-09 14:09 Subject: Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1 Message-ID: URL: https://gitlist.dev/e/CALKQrgdGCoKooM7KayNMQ_%2BFg9%3D-1YnwNE_9NHzpX0jfoRjT_A%40mail.gmail.com In-Reply-To: ``` On Wed, May 9, 2012 at 10:25 AM, Nguyen Thai Ngoc Duy wrote: > On Tue, May 8, 2012 at 11:11 PM, Jeff King wrote: >> On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote: >>> @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1) >>>               expected_type = OBJ_NONE; >>>       else if (sp[0] == '/') >>>               expected_type = OBJ_COMMIT; >>> -     else >>> +     else if (!strncmp("note}", sp, 5)) { >>> +             const unsigned char *note; >>> +             struct notes_tree t; >>> +             if (get_sha1_1(name, sp - name - 2, outer)) >>> +                     return -1; >>> +             memset(&t, 0, sizeof(t)); >>> +             init_notes(&t, NULL, NULL, 0); >>> +             note = get_note(&t, outer); >>> +             if (note) >>> +                     hashcpy(sha1, note); >>> +             free_notes(&t); >>> +             return note ? 0 : -1; >>> +     } else >> >> The notes code is relatively expensive to initialize, with the >> assumption that the effort will be amortized across multiple lookups >> (which are made faster). Is it possible to cache this initialized notes >> structure in case many lookups are done? > > No idea. I have never worked/used notes until yesterday (and these > patches were the result). I will look into it. IIRC, the initialization loads the top-level notes tree object into memory. Subtrees (if any) are loaded on demand. FTR, if you have less then ~256 notes in the notes tree, there will be no subtrees. As the number of notes grows, the number of subtree levels grow roughly logarithmically with the total number of notes (see determine_fanout() for more details). ...Johan -- Johan Herland, www.herland.net ``` ## Jeff King, 2012-05-09 17:26 Subject: Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1 Message-ID: <20120509172647.GA30487@sigill.intra.peff.net> URL: https://gitlist.dev/e/20120509172647.GA30487%40sigill.intra.peff.net In-Reply-To: ``` On Wed, May 09, 2012 at 04:09:35PM +0200, Johan Herland wrote: > > No idea. I have never worked/used notes until yesterday (and these > > patches were the result). I will look into it. > > IIRC, the initialization loads the top-level notes tree object into > memory. Subtrees (if any) are loaded on demand. FTR, if you have less > then ~256 notes in the notes tree, there will be no subtrees. As the > number of notes grows, the number of subtree levels grow roughly > logarithmically with the total number of notes (see determine_fanout() > for more details). Ah, right. I was thinking back to my original crappy implementation that didn't do fanout. So I don't think an init is that bad. It does look up the ref each time, but it will only load the top-level tree object (which you would need to do a lookup anyway). So it is probably OK to just do an init/lookup/free each time if it makes the code simpler (and I think it does). We probably won't be looking up sha1^{note} in a tight loop, anyway, since sha1 expressions like that generally come from the command line. -Peff ```