promisor-remote: lazy-fetch objects in subprocess

Teach Git to lazy-fetch missing objects in a subprocess instead of doing
it in-process. This allows any fatal errors that occur during the fetch
to be isolated and converted into an error return value, instead of
causing the current command being run to terminate.

Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
Signed-off-by: Junio C Hamano <gitster@pobox.com>

diff --git a/Documentation/technical/partial-clone.txt b/Documentation/technical/partial-clone.txt
index b9e17e7..0780d30 100644 (file)
--- a/Documentation/technical/partial-clone.txt
+++ b/Documentation/technical/partial-clone.txt
@@ -171,20 +171,13 @@ additional flag.
 Fetching Missing Objects
 ------------------------
 
-- Fetching of objects is done using the existing transport mechanism using
-  transport_fetch_refs(), setting a new transport option
-  TRANS_OPT_NO_DEPENDENTS to indicate that only the objects themselves are
-  desired, not any object that they refer to.
-+
-Because some transports invoke fetch_pack() in the same process, fetch_pack()
-has been updated to not use any object flags when the corresponding argument
-(no_dependents) is set.
+- Fetching of objects is done by invoking a "git fetch" subprocess.
 
 - The local repository sends a request with the hashes of all requested
-  objects as "want" lines, and does not perform any packfile negotiation.
+  objects, and does not perform any packfile negotiation.
   It then receives a packfile.
 
-- Because we are reusing the existing fetch-pack mechanism, fetching
+- Because we are reusing the existing fetch mechanism, fetching
   currently fetches all objects referred to by the requested objects, even
   though they are not necessary.
 

diff --git a/promisor-remote.c b/promisor-remote.c
index baaea12..6530e26 100644 (file)
--- a/promisor-remote.c
+++ b/promisor-remote.c
@@ -3,6 +3,7 @@
 #include "promisor-remote.h"
 #include "config.h"
 #include "transport.h"
+#include "strvec.h"
 
 static char *repository_format_partial_clone;
 static const char *core_partial_clone_filter_default;
@@ -12,39 +13,34 @@ void set_repository_format_partial_clone(char *partial_clone)
        repository_format_partial_clone = xstrdup_or_null(partial_clone);
 }
 
-static int fetch_refs(const char *remote_name, struct ref *ref)
-{
-       struct remote *remote;
-       struct transport *transport;
-       int res;
-
-       remote = remote_get(remote_name);
-       if (!remote->url[0])
-               die(_("Remote with no URL"));
-       transport = transport_get(remote, remote->url[0]);
-
-       transport_set_option(transport, TRANS_OPT_FROM_PROMISOR, "1");
-       transport_set_option(transport, TRANS_OPT_NO_DEPENDENTS, "1");
-       res = transport_fetch_refs(transport, ref);
-
-       return res;
-}
-
 static int fetch_objects(const char *remote_name,
                         const struct object_id *oids,
                         int oid_nr)
 {
-       struct ref *ref = NULL;
+       struct child_process child = CHILD_PROCESS_INIT;
        int i;
+       FILE *child_in;
+
+       child.git_cmd = 1;
+       child.in = -1;
+       strvec_pushl(&child.args, "-c", "fetch.negotiationAlgorithm=noop",
+                    "fetch", remote_name, "--no-tags",
+                    "--no-write-fetch-head", "--recurse-submodules=no",
+                    "--filter=blob:none", "--stdin", NULL);
+       if (start_command(&child))
+               die(_("promisor-remote: unable to fork off fetch subprocess"));
+       child_in = xfdopen(child.in, "w");
 
        for (i = 0; i < oid_nr; i++) {
-               struct ref *new_ref = alloc_ref(oid_to_hex(&oids[i]));
-               oidcpy(&new_ref->old_oid, &oids[i]);
-               new_ref->exact_oid = 1;
-               new_ref->next = ref;
-               ref = new_ref;
+               if (fputs(oid_to_hex(&oids[i]), child_in) < 0)
+                       die_errno(_("promisor-remote: could not write to fetch subprocess"));
+               if (fputc('\n', child_in) < 0)
+                       die_errno(_("promisor-remote: could not write to fetch subprocess"));
        }
-       return fetch_refs(remote_name, ref);
+
+       if (fclose(child_in) < 0)
+               die_errno(_("promisor-remote: could not close stdin to fetch subprocess"));
+       return finish_command(&child) ? -1 : 0;
 }
 
 static struct promisor_remote *promisors;

