Duy Nguyen <[email protected]> writes:
> On Mon, Jul 8, 2013 at 2:30 PM, Jeff King <[email protected]> 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 :-)
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"
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [email protected]
More majordomo info at http://vger.kernel.org/majordomo-info.html
