{"thread":{"id":"28954","subject":"Re: git clone --reference not working","startedAt":"2011-11-17T00:54:32Z","lastAt":"2011-11-17T17:40:17Z","messageCount":5,"participants":["Junio C Hamano","Michael Haggerty"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"179605","messageId":"7vobwbpnzr.fsf@alter.siamese.dyndns.org","threadId":"28954","inReplyTo":"20111116234314.GF3306@redhat.com","subject":"Re: git clone --reference not working","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-17T00:54:32Z","receivedAt":"2011-11-17T00:54:32Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Andrea Arcangeli writes:\n\n> latest git.git won't clone linux upstream with --reference. Those\n> v*^{} tags breaks it. What's that stuff anyway, looks totally ugly\n> (two commits with same data contents and header) bah.\n\nThey point at commits they tag, and are essential for auto-following. They\nhave been there forever in ls-remote output and they are not the real\nproblem.\n\nA recent topic that was merged at 9bd5000 tightened the refname checking\ncode without thinking and started to needlessly barf upon seeing them. I\nthink we have discussed about the issue on the list, but I do not think\nthere were fixes yet.\n\nThanks for reminding.\n\nMichael, how does this look?\n\n-- >8 --\nSubject: refs: loosen over-strict \"format\" check\n\nThe add_extra_ref() interface is used to add an extra-ref that is _not_\nour ref for the purpose of helping auto-following of tags and reducing\nobject transfer from remote repository, and they are typically formatted\nas a tagname followed by ^{} to make sure no valid refs match that\npattern. In other words, these entries are deliberately formatted not to\npass check-refname-format test.\n\nA recent series however added a test unconditionally to the add_ref()\nfunction that is called from add_extra_ref(). The check may be sensible\nfor other two callsites of the add_ref() interface, but definitely is\na wrong thing to do in add_extra_ref(). Disable it.\n\nSigned-off-by: Junio C Hamano \n---\n\n* In general, we should make things lenient when we are reading from\n existing source, so that the code can be used to recover a repository\n with badly formatted refs left by other tools, and enforce the format\n check when we write things out. We may further need to loosen the checks\n introduced earlier by mh/check-ref-format-3 topic for similar breakages\n as they are discovered.\n\n refs.c | 20 ++++++++++----------\n t/t5700-clone-reference.sh | 7 +++++++\n 2 files changed, 17 insertions(+), 10 deletions(-)\n\ndiff --git a/refs.c b/refs.c\nindex e69ba26..e7843eb 100644\n--- a/refs.c\n+++ b/refs.c\n@@ -48,7 +48,7 @@ static const char *parse_ref_line(char *line, unsigned char *sha1)\n }\n \n static void add_ref(const char *name, const unsigned char *sha1,\n-\t\t int flag, struct ref_array *refs,\n+\t\t int flag, int check_name, struct ref_array *refs,\n \t\t struct ref_entry **new_entry)\n {\n \tint len;\n@@ -59,7 +59,8 @@ static void add_ref(const char *name, const unsigned char *sha1,\n \tentry = xmalloc(sizeof(struct ref_entry) + len);\n \thashcpy(entry->sha1, sha1);\n \thashclr(entry->peeled);\n-\tif (check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))\n+\tif (check_name &&\n+\t check_refname_format(name, REFNAME_ALLOW_ONELEVEL|REFNAME_DOT_COMPONENT))\n \t\tdie(\"Reference has invalid format: '%s'\", name);\n \tmemcpy(entry->name, name, len);\n \tentry->flag = flag;\n@@ -234,7 +235,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)\n \n \t\tname = parse_ref_line(refline, sha1);\n \t\tif (name) {\n-\t\t\tadd_ref(name, sha1, flag, array, &last);\n+\t\t\tadd_ref(name, sha1, flag, 1, array, &last);\n \t\t\tcontinue;\n \t\t}\n \t\tif (last &&\n@@ -249,7 +250,7 @@ static void read_packed_refs(FILE *f, struct ref_array *array)\n \n void add_extra_ref(const char *name, const unsigned char *sha1, int flag)\n {\n-\tadd_ref(name, sha1, flag, &extra_refs, NULL);\n+\tadd_ref(name, sha1, flag, 0, &extra_refs, NULL);\n }\n \n void clear_extra_refs(void)\n@@ -333,12 +334,11 @@ static void get_ref_dir(const char *submodule, const char *base,\n \t\t\t\t\thashclr(sha1);\n \t\t\t\t\tflag |= REF_ISBROKEN;\n \t\t\t\t}\n-\t\t\t} else\n-\t\t\t\tif (!resolve_ref(ref, sha1, 1, &flag)) {\n-\t\t\t\t\thashclr(sha1);\n-\t\t\t\t\tflag |= REF_ISBROKEN;\n-\t\t\t\t}\n-\t\t\tadd_ref(ref, sha1, flag, array, NULL);\n+\t\t\t} else if (!resolve_ref(ref, sha1, 1, &flag)) {\n+\t\t\t\thashclr(sha1);\n+\t\t\t\tflag |= REF_ISBROKEN;\n+\t\t\t}\n+\t\t\tadd_ref(ref, sha1, flag, 1, array, NULL);\n \t\t}\n \t\tfree(ref);\n \t\tclosedir(dir);\ndiff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh\nindex 895f559..c4c375a 100755\n--- a/t/t5700-clone-reference.sh\n+++ b/t/t5700-clone-reference.sh\n@@ -146,4 +146,11 @@ test_expect_success 'cloning with reference being subset of source (-l -s)' \\\n \n cd \"$base_dir\"\n \n+test_expect_success 'clone with reference from a tagged repository' '\n+\t(\n+\t\tcd A && git tag -a -m 'tagged' HEAD\n+\t) &&\n+\tgit clone --reference=A A I\n+'\n+\n test_done\n"},{"id":"179611","messageId":"4EC4926D.5050004@alum.mit.edu","threadId":"28954","inReplyTo":"7vobwbpnzr.fsf@alter.siamese.dyndns.org","subject":"Re: git clone --reference not working","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-11-17T04:49:49Z","receivedAt":"2011-11-17T04:49:49Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/17/2011 01:54 AM, Junio C Hamano wrote:\n> Andrea Arcangeli writes:\n> \n>> latest git.git won't clone linux upstream with --reference. Those\n>> v*^{} tags breaks it. What's that stuff anyway, looks totally ugly\n>> (two commits with same data contents and header) bah.\n> \n> They point at commits they tag, and are essential for auto-following. They\n> have been there forever in ls-remote output and they are not the real\n> problem.\n> \n> A recent topic that was merged at 9bd5000 tightened the refname checking\n> code without thinking and started to needlessly barf upon seeing them. I\n> think we have discussed about the issue on the list, but I do not think\n> there were fixes yet.\n> \n> Thanks for reminding.\n> \n> Michael, how does this look?\n>\n> -- >8 --\n> Subject: refs: loosen over-strict \"format\" check\n> [...]\n\nI reviewed the patch (and ran the test suite here for good measure).\nLooks good.\n\n>From SubmittingPatches it looks like I should authorize\n\nReviewed-by: Michael Haggerty \n\nIs there a standard way to do so?\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"179613","messageId":"7vr517nvi7.fsf@alter.siamese.dyndns.org","threadId":"28954","inReplyTo":"4EC4926D.5050004@alum.mit.edu","subject":"Re: git clone --reference not working","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-17T05:55:12Z","receivedAt":"2011-11-17T05:55:12Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 11/17/2011 01:54 AM, Junio C Hamano wrote:\n> ...\n> Looks good.\n>\n>>From SubmittingPatches it looks like I should authorize\n>\n> Reviewed-by: Michael Haggerty \n>\n> Is there a standard way to do so?\n\nThat's perfect. I asked for an extra set of eyeballs from somebody who is\nfamiliar with the codepath that had a problem, and that person eyeballed\nand found the change to be an appropriate fix to the problem.\n\nEither Acked-by or Reviewed-by is fine by me.\n\nAs a tentative measure, for tonight's pushout, I am inclined to queue an\nequivalent of this patch on top of both mh/ref-api-2 and mh/ref-api-3\ntopic and merge them to 'next' and 'pu'. I'd appreciate if you can double\ncheck the two merges on master..pu after I push them out in a few hours.\n\nThanks.\n"},{"id":"179639","messageId":"4EC52088.3030308@alum.mit.edu","threadId":"28954","inReplyTo":"7vr517nvi7.fsf@alter.siamese.dyndns.org","subject":"Re: git clone --reference not working","fromName":"Michael Haggerty","fromEmail":"mhagger@alum.mit.edu","sentAt":"2011-11-17T14:56:08Z","receivedAt":"2011-11-17T14:56:08Z","isPatch":false,"sender":{"key":"mhagger@alum.mit.edu","avatar":"https://avatars.githubusercontent.com/u/119718?v=4"},"body":"On 11/17/2011 06:55 AM, Junio C Hamano wrote:\n> As a tentative measure, for tonight's pushout, I am inclined to queue an\n> equivalent of this patch on top of both mh/ref-api-2 and mh/ref-api-3\n> topic and merge them to 'next' and 'pu'. I'd appreciate if you can double\n> check the two merges on master..pu after I push them out in a few hours.\n\nI checked the merges in the following (but didn't run the test suite, as\nI assume that you have already done that):\n\n bc1bbe0..09116a1 master -> gitster/master\n 973592c..ada4ec6 mh/ref-api-2 -> gitster/mh/ref-api-2\n caa8069..37817ba mh/ref-api-3 -> gitster/mh/ref-api-3\n 25e8838..cc76151 next -> gitster/next\n + 06ad567...b032ac4 pu -> gitster/pu (forced update)\n\nThey all look fine.\n\nMichael\n\n-- \nMichael Haggerty\nmhagger@alum.mit.edu\nhttp://softwareswirl.blogspot.com/\n"},{"id":"179641","messageId":"7vvcqimyv2.fsf@alter.siamese.dyndns.org","threadId":"28954","inReplyTo":"4EC52088.3030308@alum.mit.edu","subject":"Re: git clone --reference not working","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2011-11-17T17:40:17Z","receivedAt":"2011-11-17T17:40:17Z","isPatch":false,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Michael Haggerty writes:\n\n> On 11/17/2011 06:55 AM, Junio C Hamano wrote:\n>> As a tentative measure, for tonight's pushout, I am inclined to queue an\n>> equivalent of this patch on top of both mh/ref-api-2 and mh/ref-api-3\n>> topic and merge them to 'next' and 'pu'. I'd appreciate if you can double\n>> check the two merges on master..pu after I push them out in a few hours.\n>\n> I checked the merges in the following...\n\nSorry, what I meant was eyeballing these two merges\n\n aee9699 Merge branch 'mh/ref-api-3' into jch\n 9f8b195 Merge branch 'mh/ref-api-2' into jch\n\nin \"git log --oneline --first-parent master..pu\" to see if I screwed them\nup.\n\nGoing forward, I think after 1.7.8 final is tagged, when rewinding and\nrebuilding the 'next' branch, it would be ideal to rebase these two topics\n(actually, -3 builds on top of -2, so rebasing only the lower one should\nbe sufficient) on top of 1.7.8 that will include the fix in the patch that\nstarted this thread.\n"}]}