diff --git a/t/t0410-partial-clone.sh b/t/t0410-partial-clone.sh
index a5ebdf9..d681e90 100755 (executable)
--- a/t/t0410-partial-clone.sh
+++ b/t/t0410-partial-clone.sh
@@ -214,7 +214,7 @@ test_expect_success 'fetching of missing objects works with ref-in-want enabled'
        rm -rf repo/.git/objects/* &&
        rm -f trace &&
        GIT_TRACE_PACKET="$(pwd)/trace" git -C repo cat-file -p "$HASH" &&
-       grep "git< fetch=.*ref-in-want" trace
+       grep "fetch< fetch=.*ref-in-want" trace
 '
 
 test_expect_success 'fetching of missing objects from another promisor remote' '

diff --git a/t/t4067-diff-partial-clone.sh b/t/t4067-diff-partial-clone.sh
index ef8e0e9..804f2a8 100755 (executable)
--- a/t/t4067-diff-partial-clone.sh
+++ b/t/t4067-diff-partial-clone.sh
@@ -20,7 +20,7 @@ test_expect_success 'git show batches blobs' '
        # Ensure that there is exactly 1 negotiation by checking that there is
        # only 1 "done" line sent. ("done" marks the end of negotiation.)
        GIT_TRACE_PACKET="$(pwd)/trace" git -C client show HEAD &&
-       grep "git> done" trace >done_lines &&
+       grep "fetch> done" trace >done_lines &&
        test_line_count = 1 done_lines
 '
 
@@ -44,7 +44,7 @@ test_expect_success 'diff batches blobs' '
        # Ensure that there is exactly 1 negotiation by checking that there is
        # only 1 "done" line sent. ("done" marks the end of negotiation.)
        GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff HEAD^ HEAD &&
-       grep "git> done" trace >done_lines &&
+       grep "fetch> done" trace >done_lines &&
        test_line_count = 1 done_lines
 '
 
@@ -127,7 +127,7 @@ test_expect_success 'diff with rename detection batches blobs' '
        # only 1 "done" line sent. ("done" marks the end of negotiation.)
        GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff --raw -M HEAD^ HEAD >out &&
        grep ":100644 100644.*R[0-9][0-9][0-9].*b.*c" out &&
-       grep "git> done" trace >done_lines &&
+       grep "fetch> done" trace >done_lines &&
        test_line_count = 1 done_lines
 '
 
@@ -175,7 +175,7 @@ test_expect_success 'diff --break-rewrites fetches only if necessary, and batche
        # by checking that there is only 1 "done" line sent. ("done" marks the
        # end of negotiation.)
        GIT_TRACE_PACKET="$(pwd)/trace" git -C client diff --break-rewrites --raw -M HEAD^ HEAD &&
-       grep "git> done" trace >done_lines &&
+       grep "fetch> done" trace >done_lines &&
        test_line_count = 1 done_lines
 '
 

diff --git a/t/t5300-pack-object.sh b/t/t5300-pack-object.sh
index 3d6a933..392201c 100755 (executable)
--- a/t/t5300-pack-object.sh
+++ b/t/t5300-pack-object.sh
@@ -528,7 +528,7 @@ test_expect_success 'prefetch objects' '
        TWO=$(git -C server rev-parse three_branch^) &&
        git -C client fetch --filter=blob:none origin "$TWO" &&
        GIT_TRACE_PACKET=$(pwd)/trace git -C client push origin "$TWO":refs/heads/two_branch &&
-       grep "git> done" trace >donelines &&
+       grep "fetch> done" trace >donelines &&
        test_line_count = 1 donelines
 '
 

diff --git a/t/t5601-clone.sh b/t/t5601-clone.sh
index eb9a093..15fb64c 100755 (executable)
--- a/t/t5601-clone.sh
+++ b/t/t5601-clone.sh
@@ -704,7 +704,7 @@ test_expect_success 'batch missing blob request during checkout' '
        # Ensure that there is only one negotiation by checking that there is
        # only "done" line sent. ("done" marks the end of negotiation.)
        GIT_TRACE_PACKET="$(pwd)/trace" git -C client checkout HEAD^ &&
-       grep "git> done" trace >done_lines &&
+       grep "fetch> done" trace >done_lines &&
        test_line_count = 1 done_lines
 '