Re: A local shared clone is now much slower
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 8, 2013, 15:00 UTC
- Message-ID
- <7vbo6d2j2e.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <CACsJy8Chmm0=wDV4NQ+4Gh7KZYpbd9qkb=pNzkPeG-a-xiwVmw@mail.gmail.com>
Duy Nguyen <pclouds@gmail.com> writes:
Show 21 quoted lines
> On Mon, Jul 8, 2013 at 2:30 PM, Jeff King <peff@peff.net> wrote: >> Subject: [PATCH] clone: drop connectivity check for local clones >> >> Commit 0433ad1 (clone: run check_everything_connected, >> 2013-03-25) added the same connectivity check to clone that >> we use for fetching. The intent was to provide enough safety >> checks that "git clone git://..." could be counted on to >> detect bit errors and other repo corruption, and not >> silently propagate them to the clone. >> >> For local clones, this turns out to be a bad idea, for two >> reasons: >> >> 1. Local clones use hard linking (or even shared object >> stores), and so complete far more quickly. The time >> spent on the connectivity check is therefore >> proportionally much more painful. > > There's also byte-to-byte copy when system does not support hardlinks > (or the user does not want it) but I guess it's safe to trust the OS > to copy correctly in most cases.
While that may be true, I do not think it matters that much. The check during transport is meant to guard against not just corruption during the object transfer, but also against a corrupt source repository, and your trust on "cp -R" only covers the "transfer" part. And that makes 2. below very relevant.
Show 5 quoted lines
>> 2. Local clones do not actually meet our safety guarantee >> anyway. >> ... > > Faster clones make everybody happy :-)
Yup.
I think this deserves to be backported to 'maint' track for 1.8.3.x. Here is an attempt to do so.
builtin/clone.c | 11 +++++++---- t/t5710-info-alternate.sh | 8 +++++++- 2 files changed, 14 insertions(+), 5 deletions(-)
diff --git a/builtin/clone.c b/builtin/clone.c index 035ab64..38a0a64 100644 --- a/builtin/clone.c +++ b/builtin/clone.c @@ -542,12 +542,15 @@ static void update_remote_refs(const struct ref *refs, const struct ref *mapped_refs, const struct ref *remote_head_points_at, const char *branch_top, - const char *msg) + const char *msg, + int check_connectivity) { const struct ref *rm = mapped_refs; - if (check_everything_connected(iterate_ref_map, 0, &rm)) - die(_("remote did not send all necessary objects")); + if (check_connectivity) { + if (check_everything_connected(iterate_ref_map, 0, &rm)) + die(_("remote did not send all necessary objects")); + } if (refs) { write_remote_refs(mapped_refs); @@ -950,7 +953,7 @@ int cmd_clone(int argc, const char **argv, const char *prefix) transport_fetch_refs(transport, mapped_refs); update_remote_refs(refs, mapped_refs, remote_head_points_at, - branch_top.buf, reflog_msg.buf); + branch_top.buf, reflog_msg.buf, !is_local); update_head(our_head_points_at, remote_head, reflog_msg.buf); diff --git a/t/t5710-info-alternate.sh b/t/t5710-info-alternate.sh index 8956c21..5a6e49d 100755 --- a/t/t5710-info-alternate.sh +++ b/t/t5710-info-alternate.sh @@ -58,7 +58,13 @@ test_expect_success 'creating too deep nesting' \ git clone -l -s D E && git clone -l -s E F && git clone -l -s F G && -test_must_fail git clone --bare -l -s G H' +git clone --bare -l -s G H' + +test_expect_success 'invalidity of deepest repository' \ +'cd H && { + test_valid_repo + test $? -ne 0 +}' cd "$base_dir"