Duy Nguyen <pclo...@gmail.com> writes:

> On Mon, Jul 8, 2013 at 2:30 PM, Jeff King <p...@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.

>>   2. Local clones do not actually meet our safety guarantee
>>      anyway.
>>      ...
> Faster clones make everybody happy :-)


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) {
@@ -950,7 +953,7 @@ int cmd_clone(int argc, const char **argv, const char 
                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"
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

Reply via email to