{"thread":{"id":"46286","subject":"RFC: Missing blob hook might be invoked infinitely recursively","startedAt":"2017-06-29T18:48:47Z","lastAt":"2017-06-30T18:28:32Z","messageCount":3,"participants":["Jonathan Tan","Jonathan Nieder","Jeff Hostetler"],"isPatch":false,"patchVersion":null,"patchTotal":null},"messages":[{"id":"323493","messageId":"20170629114830.39dc2161@twelve2.svl.corp.google.com","threadId":"46286","inReplyTo":null,"subject":"RFC: Missing blob hook might be invoked infinitely recursively","fromName":"Jonathan Tan","fromEmail":"jonathantanmy@google.com","sentAt":"2017-06-29T18:48:30Z","receivedAt":"2017-06-29T18:48:47Z","isPatch":false,"sender":{"key":"jonathantanmy@fastmail.com","avatar":null},"body":"As some of you may know, I'm currently working on support for partial\nclones/fetches in Git (where blobs above a user-specified size threshold\nare not downloaded - only their names and sizes are downloaded). To do\nthis, the client repository needs to be able to download blobs at will\nwhenever it needs a missing one (for example, upon checkout).\n\nSo I have done this by adding support for a hook in Git [1], and\nupdating the object-reading code in Git to, by default, automatically\ninvoke this hook whenever necessary. (This means that existing\nsubsystems will all work by default, in theory at least.) My current\ndesign is for the hook to have maximum flexibility - when invoked with a\nlist of SHA-1s, it must merely ensure that those objects are in the\nlocal repo, whether packed or loose.\n\nI am also working on a command (fetch-blob) to be bundled with Git to be\nused as a default hook, and here is where the problem lies.\n\nSuppose you have missing blob AB12 and CD34 that you now need, so\nfetch-blob is invoked. It sends the literals AB12 and CD34 to a new\nserver endpoint and obtains a packfile, which it then pipes through \"git\nindex-pack\". The issue is that \"git index-pack\" wants to try to access\nAB12 and CD34 in the local repo in order to do a SHA-1 collision check,\nand therefore fetch-blob is invoked once again, creating infinite\nrecursion.\n\nThis is straightforwardly fixed by making \"git index-pack\" understand\nabout missing blobs, but this might be a symptom of this approach being\nerror-prone (custom hooks that invoke any Git command must be extra\ncareful).\n\nSo I have thought of a few solutions, each with its pros and cons:\n\n1. Require the hook to instead output a packfile to stdout. This means\nthat that hook no longer needs to access the local repo, and thus has\nless dependence on Git commands. But this reduces the flexibility in\nthat its output must be packed, not loose. (This is fine for the use\ncases I'm thinking of, but probably not so for others.)\n\n2. Add support for an environment variable to Git that suppresses access\nto the missing blob manifest, in effect, suppressing invocation of the\nhook. This allows anyone (the person configuring Git or the hook writer)\nto suppress this access, although they might need in-depth knowledge to\nknow whether the hook is meant to be run with such access suppressed or\nrequired.\n\n3. Like the above, except for a command-line argument to Git.\n\nWhat do you think? Any solutions that I am missing?\n\n[1] Work in progress, but you can see an earlier version here: https://public-inbox.org/git/b917a463f0ad4ce0ab115203b3f24894961a2e75.1497558851.git.jonathantanmy@google.com/\n"},{"id":"323495","messageId":"20170629192441.GA68015@aiede.mtv.corp.google.com","threadId":"46286","inReplyTo":"20170629114830.39dc2161@twelve2.svl.corp.google.com","subject":"Re: RFC: Missing blob hook might be invoked infinitely recursively","fromName":"Jonathan Nieder","fromEmail":"jrnieder@gmail.com","sentAt":"2017-06-29T19:24:41Z","receivedAt":"2017-06-29T19:25:20Z","isPatch":false,"sender":{"key":"jrnieder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/281595?v=4"},"body":"Hi,\n\nJonathan Tan wrote:\n\n> Suppose you have missing blob AB12 and CD34 that you now need, so\n> fetch-blob is invoked. It sends the literals AB12 and CD34 to a new\n> server endpoint and obtains a packfile, which it then pipes through \"git\n> index-pack\". The issue is that \"git index-pack\" wants to try to access\n> AB12 and CD34 in the local repo in order to do a SHA-1 collision check,\n> and therefore fetch-blob is invoked once again, creating infinite\n> recursion.\n[...]\n> 2. Add support for an environment variable to Git that suppresses access\n> to the missing blob manifest, in effect, suppressing invocation of the\n> hook. This allows anyone (the person configuring Git or the hook writer)\n> to suppress this access, although they might need in-depth knowledge to\n> know whether the hook is meant to be run with such access suppressed or\n> required.\n\nSmall tweak: what if Git itself sets that environment variable when\ninvoking the hook?  A fetch-blob hook author can then explicitly unset\nthe environment variable to request recursion if they need it.\n\nI should credit Shawn Pearce for saying this a little more clearly\noffline a moment ago.\n\nThanks,\nJonathan\n"},{"id":"323588","messageId":"3a68d8a2-8f4a-427b-f24a-fa215a1443fd@jeffhostetler.com","threadId":"46286","inReplyTo":"20170629114830.39dc2161@twelve2.svl.corp.google.com","subject":"Re: RFC: Missing blob hook might be invoked infinitely recursively","fromName":"Jeff Hostetler","fromEmail":"git@jeffhostetler.com","sentAt":"2017-06-30T18:28:14Z","receivedAt":"2017-06-30T18:28:32Z","isPatch":false,"sender":{"key":"git@jeffhostetler.com","avatar":null},"body":"\n\nOn 6/29/2017 2:48 PM, Jonathan Tan wrote:\n> As some of you may know, I'm currently working on support for partial\n> clones/fetches in Git (where blobs above a user-specified size threshold\n> are not downloaded - only their names and sizes are downloaded). To do\n> this, the client repository needs to be able to download blobs at will\n> whenever it needs a missing one (for example, upon checkout).\n> \n> So I have done this by adding support for a hook in Git [1], and\n> updating the object-reading code in Git to, by default, automatically\n> invoke this hook whenever necessary. (This means that existing\n> subsystems will all work by default, in theory at least.) My current\n> design is for the hook to have maximum flexibility - when invoked with a\n> list of SHA-1s, it must merely ensure that those objects are in the\n> local repo, whether packed or loose.\n> \n> I am also working on a command (fetch-blob) to be bundled with Git to be\n> used as a default hook, and here is where the problem lies.\n> \n> Suppose you have missing blob AB12 and CD34 that you now need, so\n> fetch-blob is invoked. It sends the literals AB12 and CD34 to a new\n> server endpoint and obtains a packfile, which it then pipes through \"git\n> index-pack\". The issue is that \"git index-pack\" wants to try to access\n> AB12 and CD34 in the local repo in order to do a SHA-1 collision check,\n> and therefore fetch-blob is invoked once again, creating infinite\n> recursion.\n> \n> This is straightforwardly fixed by making \"git index-pack\" understand\n> about missing blobs, but this might be a symptom of this approach being\n> error-prone (custom hooks that invoke any Git command must be extra\n> careful).\n\nI'm not sure if this what you meant, but could you pass to index-pack\nthe set of missing blobs that you passed to fetch-blob?  That is, let\nindex-pack do it's normal lookups, but just not on the list passed in.\n\n> \n> So I have thought of a few solutions, each with its pros and cons:\n> \n> 1. Require the hook to instead output a packfile to stdout. This means\n> that that hook no longer needs to access the local repo, and thus has\n> less dependence on Git commands. But this reduces the flexibility in\n> that its output must be packed, not loose. (This is fine for the use\n> cases I'm thinking of, but probably not so for others.)\n> \n> 2. Add support for an environment variable to Git that suppresses access\n> to the missing blob manifest, in effect, suppressing invocation of the\n> hook. This allows anyone (the person configuring Git or the hook writer)\n> to suppress this access, although they might need in-depth knowledge to\n> know whether the hook is meant to be run with such access suppressed or\n> required.\n> \n> 3. Like the above, except for a command-line argument to Git.\n> \n> What do you think? Any solutions that I am missing?\n> \n> [1] Work in progress, but you can see an earlier version here: https://public-inbox.org/git/b917a463f0ad4ce0ab115203b3f24894961a2e75.1497558851.git.jonathantanmy@google.com/\n> \n"}]}