http: refactor finish_http_pack_request()

finish_http_pack_request() does multiple tasks, including some
housekeeping on a struct packed_git - (1) closing its index, (2)
removing it from a list, and (3) installing it. These concerns are
independent of fetching a pack through HTTP: they are there only because
(1) the calling code opens the pack's index before deciding to fetch it,
(2) the calling code maintains a list of packfiles that can be fetched,
and (3) the calling code fetches it in order to make use of its objects
in the same process.

In preparation for a subsequent commit, which adds a feature that does
not need any of this housekeeping, remove (1), (2), and (3) from
finish_http_pack_request(). (2) and (3) are now done by a helper
function, and (1) is the responsibility of the caller (in this patch,
done closer to the point where the pack index is opened).

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

diff --git a/http-push.c b/http-push.c
index 822f32659936d68de9bb7ee979a113fc12de2b65..ac7868ffeef5151a9dcc9fa2382d882f88e71aa6 100644 (file)
--- a/http-push.c
+++ b/http-push.c
@@ -117,6 +117,7 @@ enum transfer_state {
 
 struct transfer_request {
        struct object *obj;
+       struct packed_git *target;
        char *url;
        char *dest;
        struct remote_lock *lock;
@@ -314,17 +315,18 @@ static void start_fetch_packed(struct transfer_request *request)
                release_request(request);
                return;
        }
+       close_pack_index(target);
+       request->target = target;
 
        fprintf(stderr, "Fetching pack %s\n",
                hash_to_hex(target->hash));
        fprintf(stderr, " which contains %s\n", oid_to_hex(&request->obj->oid));
 
-       preq = new_http_pack_request(target, repo->url);
+       preq = new_http_pack_request(target->hash, repo->url);
        if (preq == NULL) {
                repo->can_update_info_refs = 0;
                return;
        }
-       preq->lst = &repo->packs;
 
        /* Make sure there isn't another open request for this pack */
        while (check_request) {
@@ -597,6 +599,8 @@ static void finish_request(struct transfer_request *request)
                }
                if (fail)
                        repo->can_update_info_refs = 0;
+               else
+                       http_install_packfile(request->target, &repo->packs);
                release_request(request);
        }
 }

diff --git a/http-walker.c b/http-walker.c
index fe15e325fabf76ffc01f0ce46504b8824f3b704b..4fb1235cd455464ad863a76097197f1c27827509 100644 (file)
--- a/http-walker.c
+++ b/http-walker.c
@@ -439,6 +439,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
        target = find_sha1_pack(sha1, repo->packs);
        if (!target)
                return -1;
+       close_pack_index(target);
 
        if (walker->get_verbosely) {
                fprintf(stderr, "Getting pack %s\n",
@@ -447,10 +448,9 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
                        hash_to_hex(sha1));
        }
 
-       preq = new_http_pack_request(target, repo->base);
+       preq = new_http_pack_request(target->hash, repo->base);
        if (preq == NULL)
                goto abort;
