{"thread":{"id":"43405","subject":"Re: [PATCH/RFC] avoid accessing _all_ loose refs in git-show-ref","startedAt":"2006-12-16T12:36:43Z","lastAt":"2006-12-16T14:20:57Z","messageCount":3,"participants":["Junio C Hamano","Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"295519","messageId":"Pine.LNX.4.63.0612161335140.3635@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43405","inReplyTo":null,"subject":"[PATCH/RFC] avoid accessing _all_ loose refs in git-show-ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-16T12:36:43Z","receivedAt":"2006-12-16T12:36:43Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nIf you want to verify a ref, it is overkill to first read all loose refs\ninto a linked list, and then check if the desired ref is there.\n\nSigned-off-by: Johannes Schindelin \n\n---\n\n\tThis is kind of quick and dirty.\n\n\tAn alternative would be to pack the tags _per default_. I once\n\targued for that, but it appears nobody liked that idea. I even\n\tproposed to pack _all_ refs, and I still think this would be\n\ta good idea.\n\n\tHowever, HTTP/FTP transport would have to learn about that long\n\tbefore, and/or a config option would be needed to allow public\n\trepos to cater for older clients.\n\n\tIsn't it a bug that --verify succeeds, if only _one_ ref passed to \n\tthe command exists?\n\n builtin-show-ref.c | 14 +++++++++++++-\n refs.c | 17 +++++++++++------\n refs.h | 1 +\n 3 files changed, 25 insertions(+), 7 deletions(-)\n\ndiff --git a/builtin-show-ref.c b/builtin-show-ref.c\nindex 0739798..b57b074 100644\n--- a/builtin-show-ref.c\n+++ b/builtin-show-ref.c\n@@ -157,7 +157,19 @@ int cmd_show_ref(int argc, const char **argv, const char *prefix)\n \t}\n \tif (show_head)\n \t\thead_ref(show_ref, NULL);\n-\tfor_each_ref(show_ref, NULL);\n+\tif (verify) {\n+\t\tfor (i = 0; pattern[i]; i++)\n+\t\t\tif (!access(git_path(pattern[i]), R_OK)) {\n+\t\t\t\tunsigned char sha1[20];\n+\t\t\t\tif (resolve_ref(pattern[i], sha1, 1, NULL)) {\n+\t\t\t\t\tfound_match++;\n+\t\t\t\t\tprintf(\"%s %s\\n\", sha1_to_hex(sha1),\n+\t\t\t\t\t\t\tpattern[i]);\n+\t\t\t\t}\n+\t\t\t}\n+\t\tfor_each_packed_ref(show_ref, NULL);\n+\t} else\n+\t\tfor_each_ref(show_ref, NULL);\n \tif (!found_match) {\n \t\tif (verify && !quiet)\n \t\t\tdie(\"No match\");\ndiff --git a/refs.c b/refs.c\nindex a02957c..7e6a459 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -413,11 +413,11 @@ int peel_ref(const char *ref, unsigned char *sha1)\n }\n \n static int do_for_each_ref(const char *base, each_ref_fn fn, int trim,\n-\t\t\t void *cb_data)\n+\t\t\t void *cb_data, int packed_only)\n {\n \tint retval;\n \tstruct ref_list *packed = get_packed_refs();\n-\tstruct ref_list *loose = get_loose_refs();\n+\tstruct ref_list *loose = packed_only ? NULL : get_loose_refs();\n \n \twhile (packed && loose) {\n \t\tstruct ref_list *entry;\n@@ -456,24 +456,29 @@ int head_ref(each_ref_fn fn, void *cb_data)\n \treturn 0;\n }\n \n+int for_each_packed_ref(each_ref_fn fn, void *cb_data)\n+{\n+\treturn do_for_each_ref(\"refs/\", fn, 0, cb_data, 1);\n+}\n+\n int for_each_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/\", fn, 0, cb_data);\n+\treturn do_for_each_ref(\"refs/\", fn, 0, cb_data, 0);\n }\n \n int for_each_tag_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/tags/\", fn, 10, cb_data);\n+\treturn do_for_each_ref(\"refs/tags/\", fn, 10, cb_data, 0);\n }\n \n int for_each_branch_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/heads/\", fn, 11, cb_data);\n+\treturn do_for_each_ref(\"refs/heads/\", fn, 11, cb_data, 0);\n }\n \n int for_each_remote_ref(each_ref_fn fn, void *cb_data)\n {\n-\treturn do_for_each_ref(\"refs/remotes/\", fn, 13, cb_data);\n+\treturn do_for_each_ref(\"refs/remotes/\", fn, 13, cb_data, 0);\n }\n \n /* NEEDSWORK: This is only used by ssh-upload and it should go; the\ndiff --git a/refs.h b/refs.h\nindex 51aab1e..0cfe7b5 100644\n--- a/refs.h\n+++ b/refs.h\n@@ -19,6 +19,7 @@ struct ref_lock {\n */\n typedef int each_ref_fn(const char *refname, const unsigned char *sha1, int flags, void *cb_data);\n extern int head_ref(each_ref_fn, void *);\n+extern int for_each_packed_ref(each_ref_fn, void *);\n extern int for_each_ref(each_ref_fn, void *);\n extern int for_each_tag_ref(each_ref_fn, void *);\n"},{"id":"294639","messageId":"7vk60sf0hr.fsf@assigned-by-dhcp.cox.net","threadId":"43405","inReplyTo":"Pine.LNX.4.63.0612161335140.3635@wbgn013.biozentrum.uni-wuerzburg.de","subject":"Re: [PATCH/RFC] avoid accessing _all_ loose refs in git-show-ref","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-12-16T13:12:32Z","receivedAt":"2006-12-16T13:12:32Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Johannes Schindelin writes:\n\n> \tThis is kind of quick and dirty.\n\nMaybe I am blind, but why don't you just do a resolve_ref()\nregardless of packedness? It only builds the packed refs list\nwhen it is not found as a loose ref, and never builds the loose\nrefs list. You probably would want to reject the ones that says\nREF_ISSYMREF.\n\n> \tAn alternative would be to pack the tags _per default_. I once\n> \targued for that, but it appears nobody liked that idea. I even\n> \tproposed to pack _all_ refs, and I still think this would be\n> \ta good idea.\n\nI do not think packing all refs is such a good idea. Branches\nare meant to be worked on and extended so the packed ones\nwould become stale quickly. Packing all tags by default is\nsomething we would want when everybody knows how to handle\nthem.\n\n> \tIsn't it a bug that --verify succeeds, if only _one_ ref passed to \n> \tthe command exists?\n\nI think --verify should insist a single parameter, just like\nrev-parse.\n\nSo wouldn't the code be like:\n\n\tif (verify) {\n\t\tint flag;\n \tif (pattern[1])\n \tdie(\"Eh?\");\n\t\tif (resolve_ref(pattern[0], sha1, 1, &flag) &&\n \t(flag & REF_ISSYMREF) == 0) {\n \t\tprintf(\"%s %s\\n\",\n \tsha1_to_hex(sha1), pattern[0]);\n\t\t\t\texit(0);\n\t\t} else\n \tdie(\"no match\");\n\t}\n\n???\n"},{"id":"298602","messageId":"Pine.LNX.4.63.0612161518070.3635@wbgn013.biozentrum.uni-wuerzburg.de","threadId":"43405","inReplyTo":"7vk60sf0hr.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH/RFC] avoid accessing _all_ loose refs in git-show-ref","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2006-12-16T14:20:57Z","receivedAt":"2006-12-16T14:20:57Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi,\n\nOn Sat, 16 Dec 2006, Junio C Hamano wrote:\n\n> Johannes Schindelin writes:\n> \n> > \tThis is kind of quick and dirty.\n> \n> Maybe I am blind, but why don't you just do a resolve_ref()\n> regardless of packedness? It only builds the packed refs list\n> when it is not found as a loose ref, and never builds the loose\n> refs list. You probably would want to reject the ones that says\n> REF_ISSYMREF.\n\nBut of course!\n\n> > \tAn alternative would be to pack the tags _per default_. I once\n> > \targued for that, but it appears nobody liked that idea. I even\n> > \tproposed to pack _all_ refs, and I still think this would be\n> > \ta good idea.\n> \n> I do not think packing all refs is such a good idea. Branches\n> are meant to be worked on and extended so the packed ones\n> would become stale quickly.\n\nMy idea would have been to edit them in-place. Since the hashes all take \nthe same amount of bytes, it could be done by seeking around in the file.\n\n> Packing all tags by default is something we would want when everybody \n> knows how to handle them.\n\nYou mean all the scripts? Because if you make it the default, you could \nhide that implementation detail away from the user.\n\n> > \tIsn't it a bug that --verify succeeds, if only _one_ ref passed to \n> > \tthe command exists?\n> \n> I think --verify should insist a single parameter, just like\n> rev-parse.\n> \n> So wouldn't the code be like:\n> \n> \tif (verify) {\n> \t\tint flag;\n> \tif (pattern[1])\n> \tdie(\"Eh?\");\n> \t\tif (resolve_ref(pattern[0], sha1, 1, &flag) &&\n> \t(flag & REF_ISSYMREF) == 0) {\n> \t\tprintf(\"%s %s\\n\",\n> \tsha1_to_hex(sha1), pattern[0]);\n> \t\t\t\texit(0);\n> \t\t} else\n> \tdie(\"no match\");\n> \t}\n> \n> ???\n\nAck. Scrap my patch.\n\nCiao,\nDscho\n"}]}