Re: On --depth=funny value
On Wed, Jan 9, 2013 at 12:19 PM, Junio C Hamano gits...@pobox.com wrote: Duy Nguyen pclo...@gmail.com writes: On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano gits...@pobox.com wrote: ... * We would like to update clone --depth=1 to end up with a tip only repository, but let's not to touch git fetch (and git clone) and make them send 0 over the wire when the command line tells them to use --depth=1 (i.e. let's not do the off-by-one thing). You can't anyway. Depth 0 on the wire is considered invalid by upload-pack. Yes, that is a good point that we say if (0 opt-depth) do the shallow thing everywhere, so 0 is spcial in that sense. Which suggests that if we wanted to, we could update the fetch side to do the off-by-one thing against the current upload-pack when the given depth is two or more, and still send 1 when depth=1. When talking with an updated upload-pack that advertises exact-shallow protocol extension, it can disable that off-by-one for all values of depth. That way, the updated client gets history of wrong depth only for --depth=1 when talking with the current upload-pack; all other cases, it will get history of correct depth. Hmm? I haven't checked because frankly I have never run JGit, but are we sure this off-by-one thing applies to JGit server as well? So far I'm only aware of three sever implementations: C Git, JGit and Dulwich. The last one does not support shallow extension so it's out of question. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On --depth=funny value
On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano gits...@pobox.com wrote: * Make git fetch and git clone die() when zero or negative number is given with --depth=$N, for the following reasons: ... For Stefan when you update the patch. If git fetch --depth=0 is considered invalid too as Junio outlined, then the proper place for the check is transport.c:set_git_option(), not clone.c. It already catches --depth=random-string. Adding depth 1 check should be trivial. You may want to update builtin/fetch-pack.c too because it does not share this code. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On --depth=funny value
On 01/09/2013 03:53 AM, Junio C Hamano wrote: Can people sanity check the reasoning outlined here? Anything I missed? The above outline identifies three concrete tasks that different people can tackle more or less independently, each with updated code, documentation and test: 1. git fetch --unshallow that gives a pretty surface on Duy's --depth=inf; 2. Making git fetch and git clone die on --depth=0 or --depth=-4; 3 Updating upload-pack to count correctly. I'll refrain from saying Any takers? for now. Sorry for answering with delay, I am just contributing to git in my spare time. So if I understood Duy correctly, he is going to solve 1. and 3 by his patches. I'll try to come up with a solution for 2. within the next days. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
On --depth=funny value
Here to outline my current thinking. Note that this is unrelated to the git clone --bottom=v1.2.3 to say I do not care about anything that happened before that version. * First, let's *not* do git fetch --depth=inf; if you want to unplug the bottom of your shallow clone, be more explicit and introduce a new option, e.g. git fetch --unshallow, or something. * Make git fetch and git clone die() when zero or negative number is given with --depth=$N, for the following reasons: - The --depth option is describe as: (git clone) ... a 'shallow' clone with a history truncated to the specified number of revisions. (git fetch) Limit fetching to ancestor-chains not longer than n. It is fairly clear from the above that negative $N does not make any sense. Technically, fetching the commits that were explicitly asked for and not there parents is the only possible ancestor-chain that is not longer than -4, so git fetch --depth=-4 ought to behave just like git fetch --depth=0, but you have to be very sick to read the documentation and expect things to work that way. Also there is no way to misread the git clone documentation and expect git clone --depth=-4 to create a history truncated to negative number of revisions. Which means that it is the right thing to do to die() when a negative number is given to --depth for these commands. - As people count from one, the natural way to ask for the tip commit without any history ought to be --depth=1. Let's declare the current behaviour of --depth=1 that gives the tip and one commit behind it a bug. Which means that these commands should be updated to die() when zero is given to their --depth option. Do not give me any history is inherenty incompatibe with clone or fetch. Because there is no configuration variable fetch.depth (or clone.depth) that forces all your cloned repositories to be shallow, git clone --depth=0 or git fetch --depth=0 couldn't have been ways for existing users to ask to defeat any funny configured depth value and clone or fetch everything. When they wanted to clone or fetch everything, they would have just used the command without any --depth option instead. Which means that nobody gets hurt if we change these commands to die() when zero is given to their --depth option. * We would like to update clone --depth=1 to end up with a tip only repository, but let's not to touch git fetch (and git clone) and make them send 0 over the wire when the command line tells them to use --depth=1 (i.e. let's not do the off-by-one thing). Instead, fix upload-pack (the bug is in get_shallow_commits() in shallow.c, I think), so that it counts correctly. When the other end asks for a history with 1-commit deep, it should return a history that is 1-commit deep, and tell the other end about the parents of the returned history, instead of returning a history that is 2 commmits deep. So when talking with a newer server, clients will get correct number of commits; when talkng with an older server, clients will get a bit more than they asked, but nothing will break. Can people sanity check the reasoning outlined here? Anything I missed? The above outline identifies three concrete tasks that different people can tackle more or less independently, each with updated code, documentation and test: 1. git fetch --unshallow that gives a pretty surface on Duy's --depth=inf; 2. Making git fetch and git clone die on --depth=0 or --depth=-4; 3 Updating upload-pack to count correctly. I'll refrain from saying Any takers? for now. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On --depth=funny value
On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano gits...@pobox.com wrote: * First, let's *not* do git fetch --depth=inf; if you want to unplug the bottom of your shallow clone, be more explicit and introduce a new option, e.g. git fetch --unshallow, or something. No problem. Something could be --no-depth or --no-shallow. I think any of them is better than --unshallow. * We would like to update clone --depth=1 to end up with a tip only repository, but let's not to touch git fetch (and git clone) and make them send 0 over the wire when the command line tells them to use --depth=1 (i.e. let's not do the off-by-one thing). You can't anyway. Depth 0 on the wire is considered invalid by upload-pack. Instead, fix upload-pack (the bug is in get_shallow_commits() in shallow.c, I think), so that it counts correctly. When the other end asks for a history with 1-commit deep, it should return a history that is 1-commit deep, and tell the other end about the parents of the returned history, instead of returning a history that is 2 commmits deep. So when talking with a newer server, clients will get correct number of commits; when talkng with an older server, clients will get a bit more than they asked, but nothing will break. I'll need to look at get_shallow_commits() anyway for the unshallow patch. I'll probably do this too. -- Duy -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: On --depth=funny value
Duy Nguyen pclo...@gmail.com writes: On Wed, Jan 9, 2013 at 9:53 AM, Junio C Hamano gits...@pobox.com wrote: ... * We would like to update clone --depth=1 to end up with a tip only repository, but let's not to touch git fetch (and git clone) and make them send 0 over the wire when the command line tells them to use --depth=1 (i.e. let's not do the off-by-one thing). You can't anyway. Depth 0 on the wire is considered invalid by upload-pack. Yes, that is a good point that we say if (0 opt-depth) do the shallow thing everywhere, so 0 is spcial in that sense. Which suggests that if we wanted to, we could update the fetch side to do the off-by-one thing against the current upload-pack when the given depth is two or more, and still send 1 when depth=1. When talking with an updated upload-pack that advertises exact-shallow protocol extension, it can disable that off-by-one for all values of depth. That way, the updated client gets history of wrong depth only for --depth=1 when talking with the current upload-pack; all other cases, it will get history of correct depth. Hmm? -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
