Skip to content

Commit

Permalink
clone: drop connectivity check for local clones
Browse files Browse the repository at this point in the history
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.

  2. Local clones do not actually meet our safety guarantee
     anyway. The connectivity check makes sure we have all
     of the objects we claim to, but it does not check for
     bit errors. We will notice bit errors in commits and
     trees, but we do not load blob objects at all. Whereas
     over the pack transport, we actually recompute the sha1
     of each object in the incoming packfile; bit errors
     change the sha1 of the object, which is then caught by
     the connectivity check.

This patch drops the connectivity check in the local case.
Note that we have to revert the changes from 0433ad1 to
t5710, as we no longer notice the corruption during clone.

We could go a step further and provide a "verify even local
clones" option, but it is probably not worthwhile. You can
already spell that as "cd foo.git && git fsck && git clone ."
or as "git clone --no-local foo.git".

Signed-off-by: Jeff King <peff@peff.net>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
  • Loading branch information
Jeff King authored and Junio C Hamano committed Jul 8, 2013
1 parent edca415 commit 125a05f
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 5 deletions.
11 changes: 7 additions & 4 deletions builtin/clone.c
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down Expand Up @@ -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);

Expand Down
8 changes: 7 additions & 1 deletion t/t5710-info-alternate.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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"

Expand Down

0 comments on commit 125a05f

Please sign in to comment.