{"thread":{"id":"30469","subject":"[PATCH] sha1_name: support sha1^{note} to return note sha-1","startedAt":"2012-05-08T13:14:30Z","lastAt":"2012-05-09T17:26:47Z","messageCount":5,"participants":["Nguyễn Thái Ngọc Duy","Jeff King","Nguyen Thai Ngoc Duy","Johan Herland"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"191088","messageId":"1336482870-30842-1-git-send-email-pclouds@gmail.com","threadId":"30469","inReplyTo":null,"subject":"[PATCH] sha1_name: support sha1^{note} to return note sha-1","fromName":"Nguyễn Thái Ngọc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-08T13:14:30Z","receivedAt":"2012-05-08T13:14:30Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"\nSigned-off-by: Nguyễn Thái Ngọc Duy \n---\n It may be useful for scripting, and looks nice. Though may be not\n worth adding if there are no actual users.\n\n Documentation/revisions.txt | 1 +\n sha1_name.c | 15 ++++++++++++++-\n 2 files changed, 15 insertions(+), 1 deletions(-)\n\ndiff --git a/Documentation/revisions.txt b/Documentation/revisions.txt\nindex 1725661..5a671fe 100644\n--- a/Documentation/revisions.txt\n+++ b/Documentation/revisions.txt\n@@ -114,6 +114,7 @@ the '$GIT_DIR/refs' directory or from the '$GIT_DIR/packed-refs' file.\n object of that type is found or the object cannot be\n dereferenced anymore (in which case, barf). '{caret}0'\n is a short-hand for '{caret}\\{commit\\}'.\n+ A special type \"note\" can be used to return the note object.\n \n '{caret}\\{\\}', e.g. 'v0.99.8{caret}\\{\\}'::\n A suffix '{caret}' followed by an empty brace pair\ndiff --git a/sha1_name.c b/sha1_name.c\nindex c633113..02d28df 100644\n--- a/sha1_name.c\n+++ b/sha1_name.c\n@@ -6,6 +6,7 @@\n #include \"tree-walk.h\"\n #include \"refs.h\"\n #include \"remote.h\"\n+#include \"notes.h\"\n \n static int get_sha1_oneline(const char *, unsigned char *, struct commit_list *);\n \n@@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n \t\texpected_type = OBJ_NONE;\n \telse if (sp[0] == '/')\n \t\texpected_type = OBJ_COMMIT;\n-\telse\n+\telse if (!strncmp(\"note}\", sp, 5)) {\n+\t\tconst unsigned char *note;\n+\t\tstruct notes_tree t;\n+\t\tif (get_sha1_1(name, sp - name - 2, outer))\n+\t\t\treturn -1;\n+\t\tmemset(&t, 0, sizeof(t));\n+\t\tinit_notes(&t, NULL, NULL, 0);\n+\t\tnote = get_note(&t, outer);\n+\t\tif (note)\n+\t\t\thashcpy(sha1, note);\n+\t\tfree_notes(&t);\n+\t\treturn note ? 0 : -1;\n+\t} else\n \t\treturn -1;\n \n \tif (get_sha1_1(name, sp - name - 2, outer))\n-- \n1.7.8.36.g69ee2\n"},{"id":"191108","messageId":"20120508161126.GB26838@sigill.intra.peff.net","threadId":"30469","inReplyTo":"1336482870-30842-1-git-send-email-pclouds@gmail.com","subject":"Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-08T16:11:26Z","receivedAt":"2012-05-08T16:11:26Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote:\n\n> It may be useful for scripting, and looks nice. Though may be not\n> worth adding if there are no actual users.\n\nThere can be many notes refs. So I think to do this right, you would\nwant something like:\n\n foo^{note:bar}\n\nwhich would look in refs/notes/bar (this logic is handled by\nexpand_notes_ref). And \"foo^{note}\" would be a synonym for the default\nnote ref.\n\n> @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n> \t\texpected_type = OBJ_NONE;\n> \telse if (sp[0] == '/')\n> \t\texpected_type = OBJ_COMMIT;\n> -\telse\n> +\telse if (!strncmp(\"note}\", sp, 5)) {\n> +\t\tconst unsigned char *note;\n> +\t\tstruct notes_tree t;\n> +\t\tif (get_sha1_1(name, sp - name - 2, outer))\n> +\t\t\treturn -1;\n> +\t\tmemset(&t, 0, sizeof(t));\n> +\t\tinit_notes(&t, NULL, NULL, 0);\n> +\t\tnote = get_note(&t, outer);\n> +\t\tif (note)\n> +\t\t\thashcpy(sha1, note);\n> +\t\tfree_notes(&t);\n> +\t\treturn note ? 0 : -1;\n> +\t} else\n\nThe notes code is relatively expensive to initialize, with the\nassumption that the effort will be amortized across multiple lookups\n(which are made faster). Is it possible to cache this initialized notes\nstructure in case many lookups are done?\n\nIt may not be as important as it used to be, either. I think more recent\nversions of the code will progressively load the notes tree rather than\nfilling it all in at initialization time. But it has been a while since\nI've done anything with notes.\n\n-Peff\n"},{"id":"191186","messageId":"CACsJy8AfD-N6L6nZNDG3uw4s6jAdYqkuQ8Jt2XRXRGzRwt1ncQ@mail.gmail.com","threadId":"30469","inReplyTo":"20120508161126.GB26838@sigill.intra.peff.net","subject":"Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1","fromName":"Nguyen Thai Ngoc Duy","fromEmail":"pclouds@gmail.com","sentAt":"2012-05-09T08:25:15Z","receivedAt":"2012-05-09T08:25:15Z","isPatch":true,"sender":{"key":"pclouds@gmail.com","avatar":"https://avatars.githubusercontent.com/u/720?v=4"},"body":"On Tue, May 8, 2012 at 11:11 PM, Jeff King wrote:\n> On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote:\n>\n>>  It may be useful for scripting, and looks nice. Though may be not\n>>  worth adding if there are no actual users.\n>\n> There can be many notes refs. So I think to do this right, you would\n> want something like:\n>\n>  foo^{note:bar}\n>\n> which would look in refs/notes/bar (this logic is handled by\n> expand_notes_ref). And \"foo^{note}\" would be a synonym for the default\n> note ref.\n\nRight. Thanks.\n\n>> @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n>>               expected_type = OBJ_NONE;\n>>       else if (sp[0] == '/')\n>>               expected_type = OBJ_COMMIT;\n>> -     else\n>> +     else if (!strncmp(\"note}\", sp, 5)) {\n>> +             const unsigned char *note;\n>> +             struct notes_tree t;\n>> +             if (get_sha1_1(name, sp - name - 2, outer))\n>> +                     return -1;\n>> +             memset(&t, 0, sizeof(t));\n>> +             init_notes(&t, NULL, NULL, 0);\n>> +             note = get_note(&t, outer);\n>> +             if (note)\n>> +                     hashcpy(sha1, note);\n>> +             free_notes(&t);\n>> +             return note ? 0 : -1;\n>> +     } else\n>\n> The notes code is relatively expensive to initialize, with the\n> assumption that the effort will be amortized across multiple lookups\n> (which are made faster). Is it possible to cache this initialized notes\n> structure in case many lookups are done?\n\nNo idea. I have never worked/used notes until yesterday (and these\npatches were the result). I will look into it.\n\n> It may not be as important as it used to be, either. I think more recent\n> versions of the code will progressively load the notes tree rather than\n> filling it all in at initialization time. But it has been a while since\n> I've done anything with notes.\n-- \nDuy\n"},{"id":"191197","messageId":"CALKQrgdGCoKooM7KayNMQ_+Fg9=-1YnwNE_9NHzpX0jfoRjT_A@mail.gmail.com","threadId":"30469","inReplyTo":"CACsJy8AfD-N6L6nZNDG3uw4s6jAdYqkuQ8Jt2XRXRGzRwt1ncQ@mail.gmail.com","subject":"Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1","fromName":"Johan Herland","fromEmail":"johan@herland.net","sentAt":"2012-05-09T14:09:35Z","receivedAt":"2012-05-09T14:09:35Z","isPatch":true,"sender":{"key":"johan@herland.net","avatar":"https://avatars.githubusercontent.com/u/547031?v=4"},"body":"On Wed, May 9, 2012 at 10:25 AM, Nguyen Thai Ngoc Duy wrote:\n> On Tue, May 8, 2012 at 11:11 PM, Jeff King wrote:\n>> On Tue, May 08, 2012 at 08:14:30PM +0700, Nguyen Thai Ngoc Duy wrote:\n>>> @@ -473,7 +474,19 @@ static int peel_onion(const char *name, int len, unsigned char *sha1)\n>>>               expected_type = OBJ_NONE;\n>>>       else if (sp[0] == '/')\n>>>               expected_type = OBJ_COMMIT;\n>>> -     else\n>>> +     else if (!strncmp(\"note}\", sp, 5)) {\n>>> +             const unsigned char *note;\n>>> +             struct notes_tree t;\n>>> +             if (get_sha1_1(name, sp - name - 2, outer))\n>>> +                     return -1;\n>>> +             memset(&t, 0, sizeof(t));\n>>> +             init_notes(&t, NULL, NULL, 0);\n>>> +             note = get_note(&t, outer);\n>>> +             if (note)\n>>> +                     hashcpy(sha1, note);\n>>> +             free_notes(&t);\n>>> +             return note ? 0 : -1;\n>>> +     } else\n>>\n>> The notes code is relatively expensive to initialize, with the\n>> assumption that the effort will be amortized across multiple lookups\n>> (which are made faster). Is it possible to cache this initialized notes\n>> structure in case many lookups are done?\n>\n> No idea. I have never worked/used notes until yesterday (and these\n> patches were the result). I will look into it.\n\nIIRC, the initialization loads the top-level notes tree object into\nmemory. Subtrees (if any) are loaded on demand. FTR, if you have less\nthen ~256 notes in the notes tree, there will be no subtrees. As the\nnumber of notes grows, the number of subtree levels grow roughly\nlogarithmically with the total number of notes (see determine_fanout()\nfor more details).\n\n...Johan\n\n-- \nJohan Herland, \nwww.herland.net\n"},{"id":"191203","messageId":"20120509172647.GA30487@sigill.intra.peff.net","threadId":"30469","inReplyTo":"CALKQrgdGCoKooM7KayNMQ_+Fg9=-1YnwNE_9NHzpX0jfoRjT_A@mail.gmail.com","subject":"Re: [PATCH] sha1_name: support sha1^{note} to return note sha-1","fromName":"Jeff King","fromEmail":"peff@peff.net","sentAt":"2012-05-09T17:26:47Z","receivedAt":"2012-05-09T17:26:47Z","isPatch":true,"sender":{"key":"peff@peff.net","avatar":"https://avatars.githubusercontent.com/u/45925?v=4"},"body":"On Wed, May 09, 2012 at 04:09:35PM +0200, Johan Herland wrote:\n\n> > No idea. I have never worked/used notes until yesterday (and these\n> > patches were the result). I will look into it.\n> \n> IIRC, the initialization loads the top-level notes tree object into\n> memory. Subtrees (if any) are loaded on demand. FTR, if you have less\n> then ~256 notes in the notes tree, there will be no subtrees. As the\n> number of notes grows, the number of subtree levels grow roughly\n> logarithmically with the total number of notes (see determine_fanout()\n> for more details).\n\nAh, right. I was thinking back to my original crappy implementation that\ndidn't do fanout. So I don't think an init is that bad. It does look up\nthe ref each time, but it will only load the top-level tree object\n(which you would need to do a lookup anyway).\n\nSo it is probably OK to just do an init/lookup/free each time if it\nmakes the code simpler (and I think it does). We probably won't be\nlooking up sha1^{note} in a tight loop, anyway, since sha1 expressions\nlike that generally come from the command line.\n\n-Peff\n"}]}