-       preq->lst = &repo->packs;
        preq->slot->results = &results;
 
        if (start_active_slot(preq->slot)) {
@@ -469,6 +469,7 @@ static int http_fetch_pack(struct walker *walker, struct alt_base *repo, unsigne
        release_http_pack_request(preq);
        if (ret)
                return ret;
+       http_install_packfile(target, &repo->packs);
 
        return 0;
 

diff --git a/http.c b/http.c
index 39cbd56702355ddc9dad7a3da9f68ce7e18b3061..4f6e1fb0188a7401a63e2bf01755dd3fd566f991 100644 (file)
--- a/http.c
+++ b/http.c
@@ -2268,22 +2268,13 @@ void release_http_pack_request(struct http_pack_request *preq)
 
 int finish_http_pack_request(struct http_pack_request *preq)
 {
-       struct packed_git **lst;
-       struct packed_git *p = preq->target;
        struct child_process ip = CHILD_PROCESS_INIT;
        int tmpfile_fd;
        int ret = 0;
 
-       close_pack_index(p);
-
        fclose(preq->packfile);
        preq->packfile = NULL;
 
-       lst = preq->lst;
-       while (*lst != p)
-               lst = &((*lst)->next);
-       *lst = (*lst)->next;
-
        tmpfile_fd = xopen(preq->tmpfile.buf, O_RDONLY);
 
        argv_array_push(&ip.args, "index-pack");
@@ -2297,15 +2288,26 @@ int finish_http_pack_request(struct http_pack_request *preq)
                goto cleanup;
        }
 
-       install_packed_git(the_repository, p);
 cleanup:
        close(tmpfile_fd);
        unlink(preq->tmpfile.buf);
        return ret;
 }
 
+void http_install_packfile(struct packed_git *p,
+                          struct packed_git **list_to_remove_from)
+{
+       struct packed_git **lst = list_to_remove_from;
+
+       while (*lst != p)
+               lst = &((*lst)->next);
+       *lst = (*lst)->next;
+
+       install_packed_git(the_repository, p);
+}
+
 struct http_pack_request *new_http_pack_request(
-       struct packed_git *target, const char *base_url)
+       const unsigned char *packed_git_hash, const char *base_url)
 {
        off_t prev_posn = 0;
        struct strbuf buf = STRBUF_INIT;
@@ -2313,14 +2315,13 @@ struct http_pack_request *new_http_pack_request(
 
        preq = xcalloc(1, sizeof(*preq));
        strbuf_init(&preq->tmpfile, 0);
-       preq->target = target;
 
        end_url_with_slash(&buf, base_url);
        strbuf_addf(&buf, "objects/pack/pack-%s.pack",
-               hash_to_hex(target->hash));
+               hash_to_hex(packed_git_hash));
        preq->url = strbuf_detach(&buf, NULL);
 
-       strbuf_addf(&preq->tmpfile, "%s.temp", sha1_pack_name(target->hash));
+       strbuf_addf(&preq->tmpfile, "%s.temp", sha1_pack_name(packed_git_hash));
        preq->packfile = fopen(preq->tmpfile.buf, "a");
        if (!preq->packfile) {
                error("Unable to open local file %s for pack",
@@ -2344,7 +2345,7 @@ struct http_pack_request *new_http_pack_request(
                if (http_is_verbose)
                        fprintf(stderr,
                                "Resuming fetch of pack %s at byte %"PRIuMAX"\n",
-                               hash_to_hex(target->hash),
+                               hash_to_hex(packed_git_hash),
                                (uintmax_t)prev_posn);
                http_opt_request_remainder(preq->slot->curl, prev_posn);
        }

diff --git a/http.h b/http.h
index 5e0ad724f92f3c708d2b330bb689f6eef6bb8de4..bbc6b070f148840d42d90e5692941aafd187916b 100644 (file)
--- a/http.h
+++ b/http.h
@@ -216,18 +216,25 @@ int http_get_info_packs(const char *base_url,
 
 struct http_pack_request {
        char *url;
-       struct packed_git *target;
-       struct packed_git **lst;
        FILE *packfile;
        struct strbuf tmpfile;
        struct active_request_slot *slot;
 };
 
 struct http_pack_request *new_http_pack_request(
-       struct packed_git *target, const char *base_url);
+       const unsigned char *packed_git_hash, const char *base_url);
 int finish_http_pack_request(struct http_pack_request *preq);
 void release_http_pack_request(struct http_pack_request *preq);
 
+/*
+ * Remove p from the given list, and invoke install_packed_git() on it.
+ *
+ * This is a convenience function for users that have obtained a list of packs
+ * from http_get_info_packs() and have chosen a specific pack to fetch.
+ */
+void http_install_packfile(struct packed_git *p,
+                          struct packed_git **list_to_remove_from);
+
 /* Helpers for fetching object */
 struct http_object_request {
        char *url;