fetch/receive: remove over-pessimistic connectivity check

Git 1.7.8 introduced an object and history re-validation step after
"fetch" or "push" causes new history to be added to a receiving
repository. This is to protect a malicious server or pushing client from
corrupting the repository by taking advantage of an existing corrupt
object that is unconnected to existing history.

But this check is way over-pessimistic.  During "fetch" or "receive-pack"
(the server side of "push"), unpack-objects and index-pack already
validate individual objects that are received, and the only thing we would
want to catch are corrupted objects that already happen to exist in our
repository but are not referenced from our refs.  Such objects must have
been written by an earlier run of our codepaths that write out loose
objects or packfiles, and they must have done the validation of individual
objects when they did so.  The only thing left to worry about is the
connectivity integrity, which can be checked with "rev-list --objects",
which is much cheaper.  We have been paying the 5x to 8x runtime overhead
the --verify-objects often adds for no real gain.

Revert check_everything_connected() not to use this over-pessimistic
check.

Credit goes to Nguyễn Thái Ngọc Duy, who originally identified the
performance regression and endured multiple rounds of reviews to fix it.

Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/connected.c b/connected.c
index d7624230d435ded5ef7ba126ef77ab0e888af19b..1e89c1cd1d63f160bf7c95de41be61ff2d888e70 100644 (file)
--- a/connected.c
+++ b/connected.c
@@ -6,18 +6,18 @@
 /*
  * If we feed all the commits we want to verify to this command
  *
 /*
  * If we feed all the commits we want to verify to this command
  *

- *  $ git rev-list --verify-objects --stdin --not --all
+ *  $ git rev-list --objects --stdin --not --all

  *
  * and if it does not error out, that means everything reachable from
  *
  * and if it does not error out, that means everything reachable from

- * these commits locally exists and is connected to some of our
- * existing refs.
+ * these commits locally exists and is connected to our existing refs.
+ * Note that this does _not_ validate the individual objects.

  *
  * Returns 0 if everything is connected, non-zero otherwise.
  */
 int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
 {
        struct child_process rev_list;
  *
  * Returns 0 if everything is connected, non-zero otherwise.
  */
 int check_everything_connected(sha1_iterate_fn fn, int quiet, void *cb_data)
 {
        struct child_process rev_list;

-       const char *argv[] = {"rev-list", "--verify-objects",
+       const char *argv[] = {"rev-list", "--objects",

                              "--stdin", "--not", "--all", NULL, NULL};
        char commit[41];
        unsigned char sha1[20];
                              "--stdin", "--not", "--all", NULL, NULL};
        char commit[41];
        unsigned char sha1[20];