{"thread":{"id":"4274","subject":"[RFC][PATCH] Allow transfer of any valid sha1","startedAt":"2006-05-24T07:51:36Z","lastAt":"2006-06-08T09:33:34Z","messageCount":18,"participants":["Eric W. Biederman","Junio C Hamano","Linus Torvalds"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"20613","messageId":"m164jvj1x3.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":null,"subject":"[RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-24T07:51:36Z","receivedAt":"2006-05-24T07:51:36Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"\nWhile working on git-quiltimport I decided to see if\nI could transform Andrews patches where he imports git tress into\ngit-pull commands, which should result in better history and better\nattribution.\n\nTo be accurate of his source Andrew records the sha1 of the commit\nand the git tree he pulled from.  Which looks like:\n\nGIT b307e8548921c686d2eb948ca418ab2941876daa \\\n git+ssh://master.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n\nSo I figured I would transform the above line into the obvious\ngit-pull command:\n\n git-pull \\\n  git+ssh://master.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git \\\n  b307e8548921c686d2eb948ca418ab2941876daa\n\nTo my surprise that didn't work.  There were a couple of little places\nin the scripts where git-fetch and git-fetch-pack never expected to be\ngiven a sha1 but that was easy to fix up, and had no real repercussions.  \n\nMore problematic was the little bit in git-upload pack that only\nallows you to a sha1 if it was on the list of sha1 generated from\nlooking at the heads.  I'm not at all certain of the sense of\nthat check as you can get everything by just cloning the repository.\n\nCan we fix the check in upload-pack.c something like my\npatch below does?  Are there any security implications for\ndoing that?\n\nCould we just make the final check before dying if (!o) ?\n\n\n\n\n\n\t\t/* We have sent all our refs already, and the other end\n\t\t * should have chosen out of them; otherwise they are\n\t\t * asking for nonsense.\n\t\t *\n\t\t * Hmph.  We may later want to allow \"want\" line that\n\t\t * asks for something like \"master~10\" (symbolic)...\n\t\t * would it make sense?  I don't know.\n\t\t */\n\ndiff --git a/upload-pack.c b/upload-pack.c\nindex 47560c9..0f2e544 100644\n--- a/upload-pack.c\n+++ b/upload-pack.c\n@@ -207,7 +207,9 @@ static int receive_needs(void)\n \t\t * would it make sense?  I don't know.\n \t\t */\n \t\to = lookup_object(sha1_buf);\n-\t\tif (!o || !(o->flags & OUR_REF))\n+\t\tif (!o)\n+\t\t\to = parse_object(sha1_buf);\n+\t\tif (!o || ((o->type != commit_type) && (o->type != tag_type)))\n \t\t\tdie(\"git-upload-pack: not our ref %s\", line+5);\n \t\tif (!(o->flags & WANTED)) {\n \t\t\to->flags |= WANTED;\n"},{"id":"20617","messageId":"7vejyjpz9a.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"m164jvj1x3.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-24T09:07:13Z","receivedAt":"2006-05-24T09:07:13Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ebiederm@xmission.com (Eric W. Biederman) writes:\n\n> Can we fix the check in upload-pack.c something like my\n> patch below does?  Are there any security implications for\n> doing that?\n\n> Could we just make the final check before dying if (!o) ?\n\nThe primary implication is about correctness, so I am reluctant\nto break it without a careful alternative check in place.\n\nThe issue is that having a single object in the repository does\nnot guarantee that you have everything reachable from it, and we\nneed that guarantee.  Reachability from the refs is what\nguarantees that.\n\nWe are careful to update the ref at the very end of the transfer\n(fetch/clone or push); so if an object is reachable from a ref,\nthen all the objects reachable from that object are available in\nthe repository.\n\nImagine http commit walker started fetching tip of upstream into\nyour repository and you interrupted the transfer.  Objects near\nthe tip of the upstream history are available after such an\ninterrupted transfer.  But a bit older history (but still later\nthan what we had before we started the transfer) are not.\n\nWe do not update the ref with the downloaded tip object, so that\nwe would not break the guarantee.  This guarantee is needed for\nfeeding clients from the repository later.  If you tell your\nclients, after such an interrupted transfer, that you are\nwilling to serve the objects near the (new) tip, the clients may\nrightfully request objects that are reachable from these\nobjects, some of them you do _not_ have!\n\nSo this \"on demand SHA1\" stuff needs to be solved by checking if\nthe given object is reachable from our refs in upload-pack,\ninstead of the current check to see if the given object is\npointed by our refs.  When upload-pack can prove that the object\nis reachable from one of the refs, it is OK to use it; otherwise\nyou should not.\n\nNow, proving that a given SHA1 is the name of an object that\nexists in the repository is cheap (has_sha1_file()), but proving\nthat the object is reachable from some of our refs can become\nquite expensive.  That gives this issue a security implication\nas well -- you can easily DoS the git-daemon that way, for\nexample.\n"},{"id":"20671","messageId":"m13beysnb2.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7vejyjpz9a.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-25T05:09:21Z","receivedAt":"2006-05-25T05:09:21Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> ebiederm@xmission.com (Eric W. Biederman) writes:\n>\n>> Can we fix the check in upload-pack.c something like my\n>> patch below does?  Are there any security implications for\n>> doing that?\n>\n>> Could we just make the final check before dying if (!o) ?\n>\n> The primary implication is about correctness, so I am reluctant\n> to break it without a careful alternative check in place.\n>\n> The issue is that having a single object in the repository does\n> not guarantee that you have everything reachable from it, and we\n> need that guarantee.  Reachability from the refs is what\n> guarantees that.\n\nI don't see why having something reachable from a ref guarantees\nthat everything is reachable.  Given the recent patch that added\na check to make certain a ref actually existed I believe there\nis some evidence that trees may become corrupted, and have the\nproblems you describe.\n\n> We are careful to update the ref at the very end of the transfer\n> (fetch/clone or push); so if an object is reachable from a ref,\n> then all the objects reachable from that object are available in\n> the repository.\n\nIn the normal case I agree.\n\n> Imagine http commit walker started fetching tip of upstream into\n> your repository and you interrupted the transfer.  Objects near\n> the tip of the upstream history are available after such an\n> interrupted transfer.  But a bit older history (but still later\n> than what we had before we started the transfer) are not.\n>\n> We do not update the ref with the downloaded tip object, so that\n> we would not break the guarantee.  This guarantee is needed for\n> feeding clients from the repository later.  If you tell your\n> clients, after such an interrupted transfer, that you are\n> willing to serve the objects near the (new) tip, the clients may\n> rightfully request objects that are reachable from these\n> objects, some of them you do _not_ have!\n\nI clearly would not advertise it.  My problem is that I have\nevidence that someone pulled a given sha1 at some point from \nsome branch on a given repository.  But I don't have that branch.\n\nActually trees mirrored with rsync have similar problems all of\nthe time when the catch a tree in the middle of an update.\n\n> So this \"on demand SHA1\" stuff needs to be solved by checking if\n> the given object is reachable from our refs in upload-pack,\n> instead of the current check to see if the given object is\n> pointed by our refs.  When upload-pack can prove that the object\n> is reachable from one of the refs, it is OK to use it; otherwise\n> you should not.\n\nI have a problem with that approach.  Suppose the branch I have\nevidence something came from is like your pu branch.   If I want\na copy of your pu branch at some point in the past, but you have\nrebased it since that sha1 was published then there will clearly not\nbe a path from any current head to that branch.  But if I still have a\ncopy of the sha1 I should actually be able to recover the old copy of\nthe pu branch from your tree.\n\n> Now, proving that a given SHA1 is the name of an object that\n> exists in the repository is cheap (has_sha1_file()), but proving\n> that the object is reachable from some of our refs can become\n> quite expensive.  That gives this issue a security implication\n> as well -- you can easily DoS the git-daemon that way, for\n> example.\n\nExactly, which is why I aimed for the cheap test.\n\nThere is a reasonable argument that can be made that the branches\nrepresent the policy that you are willing to serve.  If you have a\ntree and share a common object store with a much lager tree, like\nDavid Woodhouse has set up, I can see such a policy being desirable.\n\nThat is an argument I have a much harder time shooting down.\nAt the same time if it is just a policy question the policy it should\nbe modifiable with an appropriate configuration directive, or\ncommand line option.\n\nEric\n"},{"id":"20674","messageId":"7vwtcay5k8.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"m13beysnb2.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-25T06:36:07Z","receivedAt":"2006-05-25T06:36:07Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ebiederm@xmission.com (Eric W. Biederman) writes:\n\n> I clearly would not advertise it.  My problem is that I have\n> evidence that someone pulled a given sha1 at some point from \n> some branch on a given repository.  But I don't have that branch.\n\nIf that was over rsync (as you mention later), then I would\nconsider that is an unfortunate unfixable issue.  rsync mirrors\nare fundamentally unsafe for git -- Linus and I do not keep\nsaying rsync should be deprecated without good reasons.\n\nThere still might be bugs that breaks this guarantee outside\nrsync, but if that is the case we should fix it.\n\nI do not want to rehash the thread around Sep 29th 2005 here.\nThe entry point of that thread is this message:\n\n\thttp://marc.theaimsgroup.com/?l=git&m=112795140820665\n\nand the punch line are these two messages:\n\n\thttp://marc.theaimsgroup.com/?l=git&m=112801874021223\n\thttp://marc.theaimsgroup.com/?l=git&m=112802808030710\n\nI did not realize what I was breaking initially.  I am not\nashamed of having been wrong, but it was embarrassing ;-).\n\n> If I want\n> a copy of your pu branch at some point in the past, but you have\n> rebased it since that sha1 was published then there will clearly not\n> be a path from any current head to that branch.  But if I still have a\n> copy of the sha1 I should actually be able to recover the old copy of\n> the pu branch from your tree.\n\nNot necessarily.  I occasionally prune after rewinding.  When my\n\"pu\" branch head does not point at the lost commit, the\nrepository may or may not have that object you happen to know I\nused to have anymore.\n\n>> Now, proving that a given SHA1 is the name of an object that\n>> exists in the repository is cheap (has_sha1_file()), but proving\n>> that the object is reachable from some of our refs can become\n>> quite expensive.  That gives this issue a security implication\n>> as well -- you can easily DoS the git-daemon that way, for\n>> example.\n>\n> Exactly, which is why I aimed for the cheap test.\n\nBut the thing is the cheap test is broken, eh, rather,\npropagates brokenness downstream (which is perhaps worse).\n"},{"id":"20692","messageId":"m1lksqdook.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7vwtcay5k8.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-25T17:00:59Z","receivedAt":"2006-05-25T17:00:59Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> ebiederm@xmission.com (Eric W. Biederman) writes:\n>\n>> I clearly would not advertise it.  My problem is that I have\n>> evidence that someone pulled a given sha1 at some point from \n>> some branch on a given repository.  But I don't have that branch.\n>\n> If that was over rsync (as you mention later), then I would\n> consider that is an unfortunate unfixable issue.  rsync mirrors\n> are fundamentally unsafe for git -- Linus and I do not keep\n> saying rsync should be deprecated without good reasons.\n>\n> There still might be bugs that breaks this guarantee outside\n> rsync, but if that is the case we should fix it.\n\nSounds reasonable.  So far I don't believe anything I have\nproposed would result in a reference getting written\nif we don't transfer all of the dependencies.\n\nI do need to examine the algorithm by which we compute what\nto transmit and make certain I have not broke that.\nI believe I am still only using the existing references\nfor finding a common point in the history.\n\n> I do not want to rehash the thread around Sep 29th 2005 here.\n> The entry point of that thread is this message:\n>\n> \thttp://marc.theaimsgroup.com/?l=git&m=112795140820665\n>\n> and the punch line are these two messages:\n>\n> \thttp://marc.theaimsgroup.com/?l=git&m=112801874021223\n> \thttp://marc.theaimsgroup.com/?l=git&m=112802808030710\n>\n> I did not realize what I was breaking initially.  I am not\n> ashamed of having been wrong, but it was embarrassing ;-).\n>\n>> If I want\n>> a copy of your pu branch at some point in the past, but you have\n>> rebased it since that sha1 was published then there will clearly not\n>> be a path from any current head to that branch.  But if I still have a\n>> copy of the sha1 I should actually be able to recover the old copy of\n>> the pu branch from your tree.\n>\n> Not necessarily.  I occasionally prune after rewinding.  When my\n> \"pu\" branch head does not point at the lost commit, the\n> repository may or may not have that object you happen to know I\n> used to have anymore.\n\nAgreed.  Of course the simple object existence test works in that\ninstance.  Not that it does in general.  The could be a git-prune\nversus upload-pack race for instance.\n\n>>> Now, proving that a given SHA1 is the name of an object that\n>>> exists in the repository is cheap (has_sha1_file()), but proving\n>>> that the object is reachable from some of our refs can become\n>>> quite expensive.  That gives this issue a security implication\n>>> as well -- you can easily DoS the git-daemon that way, for\n>>> example.\n>>\n>> Exactly, which is why I aimed for the cheap test.\n>\n> But the thing is the cheap test is broken, eh, rather,\n> propagates brokenness downstream (which is perhaps worse).\n\nAs I understand it brokenness is writing a ref when you don't\nhave the complete tree it points to.  I have no desire\nto do that.\n\nMy basic argument is that starting a pull with a commit that is not a\nreference is no worse than staring a pull from a broken repository.  The\nsame checks that protects us should work in either case.\n\nSo as long as what I have done does not compromise the computation\nof a common ancestor I think we should be fine.\n\nI can see the argument that in a non-broken repository that finding\na path from an existing ref is proof that everything will work.\nHowever if the code can be made to work without requiring that\nproof it should be an even stronger guarantee of correctness,\nand the expensive step of walking down from an existing ref would\nbe unnecessary.\n\nEric\n"},{"id":"20693","messageId":"Pine.LNX.4.64.0605251024320.5623@g5.osdl.org","threadId":"4274","inReplyTo":"m1lksqdook.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-25T17:28:47Z","receivedAt":"2006-05-25T17:28:47Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 25 May 2006, Eric W. Biederman wrote:\n> \n> My basic argument is that starting a pull with a commit that is not a\n> reference is no worse than staring a pull from a broken repository.  The\n> same checks that protects us should work in either case.\n\nI think Junio reacted to the subject line, which was somewhat badly \nphrased. You're not looking to transfer random objects, you're looking to \n_start_ a branch at any arbitrary known point.\n\nHowever, Junio's point is probably that the \"any valid SHA1\" might \nactually point to a broken tree, even if it exists on the server.\n\nOf course, in that case hopefully git-rev-list exits with an error, and \nthe server doesn't generate any pack at all rather than generating a \nbroken one.\n\nHowever, there's a (questionable) security issue: what if the server \ndoesn't _want_ to expose certain branches? Arguably, if you know the top \nSHA1, you likely know all that it contains, but it may be a valid argument \nto say that if the SHA1 isn't an exported branch, you shouldn't \nnecessarily be able to follow it.\n\n\t\tLinus\n"},{"id":"20694","messageId":"m1bqtmdly9.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"Pine.LNX.4.64.0605251024320.5623@g5.osdl.org","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-25T17:59:58Z","receivedAt":"2006-05-25T17:59:58Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Thu, 25 May 2006, Eric W. Biederman wrote:\n>> \n>> My basic argument is that starting a pull with a commit that is not a\n>> reference is no worse than staring a pull from a broken repository.  The\n>> same checks that protects us should work in either case.\n>\n> I think Junio reacted to the subject line, which was somewhat badly \n> phrased. You're not looking to transfer random objects, you're looking to \n> _start_ a branch at any arbitrary known point.\n\nProbably, but if I understood enough to get the subject line right the\nfirst time I probably would have understood enough to just send\na patch :)\n\n> However, Junio's point is probably that the \"any valid SHA1\" might \n> actually point to a broken tree, even if it exists on the server.\n>\n> Of course, in that case hopefully git-rev-list exits with an error, and \n> the server doesn't generate any pack at all rather than generating a \n> broken one.\n>\n> However, there's a (questionable) security issue: what if the server \n> doesn't _want_ to expose certain branches? Arguably, if you know the top \n> SHA1, you likely know all that it contains, but it may be a valid argument \n> to say that if the SHA1 isn't an exported branch, you shouldn't \n> necessarily be able to follow it.\n\nAgreed and I mentioned this one earlier.\n\nHowever the only way the above scenario can even happen in a useful\nmanner is with a shared object store for several repositories.  Otherwise\nyou couldn't access the data you don't want to share.\n\nI can't think of a valid argument against not sharing an entire\nrepository except David Woodhouse's bandwidth concern.\nOf course what was wanted there was a test a limit to how far\nback in the history you could look for a common commit, which\nis something different.\n\nIn general it is much easier to guarantee that either a repository is\nshared or it is not.  Making a guarantee that objects that\n\"git-fsck-objects --unreachable --full\" identifies will never be\ndownloaded is difficult, and probably not worth encouraging\npeople to do.\n\nThat said it is easy to keep the current behavior as an option,\nso the security policy issue shouldn't limit the technical discussion.\n\nEric\n"},{"id":"20696","messageId":"7v3beyuffg.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"Pine.LNX.4.64.0605251024320.5623@g5.osdl.org","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-25T18:28:51Z","receivedAt":"2006-05-25T18:28:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Thu, 25 May 2006, Eric W. Biederman wrote:\n>> \n>> My basic argument is that starting a pull with a commit that is not a\n>> reference is no worse than staring a pull from a broken repository.  The\n>> same checks that protects us should work in either case.\n>\n> I think Junio reacted to the subject line, which was somewhat badly \n> phrased. You're not looking to transfer random objects, you're looking to \n> _start_ a branch at any arbitrary known point.\n\nI realize that now.  From Eric's original message:\n\n  To be accurate of his source Andrew records the sha1 of the commit\n  and the git tree he pulled from.  Which looks like:\n\n  GIT b307e8548921c686d2eb948ca418ab2941876daa \\\n   git+ssh://master.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n\n  So I figured I would transform the above line into the obvious\n  git-pull command:\n\n   git-pull \\\n    git+ssh://master.kernel.org/pub/scm/.../torvalds/linux-2.6.git \\\n    b307e8548921c686d2eb948ca418ab2941876daa\n\nWith the limitation of the current tool, we could do:\n\n  git-fetch master.kernel.org:/pub/scm/.../torvalds/linux-2.6.git \\\n\trefs/heads/master:refs/remotes/linus/master\n  git merge 'whatever merge message' HEAD b307e854\n\nassuming that b307e854 is reachable from your tip.  So it might\nbe just a matter of giving a convenient shorthand to do the\nabove two commands, instead of mucking with upload-pack.\n"},{"id":"20697","messageId":"Pine.LNX.4.64.0605251134410.5623@g5.osdl.org","threadId":"4274","inReplyTo":"7v3beyuffg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Linus Torvalds","fromEmail":"torvalds@osdl.org","sentAt":"2006-05-25T18:36:16Z","receivedAt":"2006-05-25T18:36:16Z","isPatch":true,"sender":{"key":"torvalds@linux-foundation.org","avatar":"https://avatars.githubusercontent.com/u/1024025?v=4"},"body":"\n\nOn Thu, 25 May 2006, Junio C Hamano wrote:\n> \n> With the limitation of the current tool, we could do:\n> \n>   git-fetch master.kernel.org:/pub/scm/.../torvalds/linux-2.6.git \\\n> \trefs/heads/master:refs/remotes/linus/master\n>   git merge 'whatever merge message' HEAD b307e854\n> \n> assuming that b307e854 is reachable from your tip.  So it might\n> be just a matter of giving a convenient shorthand to do the\n> above two commands, instead of mucking with upload-pack.\n\nIt's not upload-pack that needs mucking with. It's simply \"fetch-pack\" \nthat currently will refuse to say \"want b307e854..\", because the only \nthing it can do is say \"want <headref>\".\n\nSo the patch would literally be to have a way to tell fetch-pack directly \nwhat you want, and not have the \"only select from remote branches\" logic.\n\n\t\tLinus\n"},{"id":"20704","messageId":"m13bexetj1.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"Pine.LNX.4.64.0605251134410.5623@g5.osdl.org","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-25T20:30:58Z","receivedAt":"2006-05-25T20:30:58Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Linus Torvalds <torvalds@osdl.org> writes:\n\n> On Thu, 25 May 2006, Junio C Hamano wrote:\n>> \n>> With the limitation of the current tool, we could do:\n>> \n>>   git-fetch master.kernel.org:/pub/scm/.../torvalds/linux-2.6.git \\\n>> \trefs/heads/master:refs/remotes/linus/master\n>>   git merge 'whatever merge message' HEAD b307e854\n>> \n>> assuming that b307e854 is reachable from your tip.  So it might\n>> be just a matter of giving a convenient shorthand to do the\n>> above two commands, instead of mucking with upload-pack.\n>\n> It's not upload-pack that needs mucking with. It's simply \"fetch-pack\" \n> that currently will refuse to say \"want b307e854..\", because the only \n> thing it can do is say \"want <headref>\".\n>\n> So the patch would literally be to have a way to tell fetch-pack directly \n> what you want, and not have the \"only select from remote branches\" logic.\n\nSo fixing fetch-pack is easy and pretty non-controversial.\nThe patch below handles that.\n\nThe problem is that I then run into the limitations in upload-pack.\n\n(The movement of filter_refs may actually be overkill)\n\nEric\n\n\n\ndiff --git a/fetch-pack.c b/fetch-pack.c\nindex a3bcad0..c767d84 100644\n--- a/fetch-pack.c\n+++ b/fetch-pack.c\n@@ -260,6 +260,27 @@ static void mark_recent_complete_commits\n \t}\n }\n \n+static struct ref **get_sha1_heads(struct ref **refs, int nr_heads, char **head)\n+{\n+\tint i;\n+\tfor (i  = 0; i < nr_heads; i++) {\n+\t\tstruct ref *ref;\n+\t\tunsigned char sha1[20];\n+\t\tchar *s = head[i];\n+\t\tint len = strlen(s);\n+\n+\t\tif (len != 40 || get_sha1_hex(s, sha1))\n+\t\t\tcontinue;\n+\n+\t\tref = xcalloc(1, sizeof(*ref) + len + 1);\n+\t\tmemcpy(ref->old_sha1, sha1, 20);\n+\t\tmemcpy(ref->name, s, len + 1);\n+\t\t*refs = ref;\n+\t\trefs = &ref->next;\n+\t}\n+\treturn refs;\n+}\n+\n static void filter_refs(struct ref **refs, int nr_match, char **match)\n {\n \tstruct ref *prev, *current, *next;\n@@ -311,6 +332,8 @@ static int everything_local(struct ref *\n \tif (cutoff)\n \t\tmark_recent_complete_commits(cutoff);\n \n+\tfilter_refs(refs, nr_match, match);\n+\n \t/*\n \t * Mark all complete remote refs as common refs.\n \t * Don't mark them common yet; the server has to be told so first.\n@@ -329,8 +352,6 @@ static int everything_local(struct ref *\n \t\t}\n \t}\n \n-\tfilter_refs(refs, nr_match, match);\n-\n \tfor (retval = 1, ref = *refs; ref ; ref = ref->next) {\n \t\tconst unsigned char *remote = ref->old_sha1;\n \t\tunsigned char local[20];\n@@ -373,6 +394,7 @@ static int fetch_pack(int fd[2], int nr_\n \t\tpacket_flush(fd[1]);\n \t\tdie(\"no matching remote head\");\n \t}\n+\tget_sha1_heads(&ref, nr_match, match);\n \tif (everything_local(&ref, nr_match, match)) {\n \t\tpacket_flush(fd[1]);\n \t\tgoto all_done;\ndiff --git a/git-parse-remote.sh b/git-parse-remote.sh\nindex 187f088..2372df8 100755\n--- a/git-parse-remote.sh\n+++ b/git-parse-remote.sh\n@@ -105,6 +105,7 @@ canon_refs_list_for_fetch () {\n \t\t'') remote=HEAD ;;\n \t\trefs/heads/* | refs/tags/* | refs/remotes/*) ;;\n \t\theads/* | tags/* | remotes/* ) remote=\"refs/$remote\" ;;\n+\t\t[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]) ;;\n \t\t*) remote=\"refs/heads/$remote\" ;;\n \t\tesac\n \t\tcase \"$local\" in\n"},{"id":"20706","messageId":"m1y7wpde1w.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7v3beyuffg.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-25T20:50:35Z","receivedAt":"2006-05-25T20:50:35Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> Linus Torvalds <torvalds@osdl.org> writes:\n>\n>> On Thu, 25 May 2006, Eric W. Biederman wrote:\n>>> \n>>> My basic argument is that starting a pull with a commit that is not a\n>>> reference is no worse than staring a pull from a broken repository.  The\n>>> same checks that protects us should work in either case.\n>>\n>> I think Junio reacted to the subject line, which was somewhat badly \n>> phrased. You're not looking to transfer random objects, you're looking to \n>> _start_ a branch at any arbitrary known point.\n>\n> I realize that now.  From Eric's original message:\n>\n>   To be accurate of his source Andrew records the sha1 of the commit\n>   and the git tree he pulled from.  Which looks like:\n>\n>   GIT b307e8548921c686d2eb948ca418ab2941876daa \\\n>    git+ssh://master.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n>\n>   So I figured I would transform the above line into the obvious\n>   git-pull command:\n>\n>    git-pull \\\n>     git+ssh://master.kernel.org/pub/scm/.../torvalds/linux-2.6.git \\\n>     b307e8548921c686d2eb948ca418ab2941876daa\n>\n> With the limitation of the current tool, we could do:\n>\n>   git-fetch master.kernel.org:/pub/scm/.../torvalds/linux-2.6.git \\\n> \trefs/heads/master:refs/remotes/linus/master\n>   git merge 'whatever merge message' HEAD b307e854\n>\n> assuming that b307e854 is reachable from your tip.  So it might\n> be just a matter of giving a convenient shorthand to do the\n> above two commands, instead of mucking with upload-pack.\n\nIf we conclude the fetch by sha1 path is not practical certainly.\n\nThere are a couple of problems with the just use the tool as\nis approach.\n- I don't know which branch I need to fetch.\n  Although it looks like Andrew has kept that information when it was not the\n  default branch so I can probably use that.\n- Fetching a branch that I just want a subset of is wasteful.\n- It feels really weird when everything else allows me to use sha1s\n  for git-fetch to deny them.\n\nThen there is the big hole in my plan to get better changelog information\nthat it appears that after Andrew pulls a branch he resolves some\nmerge conflicts.  If that is right I need to figure out how to address\nthat before I can improve git-quiltimport.sh.\n\nTo get a slightly better feel of the problem below is the complete list of git\ntrees that Andrew pulled in for 2.6.17-rc4-mm3.\n\nEric\n\n\nGIT d684de2c4a498ec4edf4e6c3420b008c62be394c git+ssh://master.kernel.org/pub/scm/linux/kernel/git/lenb/linux-acpi-2.6.git#test\nGIT 34ec52e3356245e9a13dfcbc8460635e675f13cf git+ssh://master.kernel.org/pub/scm/linux/kernel/git/davej/agpgart.git\nGIT 08e66777d094d93091a914a8746a9b93599e14a9 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/perex/alsa-current.git\nGIT 0ef744735f0d82d90809935586a0d1043f7f09b5 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/audit-current.git#master.b13\nGIT cca5d8ad1f58f188500b9fb12ba6d98643a4cf49 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/axboe/linux-2.6-block.git#for-linus\nGIT b3b6a155c2b85d436b192d74e459f837eab0944e git+ssh://master.kernel.org/pub/scm/linux/kernel/git/axboe/linux-2.6-block.git#cfq\nGIT 8ba86486650c59f969c589c7d6c3dd4da734c75a git+ssh://master.kernel.org/pub/scm/linux/kernel/git/sfrench/cifs-2.6.git\nGIT 2a1db55336a9e99f5dd7ee64e42fa4cbb509ea6a git+ssh://master.kernel.org/pub/scm/linux/kernel/git/herbert/cryptodev-2.6.git\nGIT 639b4408a9e8e014878c7538859f33f852c23882 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/mchehab/v4l-dvb.git#devel\nGIT d2f222e6310b073ae3d91b8d3d676621fae1314e git+ssh://master.kernel.org/pub/scm/linux/kernel/git/steve/gfs2-2.6.git\nGIT 3ac6c7b44560fdf2ea8865536bd52d4ff038107e git://git.infradead.org/hdrcleanup-2.6.git\nGIT 12415e45ab0429a88412f4af365515adbe0bdd68 git://git.infradead.org/hdrinstall-2.6.git\nGIT 155f23d603727fcb2af6c69ff77b74d1d4eb5bde git+ssh://master.kernel.org/pub/scm/linux/kernel/git/aegl/linux-2.6.git#test\nGIT ba9cfd16a13a932f0603a7f65b3881738a698ae1 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/roland/infiniband.git#for-mm\nGIT 51d797474f87b375819d084f7583a2864c5656c4 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/airlied/intelfb-2.6#i915fb\nGIT c32217fdc98292dbafd5f51d3f43337081b01c29 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/hpa/linux-2.6-klibc.git\nGIT 9fe74aaa1dc55100d20d9b7be2ccbf84ad26ce84 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jgarzik/libata-dev.git#ALL\nGIT 864fdc881dd9e0077f9ed11191055e3eabf3b2a5 git://www.linux-mips.org/pub/scm/upstream.git#for-akpm\nGIT 0d25971d7c969debf76f9fab6d6b37cb62408f55 git://git.infradead.org/mtd-2.6.git\nGIT b748b7167cbeb11e729c6f9c3472165903dd115e git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jgarzik/netdev-2.6.git#ALL\nGIT c4bdea3ce8b1d9b9d8dc44223542b3ebbe3a3020 git://git.linux-nfs.org/pub/linux/nfs-2.6.git\nGIT f8b4c6027275d9b2d5004726a6d1bb818a13ddef git://oss.oracle.com/home/sourcebo/git/ocfs2.git/#ALL\nGIT 35b86edf75270176310cb9507745d8c02b9e6592 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/brodo/pcmcia-2.6.git/\nGIT 3c06da5ae5358e9d325d541a053e1059e9654bcc git+ssh://master.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git\nGIT aa783a8f31c79f493bd49ba926b171b79b9839fb git://git.infradead.org/users/dwmw2/rbtree-2.6.git\nGIT ee69d3f20b23250eae98a4cb20236196694f0b81 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jejb/aic94xx-sas-2.6.git\nGIT 9f434d4f84a235f6b61aec6e691d6b07bc46fc24 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-rc-fixes-2.6.git\nGIT 0df298d180556450cbe5edf12c1e890f6ac6ea97 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jejb/scsi-target-2.6.git\nGIT f1d282724317895f73c4c182041ab4385126c026 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/jgarzik/misc-2.6.git#stex\nGIT 19932e4d7d2002bc956d1636e1c3a1d4455049fa git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/bird.git#frv.b14\nGIT 6ea79eadeba9b0d0ab08dcf7ee16df13e1fdadae git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/bird.git#m32r.b14\nGIT c3d6ecc77e8e5d4ac82c3bf27ed02b8c0d83d41d git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/bird.git#m68k.b14\nGIT e7dd49b206624021c23a27512d2a31503f5207f2 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/bird.git#upf.b14\nGIT ea0d175136582dff6e3591368b1da472ef3870b8 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/viro/bird.git#volatile.b14\nGIT b29527edccbbc53a908bc34a910df3601061c950 git+ssh://master.kernel.org/pub/scm/linux/kernel/git/wim/linux-2.6-watchdog-mm.git\nGIT b307e8548921c686d2eb948ca418ab2941876daa git+ssh://master.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git\n"},{"id":"20707","messageId":"7vy7wpsu5c.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"m13bexetj1.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-25T20:53:51Z","receivedAt":"2006-05-25T20:53:51Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ebiederm@xmission.com (Eric W. Biederman) writes:\n\n> So fixing fetch-pack is easy and pretty non-controversial.\n> The patch below handles that.\n\nI am at work so I cannot really spend time on this right now,\nbut I am OK with letting it send arbitrary SHA1 the caller\nobtained out of band.  I do not know about your implementation,\nsince I haven't really looked at it.\n\n> (The movement of filter_refs may actually be overkill)\n\nIt may not just overkill but may actively be wrong, but again I\nhaven't looked at it yet.\n\nWill take a look tonight.\n"},{"id":"20708","messageId":"7virntsto6.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"m1y7wpde1w.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-25T21:04:09Z","receivedAt":"2006-05-25T21:04:09Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ebiederm@xmission.com (Eric W. Biederman) writes:\n\n> - I don't know which branch I need to fetch.\n\nAs you say yourself Andrew marks which one he fetched from, so\nthis is a non-issue.\n\n> - Fetching a branch that I just want a subset of is wasteful.\n\nGenerally this is true, but in practice and especially for this\nparticular application I do not think so.  After all Andrew\npulled from the tip and got that tip, and IIUYC you are trying\nto follow what Andrew did, so you'd be better doing this soon\nafter Andrew annouces the series, so your subset would be a\nclose to 100% subset.  Otherwise you would have different\nproblem anyway -- the tree owner after seeing -mm tree has his\nseries may rewind and rebuild the branch in preparation of\nfeeding him with the next time around.\n\n> - It feels really weird when everything else allows me to use sha1s\n>   for git-fetch to deny them.\n\nThat is a real argument and I am not opposed to change\nfetch-pack to ask for an arbitrary SHA1 the caller obtained out\nof band.\n\n> Then there is the big hole in my plan to get better changelog information\n> that it appears that after Andrew pulls a branch he resolves some\n> merge conflicts.  If that is right I need to figure out how to address\n> that before I can improve git-quiltimport.sh.\n\nThe last time I talked with Andrew, he is not doing a merge nor\nresolving merge conflicts.  He treats git primarily as a\npatchbomb distribution mechanism, and works on (a rough\nequivalent of) the output of format-patch from merge base\nbetween his base tree and individual subsystem tree.  After that\nthings are normal quilt workflow outside git, whatever it is.\n"},{"id":"20745","messageId":"m18xopchrz.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7vy7wpsu5c.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-26T08:27:44Z","receivedAt":"2006-05-26T08:27:44Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> ebiederm@xmission.com (Eric W. Biederman) writes:\n>\n>> So fixing fetch-pack is easy and pretty non-controversial.\n>> The patch below handles that.\n>\n> I am at work so I cannot really spend time on this right now,\n> but I am OK with letting it send arbitrary SHA1 the caller\n> obtained out of band.  I do not know about your implementation,\n> since I haven't really looked at it.\n\nAgreed.  I'm not certain about my implementation yet either I\njust know I was in the ball park.\n\nI needed the conversation to understand what the limits were.\n\n>> (The movement of filter_refs may actually be overkill)\n>\n> It may not just overkill but may actively be wrong, but again I\n> haven't looked at it yet.\n>\n> Will take a look tonight.\n\nSure.  The code was all a work in progress so I don't expect to\nhave all of the details ironed out.  In particular I didn't\neven look at the non fetch-pack case, and I didn't update the\ndocumentation.\n\nEric\n"},{"id":"20746","messageId":"m14pzdchje.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7virntsto6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-26T08:32:53Z","receivedAt":"2006-05-26T08:32:53Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> ebiederm@xmission.com (Eric W. Biederman) writes:\n\n>> - It feels really weird when everything else allows me to use sha1s\n>>   for git-fetch to deny them.\n>\n> That is a real argument and I am not opposed to change\n> fetch-pack to ask for an arbitrary SHA1 the caller obtained out\n> of band.\n\nGood this was the primary reason I kept pursuing the issue after\nI figured out what it was.\n\n>> Then there is the big hole in my plan to get better changelog information\n>> that it appears that after Andrew pulls a branch he resolves some\n>> merge conflicts.  If that is right I need to figure out how to address\n>> that before I can improve git-quiltimport.sh.\n>\n> The last time I talked with Andrew, he is not doing a merge nor\n> resolving merge conflicts.  He treats git primarily as a\n> patchbomb distribution mechanism, and works on (a rough\n> equivalent of) the output of format-patch from merge base\n> between his base tree and individual subsystem tree.  After that\n> things are normal quilt workflow outside git, whatever it is.\n\nThat sounds right.  I just know that there I had some strange\nmerge conflicts on the second git tree I pulled from.  Something\nabout a file being added twice.  It was one thing too many to\ninvestigate this round.\n\nEric\n"},{"id":"20747","messageId":"7vac95m799.fsf@assigned-by-dhcp.cox.net","threadId":"4274","inReplyTo":"m13bexetj1.fsf@ebiederm.dsl.xmission.com","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-05-26T10:04:50Z","receivedAt":"2006-05-26T10:04:50Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"ebiederm@xmission.com (Eric W. Biederman) writes:\n\n> diff --git a/fetch-pack.c b/fetch-pack.c\n> index a3bcad0..c767d84 100644\n> --- a/fetch-pack.c\n> +++ b/fetch-pack.c\n> @@ -260,6 +260,27 @@ static void mark_recent_complete_commits\n>  \t}\n>  }\n>  \n> +static struct ref **get_sha1_heads(struct ref **refs, int nr_heads, char **head)\n> +{\n> +\tint i;\n> +\tfor (i  = 0; i < nr_heads; i++) {\n> +\t\tstruct ref *ref;\n> +\t\tunsigned char sha1[20];\n> +\t\tchar *s = head[i];\n> +\t\tint len = strlen(s);\n> +\n> +\t\tif (len != 40 || get_sha1_hex(s, sha1))\n> +\t\t\tcontinue;\n\nSo the new convention is fetch-pack can take ref name (as\nbefore), or a bare 40-byte hexadecimal.  I think sane people\nwould not use ambiguous refname that says \"deadbeef\" five times,\nand even if the do so they could disambiguate by explicitly\nsaying \"refs/heads/\" followed by \"deadbeef\" five times, so it\nshould be OK.\n\n> +\n> +\t\tref = xcalloc(1, sizeof(*ref) + len + 1);\n> +\t\tmemcpy(ref->old_sha1, sha1, 20);\n> +\t\tmemcpy(ref->name, s, len + 1);\n> +\t\t*refs = ref;\n> +\t\trefs = &ref->next;\n> +\t}\n> +\treturn refs;\n> +}\n> +\n\nThis function takes the pointer to a location that holds a\npointer to a \"struct ref\" -- it is the location to store the\nnewly allocated ref structure, i.e. the next pointer of the last\nelement in the list.  When it returns, the location pointed at\nby the pointer given to you points at the first element you\nallocated, and it returns the next pointer of the last element\nallocated by it.  That is the same calling convention as\nconnect.c::get_remote_heads().  So when calling this function to\nappend to a list you already have, you would give the next\npointer to the last element of the existing list.  But you do\nnot seem to do that.\n\nI think the body of fetch_pack() should become something like:\n\n\tstruct ref *ref, **tail;\n\n        tail = get_remote_heads(fd[0], &ref, 0, NULL, 0);\n\tif (server_supports(\"multi_ack\")) {\n\t\t...\n\t}\n\ttail = get_sha1_heads(tail, nr_match, match);\n\tif (everything_local(&ref, nr_match, match)) {\n\t\t...\n\n> @@ -311,6 +332,8 @@ static int everything_local(struct ref *\n>  \tif (cutoff)\n>  \t\tmark_recent_complete_commits(cutoff);\n>  \n> +\tfilter_refs(refs, nr_match, match);\n> +\n\nI am not sure about this change.\n\nIn the original code we do not let get_remote_heads() to filter\nthe refs but call filter_refs() after the \"mark all complete\nremote refs as common\" step for a reason.  Even though we may\nnot be fetching from some remote refs, we would want to take\nadvantage of the knowledge of what objects they have so that we\ncan mark as many objects as common as possible in the early\nstage.  I suspect this change defeats that optimization.\n\nSo instead I would teach \"mark all complete remote refs\" loop\nthat not everything in refs list is a valid remote ref, and skip\nwhat get_sha1_heads() injected, because these arbitrary ones we\ngot from the command line are not something we know exist on the\nremote side.  Maybe something like this.\n\n\t/*\n\t * Mark all complete remote refs as common refs.\n\t * Don't mark them common yet; the server has to be told so first.\n\t */\n\tfor (ref = *refs; ref; ref = ref->next) {\n\t\tstruct object *o;\n                if (ref is SHA1 from the command line)\n                \tcontinue;\n\t\to = deref_tag(lookup_object(ref->old_sha1), NULL, 0);\n\t\tif (!o || o->type != commit_type || !(o->flags & COMPLETE))\n\t\t\tcontinue;\n\t\t...\n\nTo implement \"ref is SHA1 from the command line\", I would add\nanother 1-bit field to \"struct ref\" and mark the new ones you\ncreate in get_sha1_heads() as such (existing \"force\" field\ncould also become an 1-bit field -- we do not neeed a char).\n\n> @@ -373,6 +394,7 @@ static int fetch_pack(int fd[2], int nr_\n>  \t\tpacket_flush(fd[1]);\n>  \t\tdie(\"no matching remote head\");\n>  \t}\n> +\tget_sha1_heads(&ref, nr_match, match);\n\nI talked about this one already...\n\n> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n> index 187f088..2372df8 100755\n> --- a/git-parse-remote.sh\n> +++ b/git-parse-remote.sh\n> @@ -105,6 +105,7 @@ canon_refs_list_for_fetch () {\n>  \t\t'') remote=HEAD ;;\n>  \t\trefs/heads/* | refs/tags/* | refs/remotes/*) ;;\n>  \t\theads/* | tags/* | remotes/* ) remote=\"refs/$remote\" ;;\n> +\t\t[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]) ;;\n\nYuck.  Don't we have $_x40 somewhere?\n\nWe never use uppercase so at least we could save 24 columns from\nhere ;-).\n"},{"id":"20766","messageId":"m1ac94bsjr.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7vac95m799.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-05-26T17:32:40Z","receivedAt":"2006-05-26T17:32:40Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"Junio C Hamano <junkio@cox.net> writes:\n\n> ebiederm@xmission.com (Eric W. Biederman) writes:\n>\n>> diff --git a/fetch-pack.c b/fetch-pack.c\n>> index a3bcad0..c767d84 100644\n>> --- a/fetch-pack.c\n>> +++ b/fetch-pack.c\n>> @@ -260,6 +260,27 @@ static void mark_recent_complete_commits\n>>  \t}\n>>  }\n>>  \n>> +static struct ref **get_sha1_heads(struct ref **refs, int nr_heads, char\n> **head)\n>> +{\n>> +\tint i;\n>> +\tfor (i  = 0; i < nr_heads; i++) {\n>> +\t\tstruct ref *ref;\n>> +\t\tunsigned char sha1[20];\n>> +\t\tchar *s = head[i];\n>> +\t\tint len = strlen(s);\n>> +\n>> +\t\tif (len != 40 || get_sha1_hex(s, sha1))\n>> +\t\t\tcontinue;\n>\n> So the new convention is fetch-pack can take ref name (as\n> before), or a bare 40-byte hexadecimal.  I think sane people\n> would not use ambiguous refname that says \"deadbeef\" five times,\n> and even if the do so they could disambiguate by explicitly\n> saying \"refs/heads/\" followed by \"deadbeef\" five times, so it\n> should be OK.\n\nYes.\n\n>> +\n>> +\t\tref = xcalloc(1, sizeof(*ref) + len + 1);\n>> +\t\tmemcpy(ref->old_sha1, sha1, 20);\n>> +\t\tmemcpy(ref->name, s, len + 1);\n>> +\t\t*refs = ref;\n>> +\t\trefs = &ref->next;\n>> +\t}\n>> +\treturn refs;\n>> +}\n>> +\n>\n> This function takes the pointer to a location that holds a\n> pointer to a \"struct ref\" -- it is the location to store the\n> newly allocated ref structure, i.e. the next pointer of the last\n> element in the list.  When it returns, the location pointed at\n> by the pointer given to you points at the first element you\n> allocated, and it returns the next pointer of the last element\n> allocated by it.  That is the same calling convention as\n> connect.c::get_remote_heads().  So when calling this function to\n> append to a list you already have, you would give the next\n> pointer to the last element of the existing list.  But you do\n> not seem to do that.\n\nAck. That does look like a bug.  I knew there as something\nfishy about that code.  But it worked for my basic testing so I didn't\nworry about it.\n\n> I think the body of fetch_pack() should become something like:\n>\n> \tstruct ref *ref, **tail;\n>\n>         tail = get_remote_heads(fd[0], &ref, 0, NULL, 0);\n> \tif (server_supports(\"multi_ack\")) {\n> \t\t...\n> \t}\n> \ttail = get_sha1_heads(tail, nr_match, match);\n> \tif (everything_local(&ref, nr_match, match)) {\n> \t\t...\n\nActually because we want the filter to resolve sha1s by\ndefault in terms of what was passed on the command line.  I'm pretty\ncertain that should be:\n\n\ttail = get_sha1_heads(&ref, nr_match, match);\n\ttail = get_remote_heads(fd[0], tail, 0, NULL, 0);\n        ...\n\n\n>> @@ -311,6 +332,8 @@ static int everything_local(struct ref *\n>>  \tif (cutoff)\n>>  \t\tmark_recent_complete_commits(cutoff);\n>>  \n>> +\tfilter_refs(refs, nr_match, match);\n>> +\n>\n> I am not sure about this change.\n\nAgreed.  It was a hold over from an earlier way of injecting\nthe sha1 into the logic.  \n\nAs for what happens I think I need to audit everything that\ntakes a ref from fetch_pack.  To make certain I have not\nmessed up the logic.\n\n> In the original code we do not let get_remote_heads() to filter\n> the refs but call filter_refs() after the \"mark all complete\n> remote refs as common\" step for a reason.  Even though we may\n> not be fetching from some remote refs, we would want to take\n> advantage of the knowledge of what objects they have so that we\n> can mark as many objects as common as possible in the early\n> stage.  I suspect this change defeats that optimization.\n\nIt feels like it.\n\n> So instead I would teach \"mark all complete remote refs\" loop\n> that not everything in refs list is a valid remote ref, and skip\n> what get_sha1_heads() injected, because these arbitrary ones we\n> got from the command line are not something we know exist on the\n> remote side.  Maybe something like this.\n\nSounds sane.  We also introduce a new possibility of having a\nref that is complete but not remote.\n\n> \t/*\n> \t * Mark all complete remote refs as common refs.\n> \t * Don't mark them common yet; the server has to be told so first.\n> \t */\n> \tfor (ref = *refs; ref; ref = ref->next) {\n> \t\tstruct object *o;\n>                 if (ref is SHA1 from the command line)\n>                 \tcontinue;\n> \t\to = deref_tag(lookup_object(ref->old_sha1), NULL, 0);\n> \t\tif (!o || o->type != commit_type || !(o->flags & COMPLETE))\n> \t\t\tcontinue;\n> \t\t...\n>\n> To implement \"ref is SHA1 from the command line\", I would add\n> another 1-bit field to \"struct ref\" and mark the new ones you\n> create in get_sha1_heads() as such (existing \"force\" field\n> could also become an 1-bit field -- we do not neeed a char).\n\nSounds sane.\nSo that gives me:\n\tunsigned int force : 1;\n\tunsigned int injected : 1;\n\nWhich aligns them to an int boundary but since we are followed\nimmediately by a pointer should result in no additional storage being\nconsumed.\n\n>> @@ -373,6 +394,7 @@ static int fetch_pack(int fd[2], int nr_\n>>  \t\tpacket_flush(fd[1]);\n>>  \t\tdie(\"no matching remote head\");\n>>  \t}\n>> +\tget_sha1_heads(&ref, nr_match, match);\n>\n> I talked about this one already...\n>\n>> diff --git a/git-parse-remote.sh b/git-parse-remote.sh\n>> index 187f088..2372df8 100755\n>> --- a/git-parse-remote.sh\n>> +++ b/git-parse-remote.sh\n>> @@ -105,6 +105,7 @@ canon_refs_list_for_fetch () {\n>>  \t\t'') remote=HEAD ;;\n>>  \t\trefs/heads/* | refs/tags/* | refs/remotes/*) ;;\n>>  \t\theads/* | tags/* | remotes/* ) remote=\"refs/$remote\" ;;\n>> +\n> [0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F])\n> ;;\n>\n> Yuck.  Don't we have $_x40 somewhere?\n\nI couldn't find one in shell.  \n\n> We never use uppercase so at least we could save 24 columns from\n> here ;-).\n\nI'm not certain why we always add make $remote=\"refs/heads/$remote\" by\ndefault in that switch statement. git-fetch-pack at least doesn't need\nit.\n\nIf that is true of the other consumers we could easily make the test:\n[0-9a-fA-F][0-9a-fA-F][0-9a-fA-F][0-9a-fA-F]*) ;;\nOr even simply make the default case *) ;;\n\nBut for the moment I will stick to the long form because it is\nobviously correct.\n\nEric\n"},{"id":"21417","messageId":"m164jc9ekx.fsf@ebiederm.dsl.xmission.com","threadId":"4274","inReplyTo":"7virntsto6.fsf@assigned-by-dhcp.cox.net","subject":"Re: [RFC][PATCH] Allow transfer of any valid sha1","fromName":"Eric W. Biederman","fromEmail":"ebiederm@xmission.com","sentAt":"2006-06-08T09:33:34Z","receivedAt":"2006-06-08T09:33:34Z","isPatch":true,"sender":{"key":"ebiederm@xmission.com","avatar":"https://avatars.githubusercontent.com/u/7477136?v=4"},"body":"\nA quick status update.\n\nI think I have clean working version of the sha1 transfer code,\nI left on vacation before I could send it out so I need to dig\nit out and make certain everything still applies.\n\nI finally figured out what my problem pulling Andrew's changes\nwere.  git-quiltimport remembers what the previous commit was and when\nI added merging I forgot to update that the variable that stores\nthe previous commit.  So since I had the history wrong git-merge\nwas finding the wrong common ancestor, which is an easy way\nto mess up an automatic merge :)\n\n> The last time I talked with Andrew, he is not doing a merge nor\n> resolving merge conflicts.  He treats git primarily as a\n> patchbomb distribution mechanism, and works on (a rough\n> equivalent of) the output of format-patch from merge base\n> between his base tree and individual subsystem tree.  After that\n> things are normal quilt workflow outside git, whatever it is.\n\nAndrews git import does appear to be a git-pull from an appropriate\ntree and then a diff of the automatic merge result, so while\nthere doesn't appear to be manual merging there is a little\nbit of automatic merging going on.\n\nAnyway when I wake up in the morning I should see if I have\nsuccessfully imported Andres 2.6.17-rc5-mm3 tree.   All of that\npulling of git trees on demand noticeably slows down the import \non my dinky test machine.  I'm not certain how much of that\na machine that had plenty of memory would see though.\n\nEric\n"}]}