From 7a8e3895f68e9ae4e44e521c78fc98768c2a88ec Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Wed, 27 May 2009 07:09:40 +0200 Subject: [PATCH 01/35] bisect: drop unparse_commit() and use clear_commit_marks() The goal of this patch series is to check if good revisions are ancestor of the bad revision without forking a process to launch "git rev-list $good ^$bad". This new version of this patch series does not use an "unparse_commit" function anymore, we use "clear_commit_marks" instead. Signed-off-by: Junio C Hamano --- bisect.c | 2 +- commit.c | 20 -------------------- commit.h | 2 -- 3 files changed, 1 insertion(+), 23 deletions(-) diff --git a/bisect.c b/bisect.c index c43c120bde..18f9fa4062 100644 --- a/bisect.c +++ b/bisect.c @@ -771,7 +771,7 @@ static int check_ancestors(const char *prefix) /* Clean up objects used, as they will be reused. */ for (i = 0; i < pending_copy.nr; i++) { struct object *o = pending_copy.objects[i].item; - unparse_commit((struct commit *)o); + clear_commit_marks((struct commit *)o, ALL_REV_FLAGS); } return res; diff --git a/commit.c b/commit.c index 8f6b703c55..aa3b35b6a8 100644 --- a/commit.c +++ b/commit.c @@ -316,26 +316,6 @@ int parse_commit(struct commit *item) return ret; } -static void unparse_commit_list(struct commit_list *list) -{ - for (; list; list = list->next) - unparse_commit(list->item); -} - -void unparse_commit(struct commit *item) -{ - item->object.flags = 0; - item->object.used = 0; - if (item->object.parsed) { - item->object.parsed = 0; - if (item->parents) { - unparse_commit_list(item->parents); - free_commit_list(item->parents); - item->parents = NULL; - } - } -} - struct commit_list *commit_list_insert(struct commit *item, struct commit_list **list_p) { struct commit_list *new_list = xmalloc(sizeof(struct commit_list)); diff --git a/commit.h b/commit.h index f3eaf1d048..ba9f63813e 100644 --- a/commit.h +++ b/commit.h @@ -40,8 +40,6 @@ int parse_commit_buffer(struct commit *item, void *buffer, unsigned long size); int parse_commit(struct commit *item); -void unparse_commit(struct commit *item); - struct commit_list * commit_list_insert(struct commit *item, struct commit_list **list_p); unsigned commit_list_count(const struct commit_list *l); struct commit_list * insert_by_date(struct commit *item, struct commit_list **list); From e22278c0a0784d4285f0e3173794caad4e542658 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Thu, 28 May 2009 23:21:16 +0200 Subject: [PATCH 02/35] bisect: display first bad commit without forking a new process Previously "git diff-tree --pretty COMMIT" was run using "run_command_v_opt" to display information about the first bad commit. The goal of this patch is to avoid a "fork" and an "exec" call when displaying that information. To do that, we manually setup revision information as "git diff-tree --pretty" would do it, and then use the "log_tree_commit" function. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 30 +++++++++++++++++++++++++++--- 1 file changed, 27 insertions(+), 3 deletions(-) diff --git a/bisect.c b/bisect.c index 18f9fa4062..2c14f4d616 100644 --- a/bisect.c +++ b/bisect.c @@ -7,6 +7,7 @@ #include "quote.h" #include "sha1-lookup.h" #include "run-command.h" +#include "log-tree.h" #include "bisect.h" struct sha1_array { @@ -27,7 +28,6 @@ struct argv_array { int argv_alloc; }; -static const char *argv_diff_tree[] = {"diff-tree", "--pretty", NULL, NULL}; static const char *argv_checkout[] = {"checkout", "-q", NULL, "--", NULL}; static const char *argv_show_branch[] = {"show-branch", NULL, NULL}; @@ -815,6 +815,31 @@ static void check_good_are_ancestors_of_bad(const char *prefix) close(fd); } +/* + * This does "git diff-tree --pretty COMMIT" without one fork+exec. + */ +static void show_diff_tree(const char *prefix, struct commit *commit) +{ + struct rev_info opt; + + /* diff-tree init */ + init_revisions(&opt, prefix); + git_config(git_diff_basic_config, NULL); /* no "diff" UI options */ + opt.abbrev = 0; + opt.diff = 1; + + /* This is what "--pretty" does */ + opt.verbose_header = 1; + opt.use_terminator = 0; + opt.commit_format = CMIT_FMT_DEFAULT; + + /* diff-tree init */ + if (!opt.diffopt.output_format) + opt.diffopt.output_format = DIFF_FORMAT_RAW; + + log_tree_commit(&opt, commit); +} + /* * We use the convention that exiting with an exit code 10 means that * the bisection process finished successfully. @@ -860,8 +885,7 @@ int bisect_next_all(const char *prefix) if (!hashcmp(bisect_rev, current_bad_sha1)) { exit_if_skipped_commits(tried, current_bad_sha1); printf("%s is first bad commit\n", bisect_rev_hex); - argv_diff_tree[2] = bisect_rev_hex; - run_command_v_opt(argv_diff_tree, RUN_GIT_CMD); + show_diff_tree(prefix, revs.commits->item); /* This means the bisection process succeeded. */ exit(10); } From 16493eb0d0da26f80286b39c7b6900e261744afa Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:26 +0800 Subject: [PATCH 03/35] http*: cleanup slot->local after fclose Set slot->local to NULL after doing a fclose() on the file it points to. This prevents the passing of a FILE* pointer to a fclose()'d file to ftell() in http.c::run_active_slot(). This issue was raised by Clemens Buchacher on 30th May 2009: http://www.spinics.net/lists/git/msg104623.html Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 6 ++++++ http-walker.c | 6 ++++++ 2 files changed, 12 insertions(+) diff --git a/http-push.c b/http-push.c index 5138224cc3..673a61110e 100644 --- a/http-push.c +++ b/http-push.c @@ -724,9 +724,11 @@ static void finish_request(struct transfer_request *request) struct stat st; struct packed_git *target; struct packed_git **lst; + struct active_request_slot *slot; request->curl_result = request->slot->curl_result; request->http_code = request->slot->http_code; + slot = request->slot; request->slot = NULL; /* Keep locks active */ @@ -823,6 +825,7 @@ static void finish_request(struct transfer_request *request) fclose(request->local_stream); request->local_stream = NULL; + slot->local = NULL; if (!move_temp_to_file(request->tmpfile, request->filename)) { target = (struct packed_git *)request->userData; @@ -1024,17 +1027,20 @@ static int fetch_index(unsigned char *sha1) if (results.curl_result != CURLE_OK) { free(url); fclose(indexfile); + slot->local = NULL; return error("Unable to get pack index %s\n%s", url, curl_errorstr); } } else { free(url); fclose(indexfile); + slot->local = NULL; return error("Unable to start request"); } free(url); fclose(indexfile); + slot->local = NULL; return move_temp_to_file(tmpfile, filename); } diff --git a/http-walker.c b/http-walker.c index c5a3ea3b31..ec1c97f2ee 100644 --- a/http-walker.c +++ b/http-walker.c @@ -418,15 +418,18 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch run_active_slot(slot); if (results.curl_result != CURLE_OK) { fclose(indexfile); + slot->local = NULL; return error("Unable to get pack index %s\n%s", url, curl_errorstr); } } else { fclose(indexfile); + slot->local = NULL; return error("Unable to start request"); } fclose(indexfile); + slot->local = NULL; return move_temp_to_file(tmpfile, filename); } @@ -776,16 +779,19 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha run_active_slot(slot); if (results.curl_result != CURLE_OK) { fclose(packfile); + slot->local = NULL; return error("Unable to get pack file %s\n%s", url, curl_errorstr); } } else { fclose(packfile); + slot->local = NULL; return error("Unable to start request"); } target->pack_size = ftell(packfile); fclose(packfile); + slot->local = NULL; ret = move_temp_to_file(tmpfile, filename); if (ret) From 242a90778bb8740ffa7957b462a2c6a5f0b0278e Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:23 +0800 Subject: [PATCH 04/35] t5540-http-push: test fetching of loose objects Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- t/t5540-http-push.sh | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh index 5fe479e1c2..65a41dbe5e 100755 --- a/t/t5540-http-push.sh +++ b/t/t5540-http-push.sh @@ -67,6 +67,22 @@ test_expect_success ' push to remote repository with unpacked refs' ' test $HEAD = $(git rev-parse --verify HEAD)) ' +test_expect_failure 'http-push fetches unpacked objects' ' + cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \ + "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_unpacked.git && + + git clone $HTTPD_URL/test_repo_unpacked.git \ + "$ROOT_PATH"/fetch_unpacked && + + # By reset, we force git to retrieve the object + (cd "$ROOT_PATH"/fetch_unpacked && + git reset --hard HEAD^ && + git remote rm origin && + git reflog expire --expire=0 --all && + git prune && + git push -f -v $HTTPD_URL/test_repo_unpacked.git master) +' + test_expect_success 'create and delete remote branch' ' cd "$ROOT_PATH"/test_repo_clone && git checkout -b dev && From 86d99f6d5ce1120b23a2453168165ac45e039411 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:24 +0800 Subject: [PATCH 05/35] t5540-http-push: test fetching of packed objects Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- t/t5540-http-push.sh | 20 ++++++++++++++++++++ 1 file changed, 20 insertions(+) diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh index 65a41dbe5e..ad0f14b93c 100755 --- a/t/t5540-http-push.sh +++ b/t/t5540-http-push.sh @@ -83,6 +83,26 @@ test_expect_failure 'http-push fetches unpacked objects' ' git push -f -v $HTTPD_URL/test_repo_unpacked.git master) ' +test_expect_failure 'http-push fetches packed objects' ' + cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \ + "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git && + + git clone $HTTPD_URL/test_repo_packed.git \ + "$ROOT_PATH"/test_repo_clone_packed && + + (cd "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git && + git --bare repack && + git --bare prune-packed) && + + # By reset, we force git to retrieve the packed object + (cd "$ROOT_PATH"/test_repo_clone_packed && + git reset --hard HEAD^ && + git remote rm origin && + git reflog expire --expire=0 --all && + git prune && + git push -f -v $HTTPD_URL/test_repo_packed.git master) +' + test_expect_success 'create and delete remote branch' ' cd "$ROOT_PATH"/test_repo_clone && git checkout -b dev && From 4f66250df641362f381faae2aec439850a5a6e41 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:27 +0800 Subject: [PATCH 06/35] http-push: send out fetch requests on queue Previously, requests for remote files were simply added to the queue (pointed to by request_queue_head) and no transfer actually takes place (the fill function add_fill_function() is not added until line 2441), even though code that followed may rely on these remote files to be present (eg. the setup_revisions invocation). The code that sends out the requests on the request queue is refactored into the method run_request_queue. After the get_dav_remote_heads invocation (ie. after fetch requests are added to the queue), the requests on the queue are sent out through an invocation to run_request_queue. This invocation to run_request_queue entails adding a fill function before pushing checks take place, which may lead to accidental, unwanted pushes previously. The flag is_running_queue is introduced to prevent this from occurring. fill_active_slot is made to check the flag is_running_queue before the sending of the requests proceeds. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 37 ++++++++++++++++++++++++++----------- t/t5540-http-push.sh | 4 ++-- 2 files changed, 28 insertions(+), 13 deletions(-) diff --git a/http-push.c b/http-push.c index a4ff7be217..b3d5c4512a 100644 --- a/http-push.c +++ b/http-push.c @@ -846,11 +846,12 @@ static void finish_request(struct transfer_request *request) } #ifdef USE_CURL_MULTI +static int is_running_queue; static int fill_active_slot(void *unused) { struct transfer_request *request; - if (aborted) + if (aborted || !is_running_queue) return 0; for (request = request_queue_head; request; request = request->next) { @@ -2173,6 +2174,25 @@ static int delete_remote_branch(char *pattern, int force) return 0; } +void run_request_queue(void) +{ +#ifdef USE_CURL_MULTI + is_running_queue = 1; + fill_active_slots(); + add_fill_function(NULL, fill_active_slot); +#endif + do { + finish_all_active_slots(); +#ifdef USE_CURL_MULTI + fill_active_slots(); +#endif + } while (request_queue_head && !aborted); + +#ifdef USE_CURL_MULTI + is_running_queue = 0; +#endif +} + int main(int argc, char **argv) { struct transfer_request *request; @@ -2277,6 +2297,8 @@ int main(int argc, char **argv) repo->url = rewritten_url; } + is_running_queue = 0; + /* Verify DAV compliance/lock support */ if (!locking_available()) { rc = 1; @@ -2306,6 +2328,7 @@ int main(int argc, char **argv) local_refs = get_local_heads(); fprintf(stderr, "Fetching remote heads...\n"); get_dav_remote_heads(); + run_request_queue(); /* Remove a remote branch if -d or -D was specified */ if (delete_branch) { @@ -2435,16 +2458,8 @@ int main(int argc, char **argv) if (objects_to_send) fprintf(stderr, " sending %d objects\n", objects_to_send); -#ifdef USE_CURL_MULTI - fill_active_slots(); - add_fill_function(NULL, fill_active_slot); -#endif - do { - finish_all_active_slots(); -#ifdef USE_CURL_MULTI - fill_active_slots(); -#endif - } while (request_queue_head && !aborted); + + run_request_queue(); /* Update the remote branch if all went well */ if (aborted || !update_remote(ref->new_sha1, ref_lock)) diff --git a/t/t5540-http-push.sh b/t/t5540-http-push.sh index ad0f14b93c..f4a2cf6c17 100755 --- a/t/t5540-http-push.sh +++ b/t/t5540-http-push.sh @@ -67,7 +67,7 @@ test_expect_success ' push to remote repository with unpacked refs' ' test $HEAD = $(git rev-parse --verify HEAD)) ' -test_expect_failure 'http-push fetches unpacked objects' ' +test_expect_success 'http-push fetches unpacked objects' ' cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \ "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_unpacked.git && @@ -83,7 +83,7 @@ test_expect_failure 'http-push fetches unpacked objects' ' git push -f -v $HTTPD_URL/test_repo_unpacked.git master) ' -test_expect_failure 'http-push fetches packed objects' ' +test_expect_success 'http-push fetches packed objects' ' cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo.git \ "$HTTPD_DOCUMENT_ROOT_PATH"/test_repo_packed.git && From 68862a3152691424a146a98ce52308e09faf5bb9 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:31 +0800 Subject: [PATCH 07/35] http-push: fix missing "#ifdef USE_CURL_MULTI" around "is_running_queue" As it is breaking the build when USE_CURL_MULTI is not defined. Signed-off-by: Christian Couder Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/http-push.c b/http-push.c index b3d5c4512a..7f36a40870 100644 --- a/http-push.c +++ b/http-push.c @@ -2297,7 +2297,9 @@ int main(int argc, char **argv) repo->url = rewritten_url; } +#ifdef USE_CURL_MULTI is_running_queue = 0; +#endif /* Verify DAV compliance/lock support */ if (!locking_available()) { From 96a4f187353d176d8fa32242875ffdad2ba38a0a Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:32 +0800 Subject: [PATCH 08/35] t5550-http-fetch: test fetching of packed objects Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- t/t5550-http-fetch.sh | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/t/t5550-http-fetch.sh b/t/t5550-http-fetch.sh index 05b1b62cb6..0e69324652 100755 --- a/t/t5550-http-fetch.sh +++ b/t/t5550-http-fetch.sh @@ -53,5 +53,13 @@ test_expect_success 'http remote detects correct HEAD' ' ) ' +test_expect_success 'fetch packed objects' ' + cp -R "$HTTPD_DOCUMENT_ROOT_PATH"/repo.git "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + cd "$HTTPD_DOCUMENT_ROOT_PATH"/repo_pack.git && + git --bare repack && + git --bare prune-packed && + git clone $HTTPD_URL/repo_pack.git +' + stop_httpd test_done From 4c42aa1a1359b2571b51c3a3093f29c7b25c54c4 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:33 +0800 Subject: [PATCH 09/35] http-push, http-walker: style fixes - Use tabs to indent, instead of spaces. - Do not use curly-braces around a single statement body in if/while statement; - Do not start multi-line comment with description on the first line after "/*", i.e. /* * We prefer this over... */ /* comments like * this (notice the first line) */ Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 48 +++++++++++++++++------------ http-walker.c | 83 +++++++++++++++++++++++++++++++-------------------- 2 files changed, 79 insertions(+), 52 deletions(-) diff --git a/http-push.c b/http-push.c index 7f36a40870..64c9bb0f90 100644 --- a/http-push.c +++ b/http-push.c @@ -276,7 +276,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, struct transfer_request *request = (struct transfer_request *)data; do { ssize_t retval = xwrite(request->local_fileno, - (char *) ptr + posn, size - posn); + (char *) ptr + posn, size - posn); if (retval < 0) return posn; posn += retval; @@ -289,7 +289,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, request->stream.avail_out = sizeof(expn); request->zret = git_inflate(&request->stream, Z_SYNC_FLUSH); git_SHA1_Update(&request->c, expn, - sizeof(expn) - request->stream.avail_out); + sizeof(expn) - request->stream.avail_out); } while (request->stream.avail_in && request->zret == Z_OK); data_received++; return size; @@ -323,7 +323,8 @@ static void start_fetch_loose(struct transfer_request *request) error("fd leakage in start: %d", request->local_fileno); request->local_fileno = open(request->tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0666); - /* This could have failed due to the "lazy directory creation"; + /* + * This could have failed due to the "lazy directory creation"; * try to mkdir the last path component. */ if (request->local_fileno < 0 && errno == ENOENT) { @@ -353,8 +354,10 @@ static void start_fetch_loose(struct transfer_request *request) url = get_remote_object_url(repo->url, hex, 0); request->url = xstrdup(url); - /* If a previous temp file is present, process what was already - fetched. */ + /* + * If a previous temp file is present, process what was already + * fetched. + */ prevlocal = open(prevfile, O_RDONLY); if (prevlocal != -1) { do { @@ -363,19 +366,20 @@ static void start_fetch_loose(struct transfer_request *request) if (fwrite_sha1_file(prev_buf, 1, prev_read, - request) == prev_read) { + request) == prev_read) prev_posn += prev_read; - } else { + else prev_read = -1; - } } } while (prev_read > 0); close(prevlocal); } unlink_or_warn(prevfile); - /* Reset inflate/SHA1 if there was an error reading the previous temp - file; also rewind to the beginning of the local file. */ + /* + * Reset inflate/SHA1 if there was an error reading the previous temp + * file; also rewind to the beginning of the local file. + */ if (prev_read == -1) { memset(&request->stream, 0, sizeof(request->stream)); git_inflate_init(&request->stream); @@ -398,8 +402,10 @@ static void start_fetch_loose(struct transfer_request *request) curl_easy_setopt(slot->curl, CURLOPT_URL, url); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - /* If we have successfully processed data from a previous fetch - attempt, only fetch the data we don't already have. */ + /* + * If we have successfully processed data from a previous fetch + * attempt, only fetch the data we don't already have. + */ if (prev_posn>0) { if (push_verbosely) fprintf(stderr, @@ -514,8 +520,10 @@ static void start_fetch_packed(struct transfer_request *request) curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); slot->local = packfile; - /* If there is data present from a previous transfer attempt, - resume where it left off */ + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ prev_posn = ftell(packfile); if (prev_posn>0) { if (push_verbosely) @@ -780,7 +788,8 @@ static void finish_request(struct transfer_request *request) aborted = 1; } } else if (request->state == RUN_FETCH_LOOSE) { - close(request->local_fileno); request->local_fileno = -1; + close(request->local_fileno); + request->local_fileno = -1; if (request->curl_result != CURLE_OK && request->http_code != 416) { @@ -803,9 +812,8 @@ static void finish_request(struct transfer_request *request) move_temp_to_file( request->tmpfile, request->filename); - if (request->rename == 0) { + if (request->rename == 0) request->obj->flags |= (LOCAL | REMOTE); - } } } @@ -1010,8 +1018,10 @@ static int fetch_index(unsigned char *sha1) curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); slot->local = indexfile; - /* If there is data present from a previous transfer attempt, - resume where it left off */ + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ prev_posn = ftell(indexfile); if (prev_posn>0) { if (push_verbosely) diff --git a/http-walker.c b/http-walker.c index 9377851925..6ac17831db 100644 --- a/http-walker.c +++ b/http-walker.c @@ -71,7 +71,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, struct object_request *obj_req = (struct object_request *)data; do { ssize_t retval = xwrite(obj_req->local, - (char *) ptr + posn, size - posn); + (char *) ptr + posn, size - posn); if (retval < 0) return posn; posn += retval; @@ -84,7 +84,7 @@ static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, obj_req->stream.avail_out = sizeof(expn); obj_req->zret = git_inflate(&obj_req->stream, Z_SYNC_FLUSH); git_SHA1_Update(&obj_req->c, expn, - sizeof(expn) - obj_req->stream.avail_out); + sizeof(expn) - obj_req->stream.avail_out); } while (obj_req->stream.avail_in && obj_req->zret == Z_OK); data_received++; return size; @@ -119,7 +119,8 @@ static void start_object_request(struct walker *walker, error("fd leakage in start: %d", obj_req->local); obj_req->local = open(obj_req->tmpfile, O_WRONLY | O_CREAT | O_EXCL, 0666); - /* This could have failed due to the "lazy directory creation"; + /* + * This could have failed due to the "lazy directory creation"; * try to mkdir the last path component. */ if (obj_req->local < 0 && errno == ENOENT) { @@ -158,8 +159,10 @@ static void start_object_request(struct walker *walker, strcpy(posn, hex + 2); strcpy(obj_req->url, url); - /* If a previous temp file is present, process what was already - fetched. */ + /* + * If a previous temp file is present, process what was already + * fetched. + */ prevlocal = open(prevfile, O_RDONLY); if (prevlocal != -1) { do { @@ -168,19 +171,20 @@ static void start_object_request(struct walker *walker, if (fwrite_sha1_file(prev_buf, 1, prev_read, - obj_req) == prev_read) { + obj_req) == prev_read) prev_posn += prev_read; - } else { + else prev_read = -1; - } } } while (prev_read > 0); close(prevlocal); } unlink_or_warn(prevfile); - /* Reset inflate/SHA1 if there was an error reading the previous temp - file; also rewind to the beginning of the local file. */ + /* + * Reset inflate/SHA1 if there was an error reading the previous temp + * file; also rewind to the beginning of the local file. + */ if (prev_read == -1) { memset(&obj_req->stream, 0, sizeof(obj_req->stream)); git_inflate_init(&obj_req->stream); @@ -203,8 +207,10 @@ static void start_object_request(struct walker *walker, curl_easy_setopt(slot->curl, CURLOPT_URL, url); curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); - /* If we have successfully processed data from a previous fetch - attempt, only fetch the data we don't already have. */ + /* + * If we have successfully processed data from a previous fetch + * attempt, only fetch the data we don't already have. + */ if (prev_posn>0) { if (walker->get_verbosely) fprintf(stderr, @@ -221,7 +227,8 @@ static void start_object_request(struct walker *walker, if (!start_active_slot(slot)) { obj_req->state = ABORTED; obj_req->slot = NULL; - close(obj_req->local); obj_req->local = -1; + close(obj_req->local); + obj_req->local = -1; free(obj_req->url); return; } @@ -231,7 +238,8 @@ static void finish_object_request(struct object_request *obj_req) { struct stat st; - close(obj_req->local); obj_req->local = -1; + close(obj_req->local); + obj_req->local = -1; if (obj_req->http_code == 416) { fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n"); @@ -350,9 +358,8 @@ static void prefetch(struct walker *walker, unsigned char *sha1) object_queue_head = newreq; } else { tail = object_queue_head; - while (tail->next != NULL) { + while (tail->next != NULL) tail = tail->next; - } tail->next = newreq; } @@ -401,8 +408,10 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); slot->local = indexfile; - /* If there is data present from a previous transfer attempt, - resume where it left off */ + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ prev_posn = ftell(indexfile); if (prev_posn>0) { if (walker->get_verbosely) @@ -507,7 +516,8 @@ static void process_alternates_response(void *callback_data) struct alt_base *newalt; char *target = NULL; if (data[i] == '/') { - /* This counts + /* + * This counts * http://git.host/pub/scm/linux.git/ * -----------here^ * so memcpy(dst, base, serverlen) will @@ -520,7 +530,8 @@ static void process_alternates_response(void *callback_data) okay = 1; } } else if (!memcmp(data + i, "../", 3)) { - /* Relative URL; chop the corresponding + /* + * Relative URL; chop the corresponding * number of subpath from base (and ../ * from data), and concatenate the result. * @@ -549,7 +560,7 @@ static void process_alternates_response(void *callback_data) } /* If the server got removed, give up. */ okay = strchr(base, ':') - base + 3 < - serverlen; + serverlen; } else if (alt_req->http_specific) { char *colon = strchr(data + i, ':'); char *slash = strchr(data + i, '/'); @@ -593,9 +604,11 @@ static void fetch_alternates(struct walker *walker, const char *base) struct alternates_request alt_req; struct walker_data *cdata = walker->data; - /* If another request has already started fetching alternates, - wait for them to arrive and return to processing this request's - curl message */ + /* + * If another request has already started fetching alternates, + * wait for them to arrive and return to processing this request's + * curl message + */ #ifdef USE_CURL_MULTI while (cdata->got_alternates == 0) { step_active_slots(); @@ -615,8 +628,10 @@ static void fetch_alternates(struct walker *walker, const char *base) url = xmalloc(strlen(base) + 31); sprintf(url, "%s/objects/info/http-alternates", base); - /* Use a callback to process the result, since another request - may fail and need to have alternates loaded before continuing */ + /* + * Use a callback to process the result, since another request + * may fail and need to have alternates loaded before continuing + */ slot = get_active_slot(); slot->callback_func = process_alternates_response; alt_req.walker = walker; @@ -762,8 +777,10 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); slot->local = packfile; - /* If there is data present from a previous transfer attempt, - resume where it left off */ + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ prev_posn = ftell(packfile); if (prev_posn>0) { if (walker->get_verbosely) @@ -840,18 +857,18 @@ static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned c } #ifdef USE_CURL_MULTI - while (obj_req->state == WAITING) { + while (obj_req->state == WAITING) step_active_slots(); - } #else start_object_request(walker, obj_req); #endif - while (obj_req->state == ACTIVE) { + while (obj_req->state == ACTIVE) run_active_slot(obj_req->slot); - } + if (obj_req->local != -1) { - close(obj_req->local); obj_req->local = -1; + close(obj_req->local); + obj_req->local = -1; } if (obj_req->state == ABORTED) { From 48188c259a7e6c87c20121933287b8c8ca721e3a Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:34 +0800 Subject: [PATCH 10/35] http-walker: verify remote packs In c17fb6e ("Verify remote packs, speed up pending request queue"), changes were made to index fetching in http-push.c, particularly the methods fetch_index and setup_index. Since http-walker.c has similar code for index fetching, these improvements should apply to http-walker.c's fetch_index and setup_index. Invocations of free() of string memory are reproduced as well. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-walker.c | 33 ++++++++++++++++++++++++++++++--- 1 file changed, 30 insertions(+), 3 deletions(-) diff --git a/http-walker.c b/http-walker.c index 6ac17831db..bbc3023e40 100644 --- a/http-walker.c +++ b/http-walker.c @@ -384,24 +384,48 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch struct active_request_slot *slot; struct slot_results results; - if (has_pack_index(sha1)) + /* Don't use the index if the pack isn't there */ + url = xmalloc(strlen(repo->base) + 64); + sprintf(url, "%s/objects/pack/pack-%s.pack", repo->base, hex); + slot = get_active_slot(); + slot->results = &results; + curl_easy_setopt(slot->curl, CURLOPT_URL, url); + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); + if (start_active_slot(slot)) { + run_active_slot(slot); + if (results.curl_result != CURLE_OK) { + free(url); + return error("Unable to verify pack %s is available", + hex); + } + } else { + free(url); + return error("Unable to start request"); + } + + if (has_pack_index(sha1)) { + free(url); return 0; + } if (walker->get_verbosely) fprintf(stderr, "Getting index for pack %s\n", hex); - url = xmalloc(strlen(repo->base) + 64); sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex); filename = sha1_pack_index_name(sha1); snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); indexfile = fopen(tmpfile, "a"); - if (!indexfile) + if (!indexfile) { + free(url); return error("Unable to open local file %s for pack index", tmpfile); + } slot = get_active_slot(); slot->results = &results; + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); + curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); curl_easy_setopt(slot->curl, CURLOPT_URL, url); @@ -426,17 +450,20 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch if (start_active_slot(slot)) { run_active_slot(slot); if (results.curl_result != CURLE_OK) { + free(url); fclose(indexfile); slot->local = NULL; return error("Unable to get pack index %s\n%s", url, curl_errorstr); } } else { + free(url); fclose(indexfile); slot->local = NULL; return error("Unable to start request"); } + free(url); fclose(indexfile); slot->local = NULL; From 20cfb3aa710d302829a776d7fbad2b89f71f15b6 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:36 +0800 Subject: [PATCH 11/35] http*: copy string returned by sha1_to_hex In the fetch_index implementations in http-push.c and http-walker.c, the string returned by sha1_to_hex is assumed to stay immutable. This patch ensures that hex stays immutable by copying the string returned by sha1_to_hex (via xstrdup) and frees it subsequently. It also refactors free()'s and fclose()'s with labels. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 47 ++++++++++++++++++++++++----------------------- http-walker.c | 45 +++++++++++++++++++++++---------------------- 2 files changed, 47 insertions(+), 45 deletions(-) diff --git a/http-push.c b/http-push.c index 64c9bb0f90..82018009f3 100644 --- a/http-push.c +++ b/http-push.c @@ -958,7 +958,8 @@ static int add_send_request(struct object *obj, struct remote_lock *lock) static int fetch_index(unsigned char *sha1) { - char *hex = sha1_to_hex(sha1); + int ret = 0; + char *hex = xstrdup(sha1_to_hex(sha1)); char *filename; char *url; char tmpfile[PATH_MAX]; @@ -980,18 +981,18 @@ static int fetch_index(unsigned char *sha1) if (start_active_slot(slot)) { run_active_slot(slot); if (results.curl_result != CURLE_OK) { - free(url); - return error("Unable to verify pack %s is available", - hex); + ret = error("Unable to verify pack %s is available", + hex); + goto cleanup_pack; } } else { - free(url); - return error("Unable to start request"); + ret = error("Unable to start request"); + goto cleanup_pack; } if (has_pack_index(sha1)) { - free(url); - return 0; + ret = 0; + goto cleanup_pack; } if (push_verbosely) @@ -1003,9 +1004,9 @@ static int fetch_index(unsigned char *sha1) snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); indexfile = fopen(tmpfile, "a"); if (!indexfile) { - free(url); - return error("Unable to open local file %s for pack index", - tmpfile); + ret = error("Unable to open local file %s for pack index", + tmpfile); + goto cleanup_pack; } slot = get_active_slot(); @@ -1036,24 +1037,24 @@ static int fetch_index(unsigned char *sha1) if (start_active_slot(slot)) { run_active_slot(slot); if (results.curl_result != CURLE_OK) { - free(url); - fclose(indexfile); - slot->local = NULL; - return error("Unable to get pack index %s\n%s", url, - curl_errorstr); + ret = error("Unable to get pack index %s\n%s", url, + curl_errorstr); + goto cleanup_index; } } else { - free(url); - fclose(indexfile); - slot->local = NULL; - return error("Unable to start request"); + ret = error("Unable to start request"); + goto cleanup_index; } - free(url); + ret = move_temp_to_file(tmpfile, filename); + +cleanup_index: fclose(indexfile); slot->local = NULL; - - return move_temp_to_file(tmpfile, filename); +cleanup_pack: + free(url); + free(hex); + return ret; } static int setup_index(unsigned char *sha1) diff --git a/http-walker.c b/http-walker.c index bbc3023e40..d6cc622e96 100644 --- a/http-walker.c +++ b/http-walker.c @@ -371,7 +371,8 @@ static void prefetch(struct walker *walker, unsigned char *sha1) static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1) { - char *hex = sha1_to_hex(sha1); + int ret = 0; + char *hex = xstrdup(sha1_to_hex(sha1)); char *filename; char *url; char tmpfile[PATH_MAX]; @@ -394,18 +395,18 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch if (start_active_slot(slot)) { run_active_slot(slot); if (results.curl_result != CURLE_OK) { - free(url); - return error("Unable to verify pack %s is available", + ret = error("Unable to verify pack %s is available", hex); + goto cleanup_pack; } } else { - free(url); - return error("Unable to start request"); + ret = error("Unable to start request"); + goto cleanup_pack; } if (has_pack_index(sha1)) { - free(url); - return 0; + ret = 0; + goto cleanup_pack; } if (walker->get_verbosely) @@ -417,9 +418,9 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); indexfile = fopen(tmpfile, "a"); if (!indexfile) { - free(url); - return error("Unable to open local file %s for pack index", - tmpfile); + ret = error("Unable to open local file %s for pack index", + tmpfile); + goto cleanup_pack; } slot = get_active_slot(); @@ -450,24 +451,24 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch if (start_active_slot(slot)) { run_active_slot(slot); if (results.curl_result != CURLE_OK) { - free(url); - fclose(indexfile); - slot->local = NULL; - return error("Unable to get pack index %s\n%s", url, - curl_errorstr); + ret = error("Unable to get pack index %s\n%s", url, + curl_errorstr); + goto cleanup_index; } } else { - free(url); - fclose(indexfile); - slot->local = NULL; - return error("Unable to start request"); + ret = error("Unable to start request"); + goto cleanup_index; } - free(url); + ret = move_temp_to_file(tmpfile, filename); + +cleanup_index: fclose(indexfile); slot->local = NULL; - - return move_temp_to_file(tmpfile, filename); +cleanup_pack: + free(url); + free(hex); + return ret; } static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1) From 1b1b7b235b040f27952d48ab8811c958a1f6d052 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:37 +0800 Subject: [PATCH 12/35] http-push: do not SEGV after fetching a bad pack idx file In a70c232 ("http-fetch: do not SEGV after fetching a bad pack idx file"), changes were made to the setup_index method in http-fetch.c (known in its present form as http-walker.c after 30ae764 ("Modularize commit-walker")). Since http-push.c has similar similar code for processing index files, these changes should apply to http-push.c's implementation of setup_index as well. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 2 ++ 1 file changed, 2 insertions(+) diff --git a/http-push.c b/http-push.c index 82018009f3..281e153eb3 100644 --- a/http-push.c +++ b/http-push.c @@ -1065,6 +1065,8 @@ static int setup_index(unsigned char *sha1) return -1; new_pack = parse_pack_index(sha1); + if (!new_pack) + return -1; /* parse_pack_index() already issued error message */ new_pack->next = repo->packs; repo->packs = new_pack; return 0; From 4277c6709d1976ee5cef33b7e415ae0dded87090 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sun, 18 Jan 2009 09:04:26 +0100 Subject: [PATCH 13/35] Don't expect verify_pack() callers to set pack_size Since use_pack() will end up populating pack_size if it is not already set, we can just adapt the code in verify_packfile() such that it doesn't require pack_size to be set beforehand. This allows callers not to have to set pack_size themselves, and we can thus revert changes from 1c23d794 (Don't die in git-http-fetch when fetching packs). Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- pack-check.c | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/pack-check.c b/pack-check.c index 90c33b1b84..166ca703c1 100644 --- a/pack-check.c +++ b/pack-check.c @@ -49,7 +49,7 @@ static int verify_packfile(struct packed_git *p, const unsigned char *index_base = p->index_data; git_SHA_CTX ctx; unsigned char sha1[20], *pack_sig; - off_t offset = 0, pack_sig_ofs = p->pack_size - 20; + off_t offset = 0, pack_sig_ofs = 0; uint32_t nr_objects, i; int err = 0; struct idx_entry *entries; @@ -61,14 +61,16 @@ static int verify_packfile(struct packed_git *p, */ git_SHA1_Init(&ctx); - while (offset < pack_sig_ofs) { + do { unsigned int remaining; unsigned char *in = use_pack(p, w_curs, offset, &remaining); offset += remaining; + if (!pack_sig_ofs) + pack_sig_ofs = p->pack_size - 20; if (offset > pack_sig_ofs) remaining -= (unsigned int)(offset - pack_sig_ofs); git_SHA1_Update(&ctx, in, remaining); - } + } while (offset < pack_sig_ofs); git_SHA1_Final(sha1, &ctx); pack_sig = use_pack(p, w_curs, pack_sig_ofs, NULL); if (hashcmp(sha1, pack_sig)) From df005219dd4a4c7c310c1e1ab5946f9ba69cc82d Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:40 +0800 Subject: [PATCH 14/35] transport.c::get_refs_via_curl(): do not leak refs_url Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- transport.c | 1 + 1 file changed, 1 insertion(+) diff --git a/transport.c b/transport.c index 89d846e5c3..b7c1c391c6 100644 --- a/transport.c +++ b/transport.c @@ -519,6 +519,7 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push) free(ref); } + free(refs_url); return refs; } From e917674597cc0b345ad2d6e29fd1a03e1039615a Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:41 +0800 Subject: [PATCH 15/35] http*: move common variables and macros to http.[ch] Move RANGE_HEADER_SIZE to http.h. Create no_pragma_header, the curl header list containing the header "Pragma:" in http.[ch]. It is allocated in http_init, and freed in http_cleanup. This replaces the no_pragma_header in http-push.c, and the no_pragma_header member in walker_data in http-walker.c. Create http_is_verbose. It is to be used by methods in http.c, and is modified at the entry points of http.c's users, namely http-push.c (when parsing options) and http-walker.c (in get_http_walker). Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 8 +------- http-walker.c | 18 +++++------------- http.c | 9 +++++++++ http.h | 5 +++++ 4 files changed, 20 insertions(+), 20 deletions(-) diff --git a/http-push.c b/http-push.c index 281e153eb3..f18da2a263 100644 --- a/http-push.c +++ b/http-push.c @@ -27,7 +27,6 @@ enum XML_Status { #endif #define PREV_BUF_SIZE 4096 -#define RANGE_HEADER_SIZE 30 /* DAV methods */ #define DAV_LOCK "LOCK" @@ -76,8 +75,6 @@ static int pushing; static int aborted; static signed char remote_dir_exists[256]; -static struct curl_slist *no_pragma_header; - static int push_verbosely; static int push_all = MATCH_REFS_NONE; static int force_all; @@ -2250,6 +2247,7 @@ int main(int argc, char **argv) } if (!strcmp(arg, "--verbose")) { push_verbosely = 1; + http_is_verbose = 1; continue; } if (!strcmp(arg, "-d")) { @@ -2299,8 +2297,6 @@ int main(int argc, char **argv) remote->url[remote->url_nr++] = repo->url; http_init(remote); - no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:"); - if (repo->url && repo->url[strlen(repo->url)-1] != '/') { rewritten_url = xmalloc(strlen(repo->url)+2); strcpy(rewritten_url, repo->url); @@ -2503,8 +2499,6 @@ int main(int argc, char **argv) unlock_remote(info_ref_lock); free(repo); - curl_slist_free_all(no_pragma_header); - http_cleanup(); request = request_queue_head; diff --git a/http-walker.c b/http-walker.c index d6cc622e96..032a7be62e 100644 --- a/http-walker.c +++ b/http-walker.c @@ -5,7 +5,6 @@ #include "http.h" #define PREV_BUF_SIZE 4096 -#define RANGE_HEADER_SIZE 30 struct alt_base { @@ -57,7 +56,6 @@ struct walker_data { const char *url; int got_alternates; struct alt_base *alt; - struct curl_slist *no_pragma_header; }; static struct object_request *object_queue_head; @@ -108,7 +106,6 @@ static void start_object_request(struct walker *walker, char range[RANGE_HEADER_SIZE]; struct curl_slist *range_header = NULL; struct active_request_slot *slot; - struct walker_data *data = walker->data; snprintf(prevfile, sizeof(prevfile), "%s.prev", obj_req->filename); unlink_or_warn(prevfile); @@ -205,7 +202,7 @@ static void start_object_request(struct walker *walker, curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, obj_req->errorstr); curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); /* * If we have successfully processed data from a previous fetch @@ -354,6 +351,8 @@ static void prefetch(struct walker *walker, unsigned char *sha1) newreq->slot = NULL; newreq->next = NULL; + http_is_verbose = walker->get_verbosely; + if (object_queue_head == NULL) { object_queue_head = newreq; } else { @@ -379,7 +378,6 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch long prev_posn = 0; char range[RANGE_HEADER_SIZE]; struct curl_slist *range_header = NULL; - struct walker_data *data = walker->data; FILE *indexfile; struct active_request_slot *slot; @@ -430,7 +428,7 @@ static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned ch curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); slot->local = indexfile; /* @@ -768,7 +766,6 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha long prev_posn = 0; char range[RANGE_HEADER_SIZE]; struct curl_slist *range_header = NULL; - struct walker_data *data = walker->data; struct active_request_slot *slot; struct slot_results results; @@ -802,7 +799,7 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha curl_easy_setopt(slot->curl, CURLOPT_FILE, packfile); curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, data->no_pragma_header); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); slot->local = packfile; /* @@ -948,10 +945,7 @@ static int fetch_ref(struct walker *walker, struct ref *ref) static void cleanup(struct walker *walker) { - struct walker_data *data = walker->data; http_cleanup(); - - curl_slist_free_all(data->no_pragma_header); } struct walker *get_http_walker(const char *url, struct remote *remote) @@ -962,8 +956,6 @@ struct walker *get_http_walker(const char *url, struct remote *remote) http_init(remote); - data->no_pragma_header = curl_slist_append(NULL, "Pragma:"); - data->alt = xmalloc(sizeof(*data->alt)); data->alt->base = xmalloc(strlen(url) + 1); strcpy(data->alt->base, url); diff --git a/http.c b/http.c index 2e3d6493ef..3ca60bb8f9 100644 --- a/http.c +++ b/http.c @@ -2,6 +2,7 @@ int data_received; int active_requests; +int http_is_verbose; #ifdef USE_CURL_MULTI static int max_requests = -1; @@ -29,6 +30,8 @@ static char *user_name, *user_pass; static struct curl_slist *pragma_header; +struct curl_slist *no_pragma_header; + static struct active_request_slot *active_queue_head; size_t fread_buffer(void *ptr, size_t eltsize, size_t nmemb, void *buffer_) @@ -276,6 +279,8 @@ void http_init(struct remote *remote) char *low_speed_limit; char *low_speed_time; + http_is_verbose = 0; + git_config(http_options, NULL); curl_global_init(CURL_GLOBAL_ALL); @@ -284,6 +289,7 @@ void http_init(struct remote *remote) curl_http_proxy = xstrdup(remote->http_proxy); pragma_header = curl_slist_append(pragma_header, "Pragma: no-cache"); + no_pragma_header = curl_slist_append(no_pragma_header, "Pragma:"); #ifdef USE_CURL_MULTI { @@ -366,6 +372,9 @@ void http_cleanup(void) curl_slist_free_all(pragma_header); pragma_header = NULL; + curl_slist_free_all(no_pragma_header); + no_pragma_header = NULL; + if (curl_http_proxy) { free((void *)curl_http_proxy); curl_http_proxy = NULL; diff --git a/http.h b/http.h index 26abebed1f..1ef7dc1583 100644 --- a/http.h +++ b/http.h @@ -88,11 +88,16 @@ extern void add_fill_function(void *data, int (*fill)(void *)); extern void step_active_slots(void); #endif +extern struct curl_slist *no_pragma_header; + +#define RANGE_HEADER_SIZE 30 + extern void http_init(struct remote *remote); extern void http_cleanup(void); extern int data_received; extern int active_requests; +extern int http_is_verbose; extern char curl_errorstr[CURL_ERROR_SIZE]; From 5ace994f3384726fa993de526fc059d1343a6463 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:43 +0800 Subject: [PATCH 16/35] http: create function end_url_with_slash The logic to append a slash to the url if necessary in quote_ref_url (added in 113106e "http.c: use strbuf API in quote_ref_url") has been moved to a new function, end_url_with_slash. The method takes a strbuf, the URL, and the path to be appended to the URL. It first adds the URL to the strbuf. It then appends a slash if the URL does not end with a slash. The check on ref in quote_ref_url for a slash at the beginning has been removed as a result of using end_url_with_slash. This check is not needed, because slashes will be quoted anyway. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http.c | 12 +++++++++--- 1 file changed, 9 insertions(+), 3 deletions(-) diff --git a/http.c b/http.c index 3ca60bb8f9..75fce9e94d 100644 --- a/http.c +++ b/http.c @@ -620,6 +620,7 @@ void finish_all_active_slots(void) } } +/* Helpers for modifying and creating URLs */ static inline int needs_quote(int ch) { if (((ch >= 'A') && (ch <= 'Z')) @@ -640,15 +641,20 @@ static inline int hex(int v) return 'A' + v - 10; } +static void end_url_with_slash(struct strbuf *buf, const char *url) +{ + strbuf_addstr(buf, url); + if (buf->len && buf->buf[buf->len - 1] != '/') + strbuf_addstr(buf, "/"); +} + static char *quote_ref_url(const char *base, const char *ref) { struct strbuf buf = STRBUF_INIT; const char *cp; int ch; - strbuf_addstr(&buf, base); - if (buf.len && buf.buf[buf.len - 1] != '/' && *ref != '/') - strbuf_addstr(&buf, "/"); + end_url_with_slash(&buf, base); for (cp = ref; (ch = *cp) != 0; cp++) if (needs_quote(ch)) From e929cd20bb3557833765f95fe0e9143a47f806e7 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:53 +0800 Subject: [PATCH 17/35] http.c: new functions for the http API The new functions added are: - http_request() (internal function) - http_get_strbuf() - http_get_file() - http_error() http_get_strbuf and http_get_file allow respectively to retrieve contents of an URL to a strbuf or an opened file handle. http_error prints out an error message containing the URL and the curl error (in curl_errorstr). Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http.c | 104 +++++++++++++++++++++++++++++++++++++++++++++++++++++++++ http.h | 30 +++++++++++++++++ 2 files changed, 134 insertions(+) diff --git a/http.c b/http.c index 75fce9e94d..df22180778 100644 --- a/http.c +++ b/http.c @@ -665,6 +665,110 @@ static char *quote_ref_url(const char *base, const char *ref) return strbuf_detach(&buf, NULL); } +/* http_request() targets */ +#define HTTP_REQUEST_STRBUF 0 +#define HTTP_REQUEST_FILE 1 + +static int http_request(const char *url, void *result, int target, int options) +{ + struct active_request_slot *slot; + struct slot_results results; + struct curl_slist *headers = NULL; + struct strbuf buf = STRBUF_INIT; + int ret; + + slot = get_active_slot(); + slot->results = &results; + curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); + + if (result == NULL) { + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); + } else { + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); + curl_easy_setopt(slot->curl, CURLOPT_FILE, result); + + if (target == HTTP_REQUEST_FILE) { + long posn = ftell(result); + curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, + fwrite); + if (posn > 0) { + strbuf_addf(&buf, "Range: bytes=%ld-", posn); + headers = curl_slist_append(headers, buf.buf); + strbuf_reset(&buf); + } + slot->local = result; + } else + curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, + fwrite_buffer); + } + + strbuf_addstr(&buf, "Pragma:"); + if (options & HTTP_NO_CACHE) + strbuf_addstr(&buf, " no-cache"); + + headers = curl_slist_append(headers, buf.buf); + + curl_easy_setopt(slot->curl, CURLOPT_URL, url); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, headers); + + if (start_active_slot(slot)) { + run_active_slot(slot); + if (results.curl_result == CURLE_OK) + ret = HTTP_OK; + else if (missing_target(&results)) + ret = HTTP_MISSING_TARGET; + else + ret = HTTP_ERROR; + } else { + error("Unable to start HTTP request for %s", url); + ret = HTTP_START_FAILED; + } + + slot->local = NULL; + curl_slist_free_all(headers); + strbuf_release(&buf); + + return ret; +} + +int http_get_strbuf(const char *url, struct strbuf *result, int options) +{ + return http_request(url, result, HTTP_REQUEST_STRBUF, options); +} + +int http_get_file(const char *url, const char *filename, int options) +{ + int ret; + struct strbuf tmpfile = STRBUF_INIT; + FILE *result; + + strbuf_addf(&tmpfile, "%s.temp", filename); + result = fopen(tmpfile.buf, "a"); + if (! result) { + error("Unable to open local file %s", tmpfile.buf); + ret = HTTP_ERROR; + goto cleanup; + } + + ret = http_request(url, result, HTTP_REQUEST_FILE, options); + fclose(result); + + if ((ret == HTTP_OK) && move_temp_to_file(tmpfile.buf, filename)) + ret = HTTP_ERROR; +cleanup: + strbuf_release(&tmpfile); + return ret; +} + +int http_error(const char *url, int ret) +{ + /* http_request has already handled HTTP_START_FAILED. */ + if (ret != HTTP_START_FAILED) + error("%s while accessing %s\n", curl_errorstr, url); + + return ret; +} + int http_fetch_ref(const char *base, struct ref *ref) { char *url; diff --git a/http.h b/http.h index 1ef7dc1583..3d878d5126 100644 --- a/http.h +++ b/http.h @@ -114,6 +114,36 @@ static inline int missing__target(int code, int result) #define missing_target(a) missing__target((a)->http_code, (a)->curl_result) +/* Options for http_request_*() */ +#define HTTP_NO_CACHE 1 + +/* Return values for http_request_*() */ +#define HTTP_OK 0 +#define HTTP_MISSING_TARGET 1 +#define HTTP_ERROR 2 +#define HTTP_START_FAILED 3 + +/* + * Requests an url and stores the result in a strbuf. + * + * If the result pointer is NULL, a HTTP HEAD request is made instead of GET. + */ +int http_get_strbuf(const char *url, struct strbuf *result, int options); + +/* + * Downloads an url and stores the result in the given file. + * + * If a previous interrupted download is detected (i.e. a previous temporary + * file is still around) the download is resumed. + */ +int http_get_file(const char *url, const char *filename, int options); + +/* + * Prints an error message using error() containing url and curl_errorstr, + * and returns ret. + */ +int http_error(const char *url, int ret); + extern int http_fetch_ref(const char *base, struct ref *ref); #endif /* HTTP_H */ From 28307b99ddb8e7284793d5ec830af10f130fa287 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:54 +0800 Subject: [PATCH 18/35] transport.c::get_refs_via_curl(): use the new http API Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- transport.c | 34 ++++++++++++---------------------- 1 file changed, 12 insertions(+), 22 deletions(-) diff --git a/transport.c b/transport.c index b7c1c391c6..2128c58726 100644 --- a/transport.c +++ b/transport.c @@ -439,9 +439,7 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push) char *ref_name; char *refs_url; int i = 0; - - struct active_request_slot *slot; - struct slot_results results; + int http_ret; struct ref *refs = NULL; struct ref *ref = NULL; @@ -461,25 +459,16 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push) refs_url = xmalloc(strlen(transport->url) + 11); sprintf(refs_url, "%s/info/refs", transport->url); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_URL, refs_url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL); - - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - strbuf_release(&buffer); - if (missing_target(&results)) - die("%s not found: did you run git update-server-info on the server?", refs_url); - else - die("%s download error - %s", refs_url, curl_errorstr); - } - } else { - strbuf_release(&buffer); - die("Unable to start HTTP request"); + http_ret = http_get_strbuf(refs_url, &buffer, HTTP_NO_CACHE); + switch (http_ret) { + case HTTP_OK: + break; + case HTTP_MISSING_TARGET: + die("%s not found: did you run git update-server-info on the" + " server?", refs_url); + default: + http_error(refs_url, http_ret); + die("HTTP request failed"); } data = buffer.buf; @@ -519,6 +508,7 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push) free(ref); } + strbuf_release(&buffer); free(refs_url); return refs; } From 0d5896e1cc1cb6145526b6786c7720cfe8231709 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:55 +0800 Subject: [PATCH 19/35] http.c::http_fetch_ref(): use the new http API The error message ("Unable to start request") has been removed, since the http API already prints it. Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http.c | 33 ++++++++------------------------- 1 file changed, 8 insertions(+), 25 deletions(-) diff --git a/http.c b/http.c index df22180778..2a49372012 100644 --- a/http.c +++ b/http.c @@ -773,34 +773,17 @@ int http_fetch_ref(const char *base, struct ref *ref) { char *url; struct strbuf buffer = STRBUF_INIT; - struct active_request_slot *slot; - struct slot_results results; - int ret; + int ret = -1; url = quote_ref_url(base, ref->name); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result == CURLE_OK) { - strbuf_rtrim(&buffer); - if (buffer.len == 40) - ret = get_sha1_hex(buffer.buf, ref->old_sha1); - else if (!prefixcmp(buffer.buf, "ref: ")) { - ref->symref = xstrdup(buffer.buf + 5); - ret = 0; - } else - ret = 1; - } else { - ret = error("Couldn't get %s for %s\n%s", - url, ref->name, curl_errorstr); + if (http_get_strbuf(url, &buffer, HTTP_NO_CACHE) == HTTP_OK) { + strbuf_rtrim(&buffer); + if (buffer.len == 40) + ret = get_sha1_hex(buffer.buf, ref->old_sha1); + else if (!prefixcmp(buffer.buf, "ref: ")) { + ref->symref = xstrdup(buffer.buf + 5); + ret = 0; } - } else { - ret = error("Unable to start request"); } strbuf_release(&buffer); From 446b941a57d96e0c02375534b6b3f6816e7364e5 Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:57 +0800 Subject: [PATCH 20/35] http-push.c::remote_exists(): use the new http API Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 31 ++++++++++++------------------- 1 file changed, 12 insertions(+), 19 deletions(-) diff --git a/http-push.c b/http-push.c index f18da2a263..c5642b5d4d 100644 --- a/http-push.c +++ b/http-push.c @@ -2004,29 +2004,22 @@ static void update_remote_info_refs(struct remote_lock *lock) static int remote_exists(const char *path) { char *url = xmalloc(strlen(repo->url) + strlen(path) + 1); - struct active_request_slot *slot; - struct slot_results results; - int ret = -1; + int ret; sprintf(url, "%s%s", repo->url, path); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); - - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.http_code == 404) - ret = 0; - else if (results.curl_result == CURLE_OK) - ret = 1; - else - fprintf(stderr, "HEAD HTTP error %ld\n", results.http_code); - } else { - fprintf(stderr, "Unable to start HEAD request\n"); + switch (http_get_strbuf(url, NULL, 0)) { + case HTTP_OK: + ret = 1; + break; + case HTTP_MISSING_TARGET: + ret = 0; + break; + case HTTP_ERROR: + http_error(url, HTTP_ERROR); + default: + ret = -1; } - free(url); return ret; } From 9af5abd9939585bb588d537085138563922c6abe Mon Sep 17 00:00:00 2001 From: Mike Hommey Date: Sat, 6 Jun 2009 16:43:58 +0800 Subject: [PATCH 21/35] http-push.c::fetch_symref(): use the new http API Signed-off-by: Mike Hommey Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 20 +++----------------- 1 file changed, 3 insertions(+), 17 deletions(-) diff --git a/http-push.c b/http-push.c index c5642b5d4d..3490521475 100644 --- a/http-push.c +++ b/http-push.c @@ -2028,27 +2028,13 @@ static void fetch_symref(const char *path, char **symref, unsigned char *sha1) { char *url; struct strbuf buffer = STRBUF_INIT; - struct active_request_slot *slot; - struct slot_results results; url = xmalloc(strlen(repo->url) + strlen(path) + 1); sprintf(url, "%s%s", repo->url, path); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - die("Couldn't get %s for remote symref\n%s", - url, curl_errorstr); - } - } else { - die("Unable to start remote symref request"); - } + if (http_get_strbuf(url, &buffer, 0) != HTTP_OK) + die("Couldn't get %s for remote symref\n%s", url, + curl_errorstr); free(url); free(*symref); From b8caac2b8ab6482e7ab59c8ec18f1c3d90e7387d Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:43:59 +0800 Subject: [PATCH 22/35] http*: add http_get_info_packs http-push.c and http-walker.c no longer have to use fetch_index or setup_index; they simply need to use http_get_info_packs, a new http method, in their fetch_indices implementations. Move fetch_index() and rename to fetch_pack_index() in http.c; this method is not meant to be used outside of http.c. It invokes end_url_with_slash with base_url; apart from that change, the code is identical. Move setup_index() and rename to fetch_and_setup_pack_index() in http.c; this method is not meant to be used outside of http.c. Do not immediately set ret to 0 in http-walker.c::fetch_indices(); instead do it in the HTTP_MISSING_TARGET case, to make it clear that the HTTP_OK and HTTP_MISSING_TARGET cases both return 0. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 179 +++--------------------------------------------- http-walker.c | 184 +++----------------------------------------------- http.c | 164 ++++++++++++++++++++++++++++++++++++++++++++ http.h | 4 ++ 4 files changed, 186 insertions(+), 345 deletions(-) diff --git a/http-push.c b/http-push.c index 3490521475..7ad3b690bb 100644 --- a/http-push.c +++ b/http-push.c @@ -953,184 +953,23 @@ static int add_send_request(struct object *obj, struct remote_lock *lock) return 1; } -static int fetch_index(unsigned char *sha1) -{ - int ret = 0; - char *hex = xstrdup(sha1_to_hex(sha1)); - char *filename; - char *url; - char tmpfile[PATH_MAX]; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; - - FILE *indexfile; - struct active_request_slot *slot; - struct slot_results results; - - /* Don't use the index if the pack isn't there */ - url = xmalloc(strlen(repo->url) + 64); - sprintf(url, "%sobjects/pack/pack-%s.pack", repo->url, hex); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to verify pack %s is available", - hex); - goto cleanup_pack; - } - } else { - ret = error("Unable to start request"); - goto cleanup_pack; - } - - if (has_pack_index(sha1)) { - ret = 0; - goto cleanup_pack; - } - - if (push_verbosely) - fprintf(stderr, "Getting index for pack %s\n", hex); - - sprintf(url, "%sobjects/pack/pack-%s.idx", repo->url, hex); - - filename = sha1_pack_index_name(sha1); - snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); - indexfile = fopen(tmpfile, "a"); - if (!indexfile) { - ret = error("Unable to open local file %s for pack index", - tmpfile); - goto cleanup_pack; - } - - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); - curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); - curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - slot->local = indexfile; - - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftell(indexfile); - if (prev_posn>0) { - if (push_verbosely) - fprintf(stderr, - "Resuming fetch of index for pack %s at byte %ld\n", - hex, prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); - } - - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to get pack index %s\n%s", url, - curl_errorstr); - goto cleanup_index; - } - } else { - ret = error("Unable to start request"); - goto cleanup_index; - } - - ret = move_temp_to_file(tmpfile, filename); - -cleanup_index: - fclose(indexfile); - slot->local = NULL; -cleanup_pack: - free(url); - free(hex); - return ret; -} - -static int setup_index(unsigned char *sha1) -{ - struct packed_git *new_pack; - - if (fetch_index(sha1)) - return -1; - - new_pack = parse_pack_index(sha1); - if (!new_pack) - return -1; /* parse_pack_index() already issued error message */ - new_pack->next = repo->packs; - repo->packs = new_pack; - return 0; -} - static int fetch_indices(void) { - unsigned char sha1[20]; - char *url; - struct strbuf buffer = STRBUF_INIT; - char *data; - int i = 0; - - struct active_request_slot *slot; - struct slot_results results; + int ret; if (push_verbosely) fprintf(stderr, "Getting pack list\n"); - url = xmalloc(strlen(repo->url) + 20); - sprintf(url, "%sobjects/info/packs", repo->url); - - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - strbuf_release(&buffer); - free(url); - if (results.http_code == 404) - return 0; - else - return error("%s", curl_errorstr); - } - } else { - strbuf_release(&buffer); - free(url); - return error("Unable to start request"); - } - free(url); - - data = buffer.buf; - while (i < buffer.len) { - switch (data[i]) { - case 'P': - i++; - if (i + 52 < buffer.len && - !prefixcmp(data + i, " pack-") && - !prefixcmp(data + i + 46, ".pack\n")) { - get_sha1_hex(data + i + 6, sha1); - setup_index(sha1); - i += 51; - break; - } - default: - while (data[i] != '\n') - i++; - } - i++; + switch (http_get_info_packs(repo->url, &repo->packs)) { + case HTTP_OK: + case HTTP_MISSING_TARGET: + ret = 0; + break; + default: + ret = -1; } - strbuf_release(&buffer); - return 0; + return ret; } static void one_remote_object(const char *hex) diff --git a/http-walker.c b/http-walker.c index 032a7be62e..ac343fd191 100644 --- a/http-walker.c +++ b/http-walker.c @@ -368,124 +368,6 @@ static void prefetch(struct walker *walker, unsigned char *sha1) #endif } -static int fetch_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1) -{ - int ret = 0; - char *hex = xstrdup(sha1_to_hex(sha1)); - char *filename; - char *url; - char tmpfile[PATH_MAX]; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; - - FILE *indexfile; - struct active_request_slot *slot; - struct slot_results results; - - /* Don't use the index if the pack isn't there */ - url = xmalloc(strlen(repo->base) + 64); - sprintf(url, "%s/objects/pack/pack-%s.pack", repo->base, hex); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to verify pack %s is available", - hex); - goto cleanup_pack; - } - } else { - ret = error("Unable to start request"); - goto cleanup_pack; - } - - if (has_pack_index(sha1)) { - ret = 0; - goto cleanup_pack; - } - - if (walker->get_verbosely) - fprintf(stderr, "Getting index for pack %s\n", hex); - - sprintf(url, "%s/objects/pack/pack-%s.idx", repo->base, hex); - - filename = sha1_pack_index_name(sha1); - snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); - indexfile = fopen(tmpfile, "a"); - if (!indexfile) { - ret = error("Unable to open local file %s for pack index", - tmpfile); - goto cleanup_pack; - } - - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); - curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); - curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - slot->local = indexfile; - - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftell(indexfile); - if (prev_posn>0) { - if (walker->get_verbosely) - fprintf(stderr, - "Resuming fetch of index for pack %s at byte %ld\n", - hex, prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); - } - - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to get pack index %s\n%s", url, - curl_errorstr); - goto cleanup_index; - } - } else { - ret = error("Unable to start request"); - goto cleanup_index; - } - - ret = move_temp_to_file(tmpfile, filename); - -cleanup_index: - fclose(indexfile); - slot->local = NULL; -cleanup_pack: - free(url); - free(hex); - return ret; -} - -static int setup_index(struct walker *walker, struct alt_base *repo, unsigned char *sha1) -{ - struct packed_git *new_pack; - if (has_pack_file(sha1)) - return 0; /* don't list this as something we can get */ - - if (fetch_index(walker, repo, sha1)) - return -1; - - new_pack = parse_pack_index(sha1); - if (!new_pack) - return -1; /* parse_pack_index() already issued error message */ - new_pack->next = repo->packs; - repo->packs = new_pack; - return 0; -} - static void process_alternates_response(void *callback_data) { struct alternates_request *alt_req = @@ -684,15 +566,7 @@ static void fetch_alternates(struct walker *walker, const char *base) static int fetch_indices(struct walker *walker, struct alt_base *repo) { - unsigned char sha1[20]; - char *url; - struct strbuf buffer = STRBUF_INIT; - char *data; - int i = 0; - int ret = 0; - - struct active_request_slot *slot; - struct slot_results results; + int ret; if (repo->got_indices) return 0; @@ -700,57 +574,17 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo) if (walker->get_verbosely) fprintf(stderr, "Getting pack list for %s\n", repo->base); - url = xmalloc(strlen(repo->base) + 21); - sprintf(url, "%s/objects/info/packs", repo->base); - - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, &buffer); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_buffer); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, NULL); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - if (missing_target(&results)) { - repo->got_indices = 1; - goto cleanup; - } else { - repo->got_indices = 0; - ret = error("%s", curl_errorstr); - goto cleanup; - } - } - } else { + switch (http_get_info_packs(repo->base, &repo->packs)) { + case HTTP_OK: + case HTTP_MISSING_TARGET: + repo->got_indices = 1; + ret = 0; + break; + default: repo->got_indices = 0; - ret = error("Unable to start request"); - goto cleanup; + ret = -1; } - data = buffer.buf; - while (i < buffer.len) { - switch (data[i]) { - case 'P': - i++; - if (i + 52 <= buffer.len && - !prefixcmp(data + i, " pack-") && - !prefixcmp(data + i + 46, ".pack\n")) { - get_sha1_hex(data + i + 6, sha1); - setup_index(walker, repo, sha1); - i += 51; - break; - } - default: - while (i < buffer.len && data[i] != '\n') - i++; - } - i++; - } - - repo->got_indices = 1; -cleanup: - strbuf_release(&buffer); - free(url); return ret; } diff --git a/http.c b/http.c index 2a49372012..96d83d5c3c 100644 --- a/http.c +++ b/http.c @@ -790,3 +790,167 @@ int http_fetch_ref(const char *base, struct ref *ref) free(url); return ret; } + +/* Helpers for fetching packs */ +static int fetch_pack_index(unsigned char *sha1, const char *base_url) +{ + int ret = 0; + char *hex = xstrdup(sha1_to_hex(sha1)); + char *filename; + char *url; + char tmpfile[PATH_MAX]; + long prev_posn = 0; + char range[RANGE_HEADER_SIZE]; + struct strbuf buf = STRBUF_INIT; + struct curl_slist *range_header = NULL; + + FILE *indexfile; + struct active_request_slot *slot; + struct slot_results results; + + /* Don't use the index if the pack isn't there */ + end_url_with_slash(&buf, base_url); + strbuf_addf(&buf, "objects/pack/pack-%s.pack", hex); + url = strbuf_detach(&buf, 0); + + slot = get_active_slot(); + slot->results = &results; + curl_easy_setopt(slot->curl, CURLOPT_URL, url); + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); + if (start_active_slot(slot)) { + run_active_slot(slot); + if (results.curl_result != CURLE_OK) { + ret = error("Unable to verify pack %s is available", + hex); + goto cleanup_pack; + } + } else { + ret = error("Unable to start request"); + goto cleanup_pack; + } + + if (has_pack_index(sha1)) { + ret = 0; + goto cleanup_pack; + } + + if (http_is_verbose) + fprintf(stderr, "Getting index for pack %s\n", hex); + + end_url_with_slash(&buf, base_url); + strbuf_addf(&buf, "objects/pack/pack-%s.idx", hex); + url = strbuf_detach(&buf, NULL); + + filename = sha1_pack_index_name(sha1); + snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); + indexfile = fopen(tmpfile, "a"); + if (!indexfile) { + ret = error("Unable to open local file %s for pack index", + tmpfile); + goto cleanup_pack; + } + + slot = get_active_slot(); + slot->results = &results; + curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); + curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); + curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); + curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); + curl_easy_setopt(slot->curl, CURLOPT_URL, url); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); + slot->local = indexfile; + + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ + prev_posn = ftell(indexfile); + if (prev_posn>0) { + if (http_is_verbose) + fprintf(stderr, + "Resuming fetch of index for pack %s at byte %ld\n", + hex, prev_posn); + sprintf(range, "Range: bytes=%ld-", prev_posn); + range_header = curl_slist_append(range_header, range); + curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); + } + + if (start_active_slot(slot)) { + run_active_slot(slot); + if (results.curl_result != CURLE_OK) { + ret = error("Unable to get pack index %s\n%s", + url, curl_errorstr); + goto cleanup_index; + } + } else { + ret = error("Unable to start request"); + goto cleanup_index; + } + + ret = move_temp_to_file(tmpfile, filename); + +cleanup_index: + fclose(indexfile); + slot->local = NULL; +cleanup_pack: + free(hex); + free(url); + return ret; +} + +static int fetch_and_setup_pack_index(struct packed_git **packs_head, + unsigned char *sha1, const char *base_url) +{ + struct packed_git *new_pack; + + if (fetch_pack_index(sha1, base_url)) + return -1; + + new_pack = parse_pack_index(sha1); + if (!new_pack) + return -1; /* parse_pack_index() already issued error message */ + new_pack->next = *packs_head; + *packs_head = new_pack; + return 0; +} + +int http_get_info_packs(const char *base_url, struct packed_git **packs_head) +{ + int ret = 0, i = 0; + char *url, *data; + struct strbuf buf = STRBUF_INIT; + unsigned char sha1[20]; + + end_url_with_slash(&buf, base_url); + strbuf_addstr(&buf, "objects/info/packs"); + url = strbuf_detach(&buf, NULL); + + ret = http_get_strbuf(url, &buf, HTTP_NO_CACHE); + if (ret != HTTP_OK) + goto cleanup; + + data = buf.buf; + while (i < buf.len) { + switch (data[i]) { + case 'P': + i++; + if (i + 52 <= buf.len && + !prefixcmp(data + i, " pack-") && + !prefixcmp(data + i + 46, ".pack\n")) { + get_sha1_hex(data + i + 6, sha1); + fetch_and_setup_pack_index(packs_head, sha1, + base_url); + i += 51; + break; + } + default: + while (i < buf.len && data[i] != '\n') + i++; + } + i++; + } + +cleanup: + free(url); + return ret; +} diff --git a/http.h b/http.h index 3d878d5126..180b1c2ae2 100644 --- a/http.h +++ b/http.h @@ -146,4 +146,8 @@ int http_error(const char *url, int ret); extern int http_fetch_ref(const char *base, struct ref *ref); +/* Helpers for fetching packs */ +extern int http_get_info_packs(const char *base_url, + struct packed_git **packs_head); + #endif /* HTTP_H */ From 39dc52cf4ff04e9cd4d2562218ad619e23a81efa Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:44:00 +0800 Subject: [PATCH 23/35] http: use new http API in fetch_index() Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http.c | 81 ++++++---------------------------------------------------- 1 file changed, 8 insertions(+), 73 deletions(-) diff --git a/http.c b/http.c index 96d83d5c3c..0701a6f6eb 100644 --- a/http.c +++ b/http.c @@ -798,40 +798,22 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url) char *hex = xstrdup(sha1_to_hex(sha1)); char *filename; char *url; - char tmpfile[PATH_MAX]; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; struct strbuf buf = STRBUF_INIT; - struct curl_slist *range_header = NULL; - - FILE *indexfile; - struct active_request_slot *slot; - struct slot_results results; /* Don't use the index if the pack isn't there */ end_url_with_slash(&buf, base_url); strbuf_addf(&buf, "objects/pack/pack-%s.pack", hex); url = strbuf_detach(&buf, 0); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 1); - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to verify pack %s is available", - hex); - goto cleanup_pack; - } - } else { - ret = error("Unable to start request"); - goto cleanup_pack; + if (http_get_strbuf(url, NULL, 0)) { + ret = error("Unable to verify pack %s is available", + hex); + goto cleanup; } if (has_pack_index(sha1)) { ret = 0; - goto cleanup_pack; + goto cleanup; } if (http_is_verbose) @@ -842,57 +824,10 @@ static int fetch_pack_index(unsigned char *sha1, const char *base_url) url = strbuf_detach(&buf, NULL); filename = sha1_pack_index_name(sha1); - snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); - indexfile = fopen(tmpfile, "a"); - if (!indexfile) { - ret = error("Unable to open local file %s for pack index", - tmpfile); - goto cleanup_pack; - } + if (http_get_file(url, filename, 0) != HTTP_OK) + ret = error("Unable to get pack index %s\n", url); - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_NOBODY, 0); - curl_easy_setopt(slot->curl, CURLOPT_HTTPGET, 1); - curl_easy_setopt(slot->curl, CURLOPT_FILE, indexfile); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - slot->local = indexfile; - - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftell(indexfile); - if (prev_posn>0) { - if (http_is_verbose) - fprintf(stderr, - "Resuming fetch of index for pack %s at byte %ld\n", - hex, prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); - } - - if (start_active_slot(slot)) { - run_active_slot(slot); - if (results.curl_result != CURLE_OK) { - ret = error("Unable to get pack index %s\n%s", - url, curl_errorstr); - goto cleanup_index; - } - } else { - ret = error("Unable to start request"); - goto cleanup_index; - } - - ret = move_temp_to_file(tmpfile, filename); - -cleanup_index: - fclose(indexfile); - slot->local = NULL; -cleanup_pack: +cleanup: free(hex); free(url); return ret; From 2264dfa5c4f11e2b0e2740072208186bee361afd Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:44:01 +0800 Subject: [PATCH 24/35] http*: add helper methods for fetching packs The code handling the fetching of packs in http-push.c and http-walker.c have been refactored into new methods and a new struct (http_pack_request) in http.c. They are not meant to be invoked elsewhere. The new methods in http.c are - new_http_pack_request - finish_http_pack_request - release_http_pack_request and the new struct is http_pack_request. Add a function, new_http_pack_request(), that deals with the details of coming up with the filename to store the retrieved packfile, resuming a previously aborted request, and making a new curl request. Update http-push.c::start_fetch_packed() and http-walker.c::fetch_pack() to use this. Add a function, finish_http_pack_request(), that deals with renaming the pack, advancing the pack list, and installing the pack. Update http-push.c::finish_request() and http-walker.c::fetch_pack to use this. Update release_request() in http-push.c and http-walker.c to invoke release_http_pack_request() to clean up pack request helper data. The local_stream member of the transfer_request struct in http-push.c has been removed, as the packfile pointer will be managed in the struct http_pack_request. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 108 ++++++++++++-------------------------------------- http-walker.c | 85 +++++++++------------------------------ http.c | 106 +++++++++++++++++++++++++++++++++++++++++++++++++ http.h | 17 ++++++++ 4 files changed, 166 insertions(+), 150 deletions(-) diff --git a/http-push.c b/http-push.c index 7ad3b690bb..8232cf488e 100644 --- a/http-push.c +++ b/http-push.c @@ -1,6 +1,5 @@ #include "cache.h" #include "commit.h" -#include "pack.h" #include "tag.h" #include "blob.h" #include "http.h" @@ -119,7 +118,6 @@ struct transfer_request char filename[PATH_MAX]; char tmpfile[PATH_MAX]; int local_fileno; - FILE *local_stream; enum transfer_state state; CURLcode curl_result; char errorstr[CURL_ERROR_SIZE]; @@ -452,16 +450,10 @@ static void start_mkcol(struct transfer_request *request) static void start_fetch_packed(struct transfer_request *request) { - char *url; struct packed_git *target; - FILE *packfile; - char *filename; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; struct transfer_request *check_request = request_queue_head; - struct active_request_slot *slot; + struct http_pack_request *preq; target = find_sha1_pack(request->obj->sha1, repo->packs); if (!target) { @@ -474,68 +466,35 @@ static void start_fetch_packed(struct transfer_request *request) fprintf(stderr, "Fetching pack %s\n", sha1_to_hex(target->sha1)); fprintf(stderr, " which contains %s\n", sha1_to_hex(request->obj->sha1)); - filename = sha1_pack_name(target->sha1); - snprintf(request->filename, sizeof(request->filename), "%s", filename); - snprintf(request->tmpfile, sizeof(request->tmpfile), - "%s.temp", filename); - - url = xmalloc(strlen(repo->url) + 64); - sprintf(url, "%sobjects/pack/pack-%s.pack", - repo->url, sha1_to_hex(target->sha1)); + preq = new_http_pack_request(target, repo->url); + if (preq == NULL) { + release_http_pack_request(preq); + 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) { if (check_request->state == RUN_FETCH_PACKED && - !strcmp(check_request->url, url)) { - free(url); + !strcmp(check_request->url, preq->url)) { + release_http_pack_request(preq); release_request(request); return; } check_request = check_request->next; } - packfile = fopen(request->tmpfile, "a"); - if (!packfile) { - fprintf(stderr, "Unable to open local file %s for pack", - request->tmpfile); - repo->can_update_info_refs = 0; - free(url); - return; - } - - slot = get_active_slot(); - slot->callback_func = process_response; - slot->callback_data = request; - request->slot = slot; - request->local_stream = packfile; - request->userData = target; - - request->url = url; - curl_easy_setopt(slot->curl, CURLOPT_FILE, packfile); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - slot->local = packfile; - - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftell(packfile); - if (prev_posn>0) { - if (push_verbosely) - fprintf(stderr, - "Resuming fetch of pack %s at byte %ld\n", - sha1_to_hex(target->sha1), prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); - } + preq->slot->callback_func = process_response; + preq->slot->callback_data = request; + request->slot = preq->slot; + request->userData = preq; /* Try to get the request started, abort the request on error */ request->state = RUN_FETCH_PACKED; - if (!start_active_slot(slot)) { + if (!start_active_slot(preq->slot)) { fprintf(stderr, "Unable to start GET request\n"); + release_http_pack_request(preq); repo->can_update_info_refs = 0; release_request(request); } @@ -718,8 +677,6 @@ static void release_request(struct transfer_request *request) if (request->local_fileno != -1) close(request->local_fileno); - if (request->local_stream) - fclose(request->local_stream); free(request->url); free(request); } @@ -727,13 +684,10 @@ static void release_request(struct transfer_request *request) static void finish_request(struct transfer_request *request) { struct stat st; - struct packed_git *target; - struct packed_git **lst; - struct active_request_slot *slot; + struct http_pack_request *preq; request->curl_result = request->slot->curl_result; request->http_code = request->slot->http_code; - slot = request->slot; request->slot = NULL; /* Keep locks active */ @@ -821,31 +775,21 @@ static void finish_request(struct transfer_request *request) start_fetch_packed(request); } else if (request->state == RUN_FETCH_PACKED) { + int fail = 1; if (request->curl_result != CURLE_OK) { fprintf(stderr, "Unable to get pack file %s\n%s", request->url, curl_errorstr); - repo->can_update_info_refs = 0; } else { - off_t pack_size = ftell(request->local_stream); + preq = (struct http_pack_request *)request->userData; - fclose(request->local_stream); - request->local_stream = NULL; - slot->local = NULL; - if (!move_temp_to_file(request->tmpfile, - request->filename)) { - target = (struct packed_git *)request->userData; - target->pack_size = pack_size; - lst = &repo->packs; - while (*lst != target) - lst = &((*lst)->next); - *lst = (*lst)->next; - - if (!verify_pack(target)) - install_packed_git(target); - else - repo->can_update_info_refs = 0; + if (preq) { + if (finish_http_pack_request(preq) > 0) + fail = 0; + release_http_pack_request(preq); } } + if (fail) + repo->can_update_info_refs = 0; release_request(request); } } @@ -900,7 +844,6 @@ static void add_fetch_request(struct object *obj) request->lock = NULL; request->headers = NULL; request->local_fileno = -1; - request->local_stream = NULL; request->state = NEED_FETCH; request->next = request_queue_head; request_queue_head = request; @@ -940,7 +883,6 @@ static int add_send_request(struct object *obj, struct remote_lock *lock) request->lock = lock; request->headers = NULL; request->local_fileno = -1; - request->local_stream = NULL; request->state = NEED_PUSH; request->next = request_queue_head; request_queue_head = request; diff --git a/http-walker.c b/http-walker.c index ac343fd191..8f7a975f96 100644 --- a/http-walker.c +++ b/http-walker.c @@ -1,6 +1,5 @@ #include "cache.h" #include "commit.h" -#include "pack.h" #include "walker.h" #include "http.h" @@ -590,19 +589,10 @@ static int fetch_indices(struct walker *walker, struct alt_base *repo) static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned char *sha1) { - char *url; struct packed_git *target; - struct packed_git **lst; - FILE *packfile; - char *filename; - char tmpfile[PATH_MAX]; int ret; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; - - struct active_request_slot *slot; struct slot_results results; + struct http_pack_request *preq; if (fetch_indices(walker, repo)) return -1; @@ -617,72 +607,33 @@ static int fetch_pack(struct walker *walker, struct alt_base *repo, unsigned cha sha1_to_hex(sha1)); } - url = xmalloc(strlen(repo->base) + 65); - sprintf(url, "%s/objects/pack/pack-%s.pack", - repo->base, sha1_to_hex(target->sha1)); + preq = new_http_pack_request(target, repo->base); + if (preq == NULL) + goto abort; + preq->lst = &repo->packs; + preq->slot->results = &results; - filename = sha1_pack_name(target->sha1); - snprintf(tmpfile, sizeof(tmpfile), "%s.temp", filename); - packfile = fopen(tmpfile, "a"); - if (!packfile) - return error("Unable to open local file %s for pack", - tmpfile); - - slot = get_active_slot(); - slot->results = &results; - curl_easy_setopt(slot->curl, CURLOPT_FILE, packfile); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - slot->local = packfile; - - /* - * If there is data present from a previous transfer attempt, - * resume where it left off - */ - prev_posn = ftell(packfile); - if (prev_posn>0) { - if (walker->get_verbosely) - fprintf(stderr, - "Resuming fetch of pack %s at byte %ld\n", - sha1_to_hex(target->sha1), prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, range_header); - } - - if (start_active_slot(slot)) { - run_active_slot(slot); + if (start_active_slot(preq->slot)) { + run_active_slot(preq->slot); if (results.curl_result != CURLE_OK) { - fclose(packfile); - slot->local = NULL; - return error("Unable to get pack file %s\n%s", url, - curl_errorstr); + error("Unable to get pack file %s\n%s", preq->url, + curl_errorstr); + goto abort; } } else { - fclose(packfile); - slot->local = NULL; - return error("Unable to start request"); + error("Unable to start request"); + goto abort; } - target->pack_size = ftell(packfile); - fclose(packfile); - slot->local = NULL; - - ret = move_temp_to_file(tmpfile, filename); + ret = finish_http_pack_request(preq); + release_http_pack_request(preq); if (ret) return ret; - lst = &repo->packs; - while (*lst != target) - lst = &((*lst)->next); - *lst = (*lst)->next; - - if (verify_pack(target)) - return -1; - install_packed_git(target); - return 0; + +abort: + return -1; } static void abort_object_request(struct object_request *obj_req) diff --git a/http.c b/http.c index 0701a6f6eb..381593441e 100644 --- a/http.c +++ b/http.c @@ -1,4 +1,5 @@ #include "http.h" +#include "pack.h" int data_received; int active_requests; @@ -889,3 +890,108 @@ cleanup: free(url); return ret; } + +void release_http_pack_request(struct http_pack_request *preq) +{ + if (preq->packfile != NULL) { + fclose(preq->packfile); + preq->packfile = NULL; + preq->slot->local = NULL; + } + if (preq->range_header != NULL) { + curl_slist_free_all(preq->range_header); + preq->range_header = NULL; + } + preq->slot = NULL; + free(preq->url); +} + +int finish_http_pack_request(struct http_pack_request *preq) +{ + int ret; + struct packed_git **lst; + + preq->target->pack_size = ftell(preq->packfile); + + if (preq->packfile != NULL) { + fclose(preq->packfile); + preq->packfile = NULL; + preq->slot->local = NULL; + } + + ret = move_temp_to_file(preq->tmpfile, preq->filename); + if (ret) + return ret; + + lst = preq->lst; + while (*lst != preq->target) + lst = &((*lst)->next); + *lst = (*lst)->next; + + if (verify_pack(preq->target)) + return -1; + install_packed_git(preq->target); + + return 0; +} + +struct http_pack_request *new_http_pack_request( + struct packed_git *target, const char *base_url) +{ + char *url; + char *filename; + long prev_posn = 0; + char range[RANGE_HEADER_SIZE]; + struct strbuf buf = STRBUF_INIT; + struct http_pack_request *preq; + + preq = xmalloc(sizeof(*preq)); + preq->target = target; + preq->range_header = NULL; + + end_url_with_slash(&buf, base_url); + strbuf_addf(&buf, "objects/pack/pack-%s.pack", + sha1_to_hex(target->sha1)); + url = strbuf_detach(&buf, NULL); + preq->url = xstrdup(url); + + filename = sha1_pack_name(target->sha1); + snprintf(preq->filename, sizeof(preq->filename), "%s", filename); + snprintf(preq->tmpfile, sizeof(preq->tmpfile), "%s.temp", filename); + preq->packfile = fopen(preq->tmpfile, "a"); + if (!preq->packfile) { + error("Unable to open local file %s for pack", + preq->tmpfile); + goto abort; + } + + preq->slot = get_active_slot(); + preq->slot->local = preq->packfile; + curl_easy_setopt(preq->slot->curl, CURLOPT_FILE, preq->packfile); + curl_easy_setopt(preq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite); + curl_easy_setopt(preq->slot->curl, CURLOPT_URL, url); + curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, + no_pragma_header); + + /* + * If there is data present from a previous transfer attempt, + * resume where it left off + */ + prev_posn = ftell(preq->packfile); + if (prev_posn>0) { + if (http_is_verbose) + fprintf(stderr, + "Resuming fetch of pack %s at byte %ld\n", + sha1_to_hex(target->sha1), prev_posn); + sprintf(range, "Range: bytes=%ld-", prev_posn); + preq->range_header = curl_slist_append(NULL, range); + curl_easy_setopt(preq->slot->curl, CURLOPT_HTTPHEADER, + preq->range_header); + } + + return preq; + +abort: + free(filename); + return NULL; +} diff --git a/http.h b/http.h index 180b1c2ae2..511c0c4417 100644 --- a/http.h +++ b/http.h @@ -150,4 +150,21 @@ extern int http_fetch_ref(const char *base, struct ref *ref); extern int http_get_info_packs(const char *base_url, struct packed_git **packs_head); +struct http_pack_request +{ + char *url; + struct packed_git *target; + struct packed_git **lst; + FILE *packfile; + char filename[PATH_MAX]; + char tmpfile[PATH_MAX]; + struct curl_slist *range_header; + struct active_request_slot *slot; +}; + +extern struct http_pack_request *new_http_pack_request( + struct packed_git *target, const char *base_url); +extern int finish_http_pack_request(struct http_pack_request *preq); +extern void release_http_pack_request(struct http_pack_request *preq); + #endif /* HTTP_H */ From 5424bc557fc6414660830b470dd45774b8f5f281 Mon Sep 17 00:00:00 2001 From: Tay Ray Chuan Date: Sat, 6 Jun 2009 16:44:02 +0800 Subject: [PATCH 25/35] http*: add helper methods for fetching objects (loose) The code handling the fetching of loose objects in http-push.c and http-walker.c have been refactored into new methods and a new struct (object_http_request) in http.c. They are not meant to be invoked elsewhere. The new methods in http.c are - new_http_object_request - process_http_object_request - finish_http_object_request - abort_http_object_request - release_http_object_request and the new struct is http_object_request. RANGER_HEADER_SIZE and no_pragma_header is no longer made available outside of http.c, since after the above changes, there are no other instances of usage outside of http.c. Remove members of the transfer_request struct in http-push.c and http-walker.c, including filename, real_sha1 and zret, as they are used no longer used. Move the methods append_remote_object_url() and get_remote_object_url() from http-push.c to http.c. Additionally, get_remote_object_url() is no longer defined only when USE_CURL_MULTI is defined, since non-USE_CURL_MULTI code in http.c uses it (namely, in new_http_object_request()). Refactor code from http-push.c::start_fetch_loose() and http-walker.c::start_object_fetch_request() that deals with the details of coming up with the filename to store the retrieved object, resuming a previously aborted request, and making a new curl request, into a new function, new_http_object_request(). Refactor code from http-walker.c::process_object_request() into the function, process_http_object_request(). Refactor code from http-push.c::finish_request() and http-walker.c::finish_object_request() into a new function, finish_http_object_request(). It returns the result of the move_temp_to_file() invocation. Add a function, release_http_object_request(), which cleans up object request data. http-push.c and http-walker.c invoke this function separately; http-push.c::release_request() and http-walker.c::release_object_request() do not invoke this function. Add a function, abort_http_object_request(), which unlink()s the object file and invokes release_http_object_request(). Update http-walker.c::abort_object_request() to use this. Signed-off-by: Tay Ray Chuan Signed-off-by: Junio C Hamano --- http-push.c | 211 +++------------------------------------ http-walker.c | 267 ++++++++------------------------------------------ http.c | 250 +++++++++++++++++++++++++++++++++++++++++++++- http.h | 37 ++++++- 4 files changed, 335 insertions(+), 430 deletions(-) diff --git a/http-push.c b/http-push.c index 8232cf488e..3439dcc5b8 100644 --- a/http-push.c +++ b/http-push.c @@ -115,18 +115,10 @@ struct transfer_request struct remote_lock *lock; struct curl_slist *headers; struct buffer buffer; - char filename[PATH_MAX]; - char tmpfile[PATH_MAX]; - int local_fileno; enum transfer_state state; CURLcode curl_result; char errorstr[CURL_ERROR_SIZE]; long http_code; - unsigned char real_sha1[20]; - git_SHA_CTX c; - z_stream stream; - int zret; - int rename; void *userData; struct active_request_slot *slot; struct transfer_request *next; @@ -232,15 +224,6 @@ static struct curl_slist *get_dav_token_headers(struct remote_lock *lock, enum d return dav_headers; } -static void append_remote_object_url(struct strbuf *buf, const char *url, - const char *hex, - int only_two_digit_prefix) -{ - strbuf_addf(buf, "%sobjects/%.*s/", url, 2, hex); - if (!only_two_digit_prefix) - strbuf_addf(buf, "%s", hex+2); -} - static void finish_request(struct transfer_request *request); static void release_request(struct transfer_request *request); @@ -254,169 +237,29 @@ static void process_response(void *callback_data) #ifdef USE_CURL_MULTI -static char *get_remote_object_url(const char *url, const char *hex, - int only_two_digit_prefix) -{ - struct strbuf buf = STRBUF_INIT; - append_remote_object_url(&buf, url, hex, only_two_digit_prefix); - return strbuf_detach(&buf, NULL); -} - -static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, - void *data) -{ - unsigned char expn[4096]; - size_t size = eltsize * nmemb; - int posn = 0; - struct transfer_request *request = (struct transfer_request *)data; - do { - ssize_t retval = xwrite(request->local_fileno, - (char *) ptr + posn, size - posn); - if (retval < 0) - return posn; - posn += retval; - } while (posn < size); - - request->stream.avail_in = size; - request->stream.next_in = ptr; - do { - request->stream.next_out = expn; - request->stream.avail_out = sizeof(expn); - request->zret = git_inflate(&request->stream, Z_SYNC_FLUSH); - git_SHA1_Update(&request->c, expn, - sizeof(expn) - request->stream.avail_out); - } while (request->stream.avail_in && request->zret == Z_OK); - data_received++; - return size; -} - static void start_fetch_loose(struct transfer_request *request) { - char *hex = sha1_to_hex(request->obj->sha1); - char *filename; - char prevfile[PATH_MAX]; - char *url; - int prevlocal; - unsigned char prev_buf[PREV_BUF_SIZE]; - ssize_t prev_read = 0; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; struct active_request_slot *slot; + struct http_object_request *obj_req; - filename = sha1_file_name(request->obj->sha1); - snprintf(request->filename, sizeof(request->filename), "%s", filename); - snprintf(request->tmpfile, sizeof(request->tmpfile), - "%s.temp", filename); - - snprintf(prevfile, sizeof(prevfile), "%s.prev", request->filename); - unlink_or_warn(prevfile); - rename(request->tmpfile, prevfile); - unlink_or_warn(request->tmpfile); - - if (request->local_fileno != -1) - error("fd leakage in start: %d", request->local_fileno); - request->local_fileno = open(request->tmpfile, - O_WRONLY | O_CREAT | O_EXCL, 0666); - /* - * This could have failed due to the "lazy directory creation"; - * try to mkdir the last path component. - */ - if (request->local_fileno < 0 && errno == ENOENT) { - char *dir = strrchr(request->tmpfile, '/'); - if (dir) { - *dir = 0; - mkdir(request->tmpfile, 0777); - *dir = '/'; - } - request->local_fileno = open(request->tmpfile, - O_WRONLY | O_CREAT | O_EXCL, 0666); - } - - if (request->local_fileno < 0) { + obj_req = new_http_object_request(repo->url, request->obj->sha1); + if (obj_req == NULL) { request->state = ABORTED; - error("Couldn't create temporary file %s for %s: %s", - request->tmpfile, request->filename, strerror(errno)); return; } - memset(&request->stream, 0, sizeof(request->stream)); - - git_inflate_init(&request->stream); - - git_SHA1_Init(&request->c); - - url = get_remote_object_url(repo->url, hex, 0); - request->url = xstrdup(url); - - /* - * If a previous temp file is present, process what was already - * fetched. - */ - prevlocal = open(prevfile, O_RDONLY); - if (prevlocal != -1) { - do { - prev_read = xread(prevlocal, prev_buf, PREV_BUF_SIZE); - if (prev_read>0) { - if (fwrite_sha1_file(prev_buf, - 1, - prev_read, - request) == prev_read) - prev_posn += prev_read; - else - prev_read = -1; - } - } while (prev_read > 0); - close(prevlocal); - } - unlink_or_warn(prevfile); - - /* - * Reset inflate/SHA1 if there was an error reading the previous temp - * file; also rewind to the beginning of the local file. - */ - if (prev_read == -1) { - memset(&request->stream, 0, sizeof(request->stream)); - git_inflate_init(&request->stream); - git_SHA1_Init(&request->c); - if (prev_posn>0) { - prev_posn = 0; - lseek(request->local_fileno, 0, SEEK_SET); - ftruncate(request->local_fileno, 0); - } - } - - slot = get_active_slot(); + slot = obj_req->slot; slot->callback_func = process_response; slot->callback_data = request; request->slot = slot; - - curl_easy_setopt(slot->curl, CURLOPT_FILE, request); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); - curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, request->errorstr); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - - /* - * If we have successfully processed data from a previous fetch - * attempt, only fetch the data we don't already have. - */ - if (prev_posn>0) { - if (push_verbosely) - fprintf(stderr, - "Resuming fetch of object %s at byte %ld\n", - hex, prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, - CURLOPT_HTTPHEADER, range_header); - } + request->userData = obj_req; /* Try to get the request started, abort the request on error */ request->state = RUN_FETCH_LOOSE; if (!start_active_slot(slot)) { fprintf(stderr, "Unable to start GET request\n"); repo->can_update_info_refs = 0; + release_http_object_request(obj_req); release_request(request); } } @@ -675,16 +518,14 @@ static void release_request(struct transfer_request *request) entry->next = entry->next->next; } - if (request->local_fileno != -1) - close(request->local_fileno); free(request->url); free(request); } static void finish_request(struct transfer_request *request) { - struct stat st; struct http_pack_request *preq; + struct http_object_request *obj_req; request->curl_result = request->slot->curl_result; request->http_code = request->slot->http_code; @@ -739,39 +580,17 @@ static void finish_request(struct transfer_request *request) aborted = 1; } } else if (request->state == RUN_FETCH_LOOSE) { - close(request->local_fileno); - request->local_fileno = -1; + obj_req = (struct http_object_request *)request->userData; - if (request->curl_result != CURLE_OK && - request->http_code != 416) { - if (stat(request->tmpfile, &st) == 0) { - if (st.st_size == 0) - unlink_or_warn(request->tmpfile); - } - } else { - if (request->http_code == 416) - warning("requested range invalid; we may already have all the data."); - - git_inflate_end(&request->stream); - git_SHA1_Final(request->real_sha1, &request->c); - if (request->zret != Z_STREAM_END) { - unlink_or_warn(request->tmpfile); - } else if (hashcmp(request->obj->sha1, request->real_sha1)) { - unlink_or_warn(request->tmpfile); - } else { - request->rename = - move_temp_to_file( - request->tmpfile, - request->filename); - if (request->rename == 0) - request->obj->flags |= (LOCAL | REMOTE); - } - } + if (finish_http_object_request(obj_req) == 0) + if (obj_req->rename == 0) + request->obj->flags |= (LOCAL | REMOTE); /* Try fetching packed if necessary */ - if (request->obj->flags & LOCAL) + if (request->obj->flags & LOCAL) { + release_http_object_request(obj_req); release_request(request); - else + } else start_fetch_packed(request); } else if (request->state == RUN_FETCH_PACKED) { @@ -843,7 +662,6 @@ static void add_fetch_request(struct object *obj) request->url = NULL; request->lock = NULL; request->headers = NULL; - request->local_fileno = -1; request->state = NEED_FETCH; request->next = request_queue_head; request_queue_head = request; @@ -882,7 +700,6 @@ static int add_send_request(struct object *obj, struct remote_lock *lock) request->url = NULL; request->lock = lock; request->headers = NULL; - request->local_fileno = -1; request->state = NEED_PUSH; request->next = request_queue_head; request_queue_head = request; diff --git a/http-walker.c b/http-walker.c index 8f7a975f96..700bc13112 100644 --- a/http-walker.c +++ b/http-walker.c @@ -3,8 +3,6 @@ #include "walker.h" #include "http.h" -#define PREV_BUF_SIZE 4096 - struct alt_base { char *base; @@ -25,20 +23,8 @@ struct object_request struct walker *walker; unsigned char sha1[20]; struct alt_base *repo; - char *url; - char filename[PATH_MAX]; - char tmpfile[PATH_MAX]; - int local; enum object_request_state state; - CURLcode curl_result; - char errorstr[CURL_ERROR_SIZE]; - long http_code; - unsigned char real_sha1[20]; - git_SHA_CTX c; - z_stream stream; - int zret; - int rename; - struct active_request_slot *slot; + struct http_object_request *req; struct object_request *next; }; @@ -59,34 +45,6 @@ struct walker_data { static struct object_request *object_queue_head; -static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, - void *data) -{ - unsigned char expn[4096]; - size_t size = eltsize * nmemb; - int posn = 0; - struct object_request *obj_req = (struct object_request *)data; - do { - ssize_t retval = xwrite(obj_req->local, - (char *) ptr + posn, size - posn); - if (retval < 0) - return posn; - posn += retval; - } while (posn < size); - - obj_req->stream.avail_in = size; - obj_req->stream.next_in = ptr; - do { - obj_req->stream.next_out = expn; - obj_req->stream.avail_out = sizeof(expn); - obj_req->zret = git_inflate(&obj_req->stream, Z_SYNC_FLUSH); - git_SHA1_Update(&obj_req->c, expn, - sizeof(expn) - obj_req->stream.avail_out); - } while (obj_req->stream.avail_in && obj_req->zret == Z_OK); - data_received++; - return size; -} - static void fetch_alternates(struct walker *walker, const char *base); static void process_object_response(void *callback_data); @@ -94,172 +52,35 @@ static void process_object_response(void *callback_data); static void start_object_request(struct walker *walker, struct object_request *obj_req) { - char *hex = sha1_to_hex(obj_req->sha1); - char prevfile[PATH_MAX]; - char *url; - char *posn; - int prevlocal; - unsigned char prev_buf[PREV_BUF_SIZE]; - ssize_t prev_read = 0; - long prev_posn = 0; - char range[RANGE_HEADER_SIZE]; - struct curl_slist *range_header = NULL; struct active_request_slot *slot; + struct http_object_request *req; - snprintf(prevfile, sizeof(prevfile), "%s.prev", obj_req->filename); - unlink_or_warn(prevfile); - rename(obj_req->tmpfile, prevfile); - unlink_or_warn(obj_req->tmpfile); - - if (obj_req->local != -1) - error("fd leakage in start: %d", obj_req->local); - obj_req->local = open(obj_req->tmpfile, - O_WRONLY | O_CREAT | O_EXCL, 0666); - /* - * This could have failed due to the "lazy directory creation"; - * try to mkdir the last path component. - */ - if (obj_req->local < 0 && errno == ENOENT) { - char *dir = strrchr(obj_req->tmpfile, '/'); - if (dir) { - *dir = 0; - mkdir(obj_req->tmpfile, 0777); - *dir = '/'; - } - obj_req->local = open(obj_req->tmpfile, - O_WRONLY | O_CREAT | O_EXCL, 0666); - } - - if (obj_req->local < 0) { + req = new_http_object_request(obj_req->repo->base, obj_req->sha1); + if (req == NULL) { obj_req->state = ABORTED; - error("Couldn't create temporary file %s for %s: %s", - obj_req->tmpfile, obj_req->filename, strerror(errno)); return; } + obj_req->req = req; - memset(&obj_req->stream, 0, sizeof(obj_req->stream)); - - git_inflate_init(&obj_req->stream); - - git_SHA1_Init(&obj_req->c); - - url = xmalloc(strlen(obj_req->repo->base) + 51); - obj_req->url = xmalloc(strlen(obj_req->repo->base) + 51); - strcpy(url, obj_req->repo->base); - posn = url + strlen(obj_req->repo->base); - strcpy(posn, "/objects/"); - posn += 9; - memcpy(posn, hex, 2); - posn += 2; - *(posn++) = '/'; - strcpy(posn, hex + 2); - strcpy(obj_req->url, url); - - /* - * If a previous temp file is present, process what was already - * fetched. - */ - prevlocal = open(prevfile, O_RDONLY); - if (prevlocal != -1) { - do { - prev_read = xread(prevlocal, prev_buf, PREV_BUF_SIZE); - if (prev_read>0) { - if (fwrite_sha1_file(prev_buf, - 1, - prev_read, - obj_req) == prev_read) - prev_posn += prev_read; - else - prev_read = -1; - } - } while (prev_read > 0); - close(prevlocal); - } - unlink_or_warn(prevfile); - - /* - * Reset inflate/SHA1 if there was an error reading the previous temp - * file; also rewind to the beginning of the local file. - */ - if (prev_read == -1) { - memset(&obj_req->stream, 0, sizeof(obj_req->stream)); - git_inflate_init(&obj_req->stream); - git_SHA1_Init(&obj_req->c); - if (prev_posn>0) { - prev_posn = 0; - lseek(obj_req->local, 0, SEEK_SET); - ftruncate(obj_req->local, 0); - } - } - - slot = get_active_slot(); + slot = req->slot; slot->callback_func = process_object_response; slot->callback_data = obj_req; - obj_req->slot = slot; - - curl_easy_setopt(slot->curl, CURLOPT_FILE, obj_req); - curl_easy_setopt(slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); - curl_easy_setopt(slot->curl, CURLOPT_ERRORBUFFER, obj_req->errorstr); - curl_easy_setopt(slot->curl, CURLOPT_URL, url); - curl_easy_setopt(slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); - - /* - * If we have successfully processed data from a previous fetch - * attempt, only fetch the data we don't already have. - */ - if (prev_posn>0) { - if (walker->get_verbosely) - fprintf(stderr, - "Resuming fetch of object %s at byte %ld\n", - hex, prev_posn); - sprintf(range, "Range: bytes=%ld-", prev_posn); - range_header = curl_slist_append(range_header, range); - curl_easy_setopt(slot->curl, - CURLOPT_HTTPHEADER, range_header); - } /* Try to get the request started, abort the request on error */ obj_req->state = ACTIVE; if (!start_active_slot(slot)) { obj_req->state = ABORTED; - obj_req->slot = NULL; - close(obj_req->local); - obj_req->local = -1; - free(obj_req->url); + release_http_object_request(req); return; } } static void finish_object_request(struct object_request *obj_req) { - struct stat st; - - close(obj_req->local); - obj_req->local = -1; - - if (obj_req->http_code == 416) { - fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n"); - } else if (obj_req->curl_result != CURLE_OK) { - if (stat(obj_req->tmpfile, &st) == 0) - if (st.st_size == 0) - unlink_or_warn(obj_req->tmpfile); + if (finish_http_object_request(obj_req->req)) return; - } - git_inflate_end(&obj_req->stream); - git_SHA1_Final(obj_req->real_sha1, &obj_req->c); - if (obj_req->zret != Z_STREAM_END) { - unlink_or_warn(obj_req->tmpfile); - return; - } - if (hashcmp(obj_req->sha1, obj_req->real_sha1)) { - unlink_or_warn(obj_req->tmpfile); - return; - } - obj_req->rename = - move_temp_to_file(obj_req->tmpfile, obj_req->filename); - - if (obj_req->rename == 0) + if (obj_req->req->rename == 0) walker_say(obj_req->walker, "got %s\n", sha1_to_hex(obj_req->sha1)); } @@ -271,19 +92,16 @@ static void process_object_response(void *callback_data) struct walker_data *data = walker->data; struct alt_base *alt = data->alt; - obj_req->curl_result = obj_req->slot->curl_result; - obj_req->http_code = obj_req->slot->http_code; - obj_req->slot = NULL; + process_http_object_request(obj_req->req); obj_req->state = COMPLETE; /* Use alternates if necessary */ - if (missing_target(obj_req)) { + if (missing_target(obj_req->req)) { fetch_alternates(walker, alt->base); if (obj_req->repo->next != NULL) { obj_req->repo = obj_req->repo->next; - close(obj_req->local); - obj_req->local = -1; + release_http_object_request(obj_req->req); start_object_request(walker, obj_req); return; } @@ -296,8 +114,8 @@ static void release_object_request(struct object_request *obj_req) { struct object_request *entry = object_queue_head; - if (obj_req->local != -1) - error("fd leakage in release: %d", obj_req->local); + if (obj_req->req !=NULL && obj_req->req->localfile != -1) + error("fd leakage in release: %d", obj_req->req->localfile); if (obj_req == object_queue_head) { object_queue_head = obj_req->next; } else { @@ -307,7 +125,6 @@ static void release_object_request(struct object_request *obj_req) entry->next = entry->next->next; } - free(obj_req->url); free(obj_req); } @@ -335,19 +152,13 @@ static void prefetch(struct walker *walker, unsigned char *sha1) struct object_request *newreq; struct object_request *tail; struct walker_data *data = walker->data; - char *filename = sha1_file_name(sha1); newreq = xmalloc(sizeof(*newreq)); newreq->walker = walker; hashcpy(newreq->sha1, sha1); newreq->repo = data->alt; - newreq->url = NULL; - newreq->local = -1; newreq->state = WAITING; - snprintf(newreq->filename, sizeof(newreq->filename), "%s", filename); - snprintf(newreq->tmpfile, sizeof(newreq->tmpfile), - "%s.temp", filename); - newreq->slot = NULL; + newreq->req = NULL; newreq->next = NULL; http_is_verbose = walker->get_verbosely; @@ -638,15 +449,6 @@ abort: static void abort_object_request(struct object_request *obj_req) { - if (obj_req->local >= 0) { - close(obj_req->local); - obj_req->local = -1; - } - unlink_or_warn(obj_req->tmpfile); - if (obj_req->slot) { - release_active_slot(obj_req->slot); - obj_req->slot = NULL; - } release_object_request(obj_req); } @@ -655,6 +457,7 @@ static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned c char *hex = sha1_to_hex(sha1); int ret = 0; struct object_request *obj_req = object_queue_head; + struct http_object_request *req; while (obj_req != NULL && hashcmp(obj_req->sha1, sha1)) obj_req = obj_req->next; @@ -662,6 +465,8 @@ static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned c return error("Couldn't find request for %s in the queue", hex); if (has_sha1_file(obj_req->sha1)) { + if (obj_req->req != NULL) + abort_http_object_request(obj_req->req); abort_object_request(obj_req); return 0; } @@ -673,34 +478,42 @@ static int fetch_object(struct walker *walker, struct alt_base *repo, unsigned c start_object_request(walker, obj_req); #endif + /* + * obj_req->req might change when fetching alternates in the callback + * process_object_response; therefore, the "shortcut" variable, req, + * is used only after we're done with slots. + */ while (obj_req->state == ACTIVE) - run_active_slot(obj_req->slot); + run_active_slot(obj_req->req->slot); - if (obj_req->local != -1) { - close(obj_req->local); - obj_req->local = -1; + req = obj_req->req; + + if (req->localfile != -1) { + close(req->localfile); + req->localfile = -1; } if (obj_req->state == ABORTED) { ret = error("Request for %s aborted", hex); - } else if (obj_req->curl_result != CURLE_OK && - obj_req->http_code != 416) { - if (missing_target(obj_req)) + } else if (req->curl_result != CURLE_OK && + req->http_code != 416) { + if (missing_target(req)) ret = -1; /* Be silent, it is probably in a pack. */ else ret = error("%s (curl_result = %d, http_code = %ld, sha1 = %s)", - obj_req->errorstr, obj_req->curl_result, - obj_req->http_code, hex); - } else if (obj_req->zret != Z_STREAM_END) { + req->errorstr, req->curl_result, + req->http_code, hex); + } else if (req->zret != Z_STREAM_END) { walker->corrupt_object_found++; - ret = error("File %s (%s) corrupt", hex, obj_req->url); - } else if (hashcmp(obj_req->sha1, obj_req->real_sha1)) { + ret = error("File %s (%s) corrupt", hex, req->url); + } else if (hashcmp(obj_req->sha1, req->real_sha1)) { ret = error("File %s has bad hash", hex); - } else if (obj_req->rename < 0) { + } else if (req->rename < 0) { ret = error("unable to write sha1 filename %s", - obj_req->filename); + req->filename); } + release_http_object_request(req); release_object_request(obj_req); return ret; } diff --git a/http.c b/http.c index 381593441e..95b2137cfb 100644 --- a/http.c +++ b/http.c @@ -12,6 +12,10 @@ static CURLM *curlm; #ifndef NO_CURL_EASY_DUPHANDLE static CURL *curl_default; #endif + +#define PREV_BUF_SIZE 4096 +#define RANGE_HEADER_SIZE 30 + char curl_errorstr[CURL_ERROR_SIZE]; static int curl_ssl_verify = -1; @@ -30,8 +34,7 @@ static const char *curl_http_proxy; static char *user_name, *user_pass; static struct curl_slist *pragma_header; - -struct curl_slist *no_pragma_header; +static struct curl_slist *no_pragma_header; static struct active_request_slot *active_queue_head; @@ -666,6 +669,23 @@ static char *quote_ref_url(const char *base, const char *ref) return strbuf_detach(&buf, NULL); } +void append_remote_object_url(struct strbuf *buf, const char *url, + const char *hex, + int only_two_digit_prefix) +{ + strbuf_addf(buf, "%s/objects/%.*s/", url, 2, hex); + if (!only_two_digit_prefix) + strbuf_addf(buf, "%s", hex+2); +} + +char *get_remote_object_url(const char *url, const char *hex, + int only_two_digit_prefix) +{ + struct strbuf buf = STRBUF_INIT; + append_remote_object_url(&buf, url, hex, only_two_digit_prefix); + return strbuf_detach(&buf, NULL); +} + /* http_request() targets */ #define HTTP_REQUEST_STRBUF 0 #define HTTP_REQUEST_FILE 1 @@ -995,3 +1015,229 @@ abort: free(filename); return NULL; } + +/* Helpers for fetching objects (loose) */ +static size_t fwrite_sha1_file(void *ptr, size_t eltsize, size_t nmemb, + void *data) +{ + unsigned char expn[4096]; + size_t size = eltsize * nmemb; + int posn = 0; + struct http_object_request *freq = + (struct http_object_request *)data; + do { + ssize_t retval = xwrite(freq->localfile, + (char *) ptr + posn, size - posn); + if (retval < 0) + return posn; + posn += retval; + } while (posn < size); + + freq->stream.avail_in = size; + freq->stream.next_in = ptr; + do { + freq->stream.next_out = expn; + freq->stream.avail_out = sizeof(expn); + freq->zret = git_inflate(&freq->stream, Z_SYNC_FLUSH); + git_SHA1_Update(&freq->c, expn, + sizeof(expn) - freq->stream.avail_out); + } while (freq->stream.avail_in && freq->zret == Z_OK); + data_received++; + return size; +} + +struct http_object_request *new_http_object_request(const char *base_url, + unsigned char *sha1) +{ + char *hex = sha1_to_hex(sha1); + char *filename; + char prevfile[PATH_MAX]; + char *url; + int prevlocal; + unsigned char prev_buf[PREV_BUF_SIZE]; + ssize_t prev_read = 0; + long prev_posn = 0; + char range[RANGE_HEADER_SIZE]; + struct curl_slist *range_header = NULL; + struct http_object_request *freq; + + freq = xmalloc(sizeof(*freq)); + hashcpy(freq->sha1, sha1); + freq->localfile = -1; + + filename = sha1_file_name(sha1); + snprintf(freq->filename, sizeof(freq->filename), "%s", filename); + snprintf(freq->tmpfile, sizeof(freq->tmpfile), + "%s.temp", filename); + + snprintf(prevfile, sizeof(prevfile), "%s.prev", filename); + unlink_or_warn(prevfile); + rename(freq->tmpfile, prevfile); + unlink_or_warn(freq->tmpfile); + + if (freq->localfile != -1) + error("fd leakage in start: %d", freq->localfile); + freq->localfile = open(freq->tmpfile, + O_WRONLY | O_CREAT | O_EXCL, 0666); + /* + * This could have failed due to the "lazy directory creation"; + * try to mkdir the last path component. + */ + if (freq->localfile < 0 && errno == ENOENT) { + char *dir = strrchr(freq->tmpfile, '/'); + if (dir) { + *dir = 0; + mkdir(freq->tmpfile, 0777); + *dir = '/'; + } + freq->localfile = open(freq->tmpfile, + O_WRONLY | O_CREAT | O_EXCL, 0666); + } + + if (freq->localfile < 0) { + error("Couldn't create temporary file %s for %s: %s", + freq->tmpfile, freq->filename, strerror(errno)); + goto abort; + } + + memset(&freq->stream, 0, sizeof(freq->stream)); + + git_inflate_init(&freq->stream); + + git_SHA1_Init(&freq->c); + + url = get_remote_object_url(base_url, hex, 0); + freq->url = xstrdup(url); + + /* + * If a previous temp file is present, process what was already + * fetched. + */ + prevlocal = open(prevfile, O_RDONLY); + if (prevlocal != -1) { + do { + prev_read = xread(prevlocal, prev_buf, PREV_BUF_SIZE); + if (prev_read>0) { + if (fwrite_sha1_file(prev_buf, + 1, + prev_read, + freq) == prev_read) { + prev_posn += prev_read; + } else { + prev_read = -1; + } + } + } while (prev_read > 0); + close(prevlocal); + } + unlink_or_warn(prevfile); + + /* + * Reset inflate/SHA1 if there was an error reading the previous temp + * file; also rewind to the beginning of the local file. + */ + if (prev_read == -1) { + memset(&freq->stream, 0, sizeof(freq->stream)); + git_inflate_init(&freq->stream); + git_SHA1_Init(&freq->c); + if (prev_posn>0) { + prev_posn = 0; + lseek(freq->localfile, 0, SEEK_SET); + ftruncate(freq->localfile, 0); + } + } + + freq->slot = get_active_slot(); + + curl_easy_setopt(freq->slot->curl, CURLOPT_FILE, freq); + curl_easy_setopt(freq->slot->curl, CURLOPT_WRITEFUNCTION, fwrite_sha1_file); + curl_easy_setopt(freq->slot->curl, CURLOPT_ERRORBUFFER, freq->errorstr); + curl_easy_setopt(freq->slot->curl, CURLOPT_URL, url); + curl_easy_setopt(freq->slot->curl, CURLOPT_HTTPHEADER, no_pragma_header); + + /* + * If we have successfully processed data from a previous fetch + * attempt, only fetch the data we don't already have. + */ + if (prev_posn>0) { + if (http_is_verbose) + fprintf(stderr, + "Resuming fetch of object %s at byte %ld\n", + hex, prev_posn); + sprintf(range, "Range: bytes=%ld-", prev_posn); + range_header = curl_slist_append(range_header, range); + curl_easy_setopt(freq->slot->curl, + CURLOPT_HTTPHEADER, range_header); + } + + return freq; + + free(url); +abort: + free(filename); + free(freq); + return NULL; +} + +void process_http_object_request(struct http_object_request *freq) +{ + if (freq->slot == NULL) + return; + freq->curl_result = freq->slot->curl_result; + freq->http_code = freq->slot->http_code; + freq->slot = NULL; +} + +int finish_http_object_request(struct http_object_request *freq) +{ + struct stat st; + + close(freq->localfile); + freq->localfile = -1; + + process_http_object_request(freq); + + if (freq->http_code == 416) { + fprintf(stderr, "Warning: requested range invalid; we may already have all the data.\n"); + } else if (freq->curl_result != CURLE_OK) { + if (stat(freq->tmpfile, &st) == 0) + if (st.st_size == 0) + unlink_or_warn(freq->tmpfile); + return -1; + } + + git_inflate_end(&freq->stream); + git_SHA1_Final(freq->real_sha1, &freq->c); + if (freq->zret != Z_STREAM_END) { + unlink_or_warn(freq->tmpfile); + return -1; + } + if (hashcmp(freq->sha1, freq->real_sha1)) { + unlink_or_warn(freq->tmpfile); + return -1; + } + freq->rename = + move_temp_to_file(freq->tmpfile, freq->filename); + + return freq->rename; +} + +void abort_http_object_request(struct http_object_request *freq) +{ + unlink_or_warn(freq->tmpfile); + + release_http_object_request(freq); +} + +void release_http_object_request(struct http_object_request *freq) +{ + if (freq->localfile != -1) { + close(freq->localfile); + freq->localfile = -1; + } + if (freq->url != NULL) { + free(freq->url); + freq->url = NULL; + } + freq->slot = NULL; +} diff --git a/http.h b/http.h index 511c0c4417..4c4e99c2f6 100644 --- a/http.h +++ b/http.h @@ -88,10 +88,6 @@ extern void add_fill_function(void *data, int (*fill)(void *)); extern void step_active_slots(void); #endif -extern struct curl_slist *no_pragma_header; - -#define RANGE_HEADER_SIZE 30 - extern void http_init(struct remote *remote); extern void http_cleanup(void); @@ -114,6 +110,13 @@ static inline int missing__target(int code, int result) #define missing_target(a) missing__target((a)->http_code, (a)->curl_result) +/* Helpers for modifying and creating URLs */ +extern void append_remote_object_url(struct strbuf *buf, const char *url, + const char *hex, + int only_two_digit_prefix); +extern char *get_remote_object_url(const char *url, const char *hex, + int only_two_digit_prefix); + /* Options for http_request_*() */ #define HTTP_NO_CACHE 1 @@ -167,4 +170,30 @@ extern struct http_pack_request *new_http_pack_request( extern int finish_http_pack_request(struct http_pack_request *preq); extern void release_http_pack_request(struct http_pack_request *preq); +/* Helpers for fetching object */ +struct http_object_request +{ + char *url; + char filename[PATH_MAX]; + char tmpfile[PATH_MAX]; + int localfile; + CURLcode curl_result; + char errorstr[CURL_ERROR_SIZE]; + long http_code; + unsigned char sha1[20]; + unsigned char real_sha1[20]; + git_SHA_CTX c; + z_stream stream; + int zret; + int rename; + struct active_request_slot *slot; +}; + +extern struct http_object_request *new_http_object_request( + const char *base_url, unsigned char *sha1); +extern void process_http_object_request(struct http_object_request *freq); +extern int finish_http_object_request(struct http_object_request *freq); +extern void abort_http_object_request(struct http_object_request *freq); +extern void release_http_object_request(struct http_object_request *freq); + #endif /* HTTP_H */ From 9af3589e0e42eb289dfdb8bb4031e5bec4923308 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 6 Jun 2009 06:41:33 +0200 Subject: [PATCH 26/35] bisect: add parameters to "filter_skipped" because we will need to get more information from this function in some later patches. The new "int *count" parameter gives the number of commits left after the skipped commit have been filtered out. The new "int *skipped_first" parameter tells us if the first commit in the list has been skipped. Note that using this parameter also changes the behavior of the function if the first commit is indeed skipped. Because we assume that in this case we will want all the filtered commits, not just the first one, even if "show_all" is not set. So using a not NULL "skipped_first" parameter really means that we plan to choose to test another commit than the first non skipped one if the first commit in the list is skipped. That in turn means that, in case the first commit is skipped, we have to return a fully filtered list. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 40 ++++++++++++++++++++++++++++++++++++---- bisect.h | 4 +++- builtin-rev-list.c | 4 +++- 3 files changed, 42 insertions(+), 6 deletions(-) diff --git a/bisect.c b/bisect.c index 2c14f4d616..115cf5fa41 100644 --- a/bisect.c +++ b/bisect.c @@ -521,14 +521,34 @@ static char *join_sha1_array_hex(struct sha1_array *array, char delim) return strbuf_detach(&joined_hexs, NULL); } +/* + * In this function, passing a not NULL skipped_first is very special. + * It means that we want to know if the first commit in the list is + * skipped because we will want to test a commit away from it if it is + * indeed skipped. + * So if the first commit is skipped, we cannot take the shortcut to + * just "return list" when we find the first non skipped commit, we + * have to return a fully filtered list. + * + * We use (*skipped_first == -1) to mean "it has been found that the + * first commit is not skipped". In this case *skipped_first is set back + * to 0 just before the function returns. + */ struct commit_list *filter_skipped(struct commit_list *list, struct commit_list **tried, - int show_all) + int show_all, + int *count, + int *skipped_first) { struct commit_list *filtered = NULL, **f = &filtered; *tried = NULL; + if (skipped_first) + *skipped_first = 0; + if (count) + *count = 0; + if (!skipped_revs.sha1_nr) return list; @@ -537,19 +557,31 @@ struct commit_list *filter_skipped(struct commit_list *list, list->next = NULL; if (0 <= lookup_sha1_array(&skipped_revs, list->item->object.sha1)) { + if (skipped_first && !*skipped_first) + *skipped_first = 1; /* Move current to tried list */ *tried = list; tried = &list->next; } else { - if (!show_all) - return list; + if (!show_all) { + if (!skipped_first || !*skipped_first) + return list; + } else if (skipped_first && !*skipped_first) { + /* This means we know it's not skipped */ + *skipped_first = -1; + } /* Move current to filtered list */ *f = list; f = &list->next; + if (count) + (*count)++; } list = next; } + if (skipped_first && *skipped_first == -1) + *skipped_first = 0; + return filtered; } @@ -865,7 +897,7 @@ int bisect_next_all(const char *prefix) revs.commits = find_bisection(revs.commits, &reaches, &all, !!skipped_revs.sha1_nr); - revs.commits = filter_skipped(revs.commits, &tried, 0); + revs.commits = filter_skipped(revs.commits, &tried, 0, NULL, NULL); if (!revs.commits) { /* diff --git a/bisect.h b/bisect.h index fb744fdb79..82f8fc1910 100644 --- a/bisect.h +++ b/bisect.h @@ -7,7 +7,9 @@ extern struct commit_list *find_bisection(struct commit_list *list, extern struct commit_list *filter_skipped(struct commit_list *list, struct commit_list **tried, - int show_all); + int show_all, + int *count, + int *skipped_first); extern void print_commit_list(struct commit_list *list, const char *format_cur, diff --git a/builtin-rev-list.c b/builtin-rev-list.c index 31ea5f4aac..4ba1c12e0b 100644 --- a/builtin-rev-list.c +++ b/builtin-rev-list.c @@ -262,7 +262,9 @@ int show_bisect_vars(struct rev_list_info *info, int reaches, int all) if (!revs->commits && !(flags & BISECT_SHOW_TRIED)) return 1; - revs->commits = filter_skipped(revs->commits, &tried, flags & BISECT_SHOW_ALL); + revs->commits = filter_skipped(revs->commits, &tried, + flags & BISECT_SHOW_ALL, + NULL, NULL); /* * revs->commits can reach "reaches" commits among From 62d0b0daf12239fdb898a0d197dfc49a5e2742b0 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 6 Jun 2009 06:41:34 +0200 Subject: [PATCH 27/35] bisect: when skipping, choose a commit away from a skipped commit To do that a new function "apply_skip_ratio" is added and another function "managed_skipped" is created to wrap both "filter_skipped" and the previous one. In "managed_skipped" we detect when we should choose a commit away from a skipped one and then we automatically choose a skip ratio to pass to "apply_skip_ratio". The ratio is choosen so that it alternates between 1/5, 2/5 and 3/5. In "apply_skip_ratio", we ignore a given ratio of all the commits that could be tested. Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- bisect.c | 50 +++++++++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 49 insertions(+), 1 deletion(-) diff --git a/bisect.c b/bisect.c index 115cf5fa41..6fdff05722 100644 --- a/bisect.c +++ b/bisect.c @@ -585,6 +585,54 @@ struct commit_list *filter_skipped(struct commit_list *list, return filtered; } +static struct commit_list *apply_skip_ratio(struct commit_list *list, + int count, + int skip_num, int skip_denom) +{ + int index, i; + struct commit_list *cur, *previous; + + cur = list; + previous = NULL; + index = count * skip_num / skip_denom; + + for (i = 0; cur; cur = cur->next, i++) { + if (i == index) { + if (hashcmp(cur->item->object.sha1, current_bad_sha1)) + return cur; + if (previous) + return previous; + return list; + } + previous = cur; + } + + return list; +} + +static struct commit_list *managed_skipped(struct commit_list *list, + struct commit_list **tried) +{ + int count, skipped_first; + int skip_num, skip_denom; + + *tried = NULL; + + if (!skipped_revs.sha1_nr) + return list; + + list = filter_skipped(list, tried, 0, &count, &skipped_first); + + if (!skipped_first) + return list; + + /* Use alternatively 1/5, 2/5 and 3/5 as skip ratio. */ + skip_num = count % 3 + 1; + skip_denom = 5; + + return apply_skip_ratio(list, count, skip_num, skip_denom); +} + static void bisect_rev_setup(struct rev_info *revs, const char *prefix, const char *bad_format, const char *good_format, int read_paths) @@ -897,7 +945,7 @@ int bisect_next_all(const char *prefix) revs.commits = find_bisection(revs.commits, &reaches, &all, !!skipped_revs.sha1_nr); - revs.commits = filter_skipped(revs.commits, &tried, 0, NULL, NULL); + revs.commits = managed_skipped(revs.commits, &tried); if (!revs.commits) { /* From a66037c9755a2beb49bbd915e6f0dd9b1a732925 Mon Sep 17 00:00:00 2001 From: Christian Couder Date: Sat, 6 Jun 2009 06:41:35 +0200 Subject: [PATCH 28/35] t6030: test skipping away from an already skipped commit Signed-off-by: Christian Couder Signed-off-by: Junio C Hamano --- t/t6030-bisect-porcelain.sh | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/t/t6030-bisect-porcelain.sh b/t/t6030-bisect-porcelain.sh index 5254b23512..4556cdd8d2 100755 --- a/t/t6030-bisect-porcelain.sh +++ b/t/t6030-bisect-porcelain.sh @@ -555,6 +555,18 @@ test_expect_success 'restricting bisection on one dir and a file' ' grep "$PARA_HASH4 is first bad commit" my_bisect_log.txt ' +test_expect_success 'skipping away from skipped commit' ' + git bisect start $PARA_HASH7 $HASH1 && + para4=$(git rev-parse --verify HEAD) && + test "$para4" = "$PARA_HASH4" && + git bisect skip && + hash7=$(git rev-parse --verify HEAD) && + test "$hash7" = "$HASH7" && + git bisect skip && + hash3=$(git rev-parse --verify HEAD) && + test "$hash3" = "$HASH3" +' + # # test_done From 32ae83194b0f287a9b6644cdad175c56417c31f3 Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:37 +0200 Subject: [PATCH 29/35] add a test for git-send-email for non-threaded mails Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- t/t9001-send-email.sh | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index ce26ea4ac5..5bfa36eccf 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -621,4 +621,14 @@ test_expect_success 'in-reply-to but no threading' ' grep "In-Reply-To: " ' +test_expect_failure 'no in-reply-to and no threading' ' + git send-email \ + --dry-run \ + --from="Example " \ + --to=nobody@example.com \ + --nothread \ + $patches $patches >stdout && + ! grep "In-Reply-To: " stdout +' + test_done From 5e9758e2968238906c730c9c77ecc95c21e7495e Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:38 +0200 Subject: [PATCH 30/35] send-email: fix non-threaded mails After commit 3e0c4ff (send-email: respect in-reply-to regardless of threading, 2009-03-01) the variable $thread was only used for prompting for an "In-Reply-To", but not for controlling whether the "In-Reply-To" and "References" fields should be written into the email. Thus these fields were always used beginning with the second mail and it was not possible to produce non-threaded mails anymore. However, a later commit 15da108 ("send-email: 'References:' should only reference what is sent", 2009-04-13) introduced a regression with the side effect to make non-threaded mails possible again, but only when --no-chain-reply-to was used. Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- git-send-email.perl | 3 ++- t/t9001-send-email.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index cccbf4517a..5d5169707b 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1137,7 +1137,8 @@ foreach my $t (@files) { send_message(); # set up for the next message - if ($chain_reply_to || !defined $reply_to || length($reply_to) == 0) { + if ($thread && + ($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) { $reply_to = $message_id; if (length $references > 0) { $references .= "\n $message_id"; diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 5bfa36eccf..8518acaca6 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' ' grep "In-Reply-To: " ' -test_expect_failure 'no in-reply-to and no threading' ' +test_expect_success 'no in-reply-to and no threading' ' git send-email \ --dry-run \ --from="Example " \ From 0fd41f2d66fdb7c8125f341c72f075aca045017f Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:39 +0200 Subject: [PATCH 31/35] doc/send-email: clarify the behavior of --in-reply-to with --no-thread Also remove the argument from --[no-]chain-reply-to. Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- Documentation/git-send-email.txt | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/Documentation/git-send-email.txt b/Documentation/git-send-email.txt index a2821907c7..0ec53431a5 100644 --- a/Documentation/git-send-email.txt +++ b/Documentation/git-send-email.txt @@ -159,7 +159,7 @@ Automating Output of this command must be single email address per line. Default is the value of 'sendemail.cccmd' configuration value. ---[no-]chain-reply-to=:: +--[no-]chain-reply-to:: If this is set, each email will be sent as a reply to the previous email sent. If disabled with "--no-chain-reply-to", all emails after the first will be sent as replies to the first email sent. When using @@ -208,7 +208,8 @@ specified, as well as 'body' if --no-signed-off-cc is specified. --[no-]thread:: If this is set, the In-Reply-To header will be set on each email sent. If disabled with "--no-thread", no emails will have the In-Reply-To - header set. Default is the value of the 'sendemail.thread' configuration + header set, unless specified with --in-reply-to. + Default is the value of the 'sendemail.thread' configuration value; if that is unspecified, default to --thread. From d67114a5f3cbbedd4f01e6ff87fd5d4db9563ead Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:40 +0200 Subject: [PATCH 32/35] add a test for git-send-email for threaded mails without chain-reply-to Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- t/t9001-send-email.sh | 11 +++++++++++ 1 file changed, 11 insertions(+) diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 2ce24cd5a6..4f67de3ac7 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -621,4 +621,15 @@ test_expect_success 'in-reply-to but no threading' ' grep "In-Reply-To: " ' +test_expect_failure 'threading but no chain-reply-to' ' + git send-email \ + --dry-run \ + --from="Example " \ + --to=nobody@example.com \ + --thread \ + --nochain-reply-to \ + $patches $patches >stdout && + grep "In-Reply-To: " stdout +' + test_done From f74fe34b96816bad1f568202ec51ef18ae7513b3 Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:41 +0200 Subject: [PATCH 33/35] send-email: fix threaded mails without chain-reply-to An earlier commit 15da108 ("send-email: 'References:' should only reference what is sent", 2009-04-13) broke logic to set up threading information for the next message by rewriting "!" to "not" without understanding the precedence rules of the language. Namely, ! defined $reply_to || length($reply_to) == 0 was changed to not defined $reply_to || length($reply_to) == 0 which is not (defined $reply_to || length($reply_to) == 0) and different from what was intended, which is (not defined $reply_to) || (length($reply_to) == 0) Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- git-send-email.perl | 3 ++- t/t9001-send-email.sh | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/git-send-email.perl b/git-send-email.perl index 4c795a4b03..16d12e082b 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -1150,7 +1150,8 @@ foreach my $t (@files) { my $message_was_sent = send_message(); # set up for the next message - if ($message_was_sent and $chain_reply_to || not defined $reply_to || length($reply_to) == 0) { + if ($message_was_sent && + ($chain_reply_to || !defined $reply_to || length($reply_to) == 0)) { $reply_to = $message_id; if (length $references > 0) { $references .= "\n $message_id"; diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh index 4f67de3ac7..8ab1a78bf5 100755 --- a/t/t9001-send-email.sh +++ b/t/t9001-send-email.sh @@ -621,7 +621,7 @@ test_expect_success 'in-reply-to but no threading' ' grep "In-Reply-To: " ' -test_expect_failure 'threading but no chain-reply-to' ' +test_expect_success 'threading but no chain-reply-to' ' git send-email \ --dry-run \ --from="Example " \ From a1b5b371994beb044053da22ec4a9607630a83a2 Mon Sep 17 00:00:00 2001 From: Markus Heidelberg Date: Fri, 12 Jun 2009 12:51:42 +0200 Subject: [PATCH 34/35] send-email: fix a typo in a comment Signed-off-by: Markus Heidelberg Signed-off-by: Junio C Hamano --- git-send-email.perl | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/git-send-email.perl b/git-send-email.perl index 16d12e082b..4a77d445cd 100755 --- a/git-send-email.perl +++ b/git-send-email.perl @@ -812,7 +812,7 @@ sub sanitize_address } # Returns 1 if the message was sent, and 0 otherwise. -# In actuality, the whole program dies when a there +# In actuality, the whole program dies when there # is an error sending a message. sub send_message From c97038d1cfbd98ea258086c417fe4f0c094596d3 Mon Sep 17 00:00:00 2001 From: Stephen Boyd Date: Sat, 13 Jun 2009 11:20:00 -0700 Subject: [PATCH 35/35] git-rerere.txt: grammatical fixups and cleanups Rewrite the gc section using unresolved and resolved instead of "not recorded". Add plurals and missing articles. Make some sentences have consistent tense. Try and be more active by removing "that" and simplifying sentences. The terms "hand-resolve" and "hand resolve" were used, so just use "hand resolve" to be more consistent. Signed-off-by: Stephen Boyd Signed-off-by: Junio C Hamano --- Documentation/git-rerere.txt | 65 ++++++++++++++++++------------------ 1 file changed, 32 insertions(+), 33 deletions(-) diff --git a/Documentation/git-rerere.txt b/Documentation/git-rerere.txt index 64715c17da..a53c3cd35b 100644 --- a/Documentation/git-rerere.txt +++ b/Documentation/git-rerere.txt @@ -12,15 +12,15 @@ SYNOPSIS DESCRIPTION ----------- -In a workflow that employs relatively long lived topic branches, -the developer sometimes needs to resolve the same conflict over +In a workflow employing relatively long lived topic branches, +the developer sometimes needs to resolve the same conflicts over and over again until the topic branches are done (either merged to the "release" branch, or sent out and accepted upstream). -This command helps this process by recording conflicted -automerge results and corresponding hand-resolve results on the -initial manual merge, and later by noticing the same automerge -results and applying the previously recorded hand resolution. +This command assists the developer in this process by recording +conflicted automerge results and corresponding hand resolve results +on the initial manual merge, and applying previously recorded +hand resolutions to their corresponding automerge results. [NOTE] You need to set the configuration variable rerere.enabled to @@ -54,18 +54,18 @@ for resolutions. 'gc':: -This command is used to prune records of conflicted merge that -occurred long time ago. By default, conflicts older than 15 -days that you have not recorded their resolution, and conflicts -older than 60 days, are pruned. These are controlled with +This prunes records of conflicted merges that +occurred a long time ago. By default, unresolved conflicts older +than 15 days and resolved conflicts older than 60 +days are pruned. These defaults are controlled via the `gc.rerereunresolved` and `gc.rerereresolved` configuration -variables. +variables respectively. DISCUSSION ---------- -When your topic branch modifies overlapping area that your +When your topic branch modifies an overlapping area that your master branch (or upstream) touched since your topic branch forked from it, you may want to test it with the latest master, even before your topic branch is ready to be pushed upstream: @@ -140,9 +140,9 @@ top of the tip before the test merge: This would leave only one merge commit when your topic branch is finally ready and merged into the master branch. This merge would require you to resolve the conflict, introduced by the -commits marked with `*`. However, often this conflict is the +commits marked with `*`. However, this conflict is often the same conflict you resolved when you created the test merge you -blew away. 'git-rerere' command helps you to resolve this final +blew away. 'git-rerere' helps you resolve this final conflicted merge using the information from your earlier hand resolve. @@ -150,33 +150,32 @@ Running the 'git-rerere' command immediately after a conflicted automerge records the conflicted working tree files, with the usual conflict markers `<<<<<<<`, `=======`, and `>>>>>>>` in them. Later, after you are done resolving the conflicts, -running 'git-rerere' again records the resolved state of these +running 'git-rerere' again will record the resolved state of these files. Suppose you did this when you created the test merge of master into the topic branch. -Next time, running 'git-rerere' after seeing a conflicted -automerge, if the conflict is the same as the earlier one -recorded, it is noticed and a three-way merge between the +Next time, after seeing the same conflicted automerge, +running 'git-rerere' will perform a three-way merge between the earlier conflicted automerge, the earlier manual resolution, and -the current conflicted automerge is performed by the command. +the current conflicted automerge. If this three-way merge resolves cleanly, the result is written -out to your working tree file, so you would not have to manually +out to your working tree file, so you do not have to manually resolve it. Note that 'git-rerere' leaves the index file alone, so you still need to do the final sanity checks with `git diff` (or `git diff -c`) and 'git-add' when you are satisfied. As a convenience measure, 'git-merge' automatically invokes -'git-rerere' when it exits with a failed automerge, which -records it if it is a new conflict, or reuses the earlier hand +'git-rerere' upon exiting with a failed automerge and 'git-rerere' +records the hand resolve when it is a new conflict, or reuses the earlier hand resolve when it is not. 'git-commit' also invokes 'git-rerere' -when recording a merge result. What this means is that you do -not have to do anything special yourself (Note: you still have -to set the config variable rerere.enabled to enable this command). +when committing a merge result. What this means is that you do +not have to do anything special yourself (besides enabling +the rerere.enabled config variable). -In our example, when you did the test merge, the manual +In our example, when you do the test merge, the manual resolution is recorded, and it will be reused when you do the -actual merge later with updated master and topic branch, as long -as the earlier resolution is still applicable. +actual merge later with the updated master and topic branch, as long +as the recorded resolution is still applicable. The information 'git-rerere' records is also used when running 'git-rebase'. After blowing away the test merge and continuing @@ -194,11 +193,11 @@ development on the topic branch: o---o---o---*---o---o---o---o master ------------ -you could run `git rebase master topic`, to keep yourself -up-to-date even before your topic is ready to be sent upstream. -This would result in falling back to three-way merge, and it -would conflict the same way the test merge you resolved earlier. -'git-rerere' is run by 'git-rebase' to help you resolve this +you could run `git rebase master topic`, to bring yourself +up-to-date before your topic is ready to be sent upstream. +This would result in falling back to a three-way merge, and it +would conflict the same way as the test merge you resolved earlier. +'git-rerere' will be run by 'git-rebase' to help you resolve this conflict.