git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Matthijs Kooijman <matthijs@stdin.nl>
To: Junio C Hamano <gitster@pobox.com>,
	Duy Nguyen <pclouds@gmail.com>,
	Jonathan Nieder <jrnieder@gmail.com>,
	Stefan Beller <stefanbeller@googlemail.com>,
	schlotter@users.sourceforge.net, Ralf.Wildenhues@gmx.de,
	git@vger.kernel.org
Subject: Re: [PATCH] git clone depth of 0 not possible.
Date: Thu, 11 Jul 2013 12:57:33 +0200	[thread overview]
Message-ID: <20130711105733.GG10217@login.drsnuggles.stderr.nl> (raw)
In-Reply-To: <20130709133542.GJ10217@login.drsnuggles.stderr.nl>

Hi Junio,

> While implementing the above, I noticed my fix now introduced an
> off-by-one error the other way. When investigating, I found this commit:
> 
> 	commit 682c7d2f1a2d1a5443777237450505738af2ff1a
> 	Author: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> 	Date:   Fri Jan 11 16:05:47 2013 +0700
> 
> 	    upload-pack: fix off-by-one depth calculation in shallow clone
> 
> 	    get_shallow_commits() is used to determine the cut points at a given
> 	    depth (i.e. the number of commits in a chain that the user likes to
> 	    get). However we count current depth up to the commit "commit" but we
> 	    do the cutting at its parents (i.e. current depth + 1). This makes
> 	    upload-pack always return one commit more than requested. This patch
> 	    fixes it.
> 
> 	    Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> 	    Signed-off-by: Junio C Hamano <gitster@pobox.com>
> 
> Which actually seems to fix the off-by-one bug that is described in this
> thread, but without going through the hoops of preserving current
> behaviour for older git versions (that is, it makes behaviour dependent
> on server version instead of client version).
> 
> Does this mean the discussion in this thread is meaningless, or is that
> commit not intended to be the final fix?
Looking more closely, I also see that the above change is already
released in 1.8.2 versions. Given that, I don't think it makes sense to
to still try to provide this capability to get backward compatible
behaviour, since this would cause a off-by-one error the other way when
talking to 1.8.2.x servers...

However, since I pretty much finished the code for this, I'll send over
the patches and let you decide wether to include them or not. If you
want to include them but they need to be changed in some way, just let
me know.

The first patch of the series should be merged regardless.

Gr.

Matthijs

  reply	other threads:[~2013-07-11 10:57 UTC|newest]

Thread overview: 29+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2013-01-07 18:06 Stefan Beller
2013-01-07 18:06 ` [PATCH] Documentation on depth option in git clone Stefan Beller
2013-01-08  6:28 ` [PATCH] git clone depth of 0 not possible Jonathan Nieder
2013-01-08  6:54   ` Junio C Hamano
2013-01-08  7:36     ` Junio C Hamano
2013-01-08  8:19       ` Junio C Hamano
2013-01-08 14:28       ` Duy Nguyen
2013-01-08 14:32         ` Stefan Beller
2013-01-08 14:45           ` Duy Nguyen
2013-01-08 17:24           ` Junio C Hamano
2013-01-08  7:38     ` Duy Nguyen
2013-01-08  8:05       ` Junio C Hamano
2013-05-28  9:18         ` Matthijs Kooijman
2013-05-28 16:28           ` Jonathan Nieder
2013-05-28 16:31             ` Jonathan Nieder
2013-05-28 16:34             ` Matthijs Kooijman
2013-05-28 16:58               ` Jonathan Nieder
2013-05-28 17:04           ` Junio C Hamano
2013-05-30  8:23             ` Matthijs Kooijman
2013-06-02 19:14               ` Junio C Hamano
2013-07-09 13:35             ` Matthijs Kooijman
2013-07-11 10:57               ` Matthijs Kooijman [this message]
2013-07-11 11:25                 ` [PATCH 1/3] upload-pack: Remove a piece of dead code Matthijs Kooijman
2013-07-11 11:25                   ` [PATCH 2/3] upload-pack: Introduce new "fixed-off-by-one-depth" server feature Matthijs Kooijman
2013-07-11 11:25                   ` [PATCH 3/3] fetch-pack: Request fixed-off-by-one-depth when available Matthijs Kooijman
2013-07-11 12:08                   ` [PATCH 1/3] upload-pack: Remove a piece of dead code Duy Nguyen
2013-07-11 15:49                     ` Junio C Hamano
2013-01-08  7:33 ` [PATCH] git clone depth of 0 not possible Duy Nguyen
2013-01-08  7:37   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20130711105733.GG10217@login.drsnuggles.stderr.nl \
    --to=matthijs@stdin.nl \
    --cc=Ralf.Wildenhues@gmx.de \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=jrnieder@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=schlotter@users.sourceforge.net \
    --cc=stefanbeller@googlemail.com \
    --subject='Re: [PATCH] git clone depth of 0 not possible.' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).