Skip to contentSkip to conversation

t5540-http-push: test fetching of loose objects

PATCH New Tay Ray Chuan 1 participant 23 messages Jun 15, 2016 1 message not in the archive

lore
Share & tools
lore

A message that is not in the archive (sent off-list or not yet received): [email protected]

#1 lore raw

PATCH 1/23 t5540-http-push: test fetching of loose objects

---
 t/t5540-http-push.sh |   16 ++++++++++++++++
 1 files changed, 16 insertions(+), 0 deletions(-)
t/t5540-http-push.sh +16 -0
index 5fe479e..65a41db 100755
@@ -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 &&

--
1.6.3.1

#2 lore raw

PATCH 2/23 t5540-http-push: test fetching of packed objects

In reply to a message not in the archive
---
 t/t5540-http-push.sh |   20 ++++++++++++++++++++
 1 files changed, 20 insertions(+), 0 deletions(-)
t/t5540-http-push.sh +20 -0
index 65a41db..ad0f14b 100755
@@ -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 &&

--
1.6.3.1

#3 lore raw

PATCH 3/23 http*: cleanup slot->local after fclose

In reply to a message not in the archive

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

---
 http-push.c   |    6 ++++++
 http-walker.c |    6 ++++++
 2 files changed, 12 insertions(+), 0 deletions(-)
http-push.c +6 -0
index e16a0ad..0b12ffe 100644
@@ -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);
 }
http-walker.c +6 -0
index 7321ccc..9377851 100644
@@ -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)

--
1.6.3.1

#4 lore raw

PATCH 4/23 http-push: send out fetch requests on queue

In reply to a message not in the archive

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.

---
 http-push.c          |   37 ++++++++++++++++++++++++++-----------
 t/t5540-http-push.sh |    4 ++--
 2 files changed, 28 insertions(+), 13 deletions(-)
http-push.c +26 -11
index 0b12ffe..218e263 100644
@@ -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) {
@@ -2174,6 +2175,25 @@ static int delete_remote_branch(char *pattern, int force)
 	return 0;
 }
 
+void run_request_queue()
+{
+#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;
@@ -2278,6 +2298,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;
@@ -2307,6 +2329,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) {
@@ -2436,16 +2459,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))
t/t5540-http-push.sh +2 -2
index ad0f14b..f4a2cf6 100755
@@ -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 &&
 

--
1.6.3.1

#5 lore raw

PATCH 5/23 http-push: fix missing "#ifdef USE_CURL_MULTI" around "is_running_queue"

In reply to a message not in the archive

As it is breaking the build when USE_CURL_MULTI is not defined.

---
 http-push.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
http-push.c +2 -0
index 218e263..1d84f3b 100644
@@ -2298,7 +2298,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()) {

--
1.6.3.1

#6 lore raw

PATCH 6/23 t5550-http-fetch: test fetching of packed objects

In reply to a message not in the archive
---
 t/t5550-http-fetch.sh |    8 ++++++++
 1 files changed, 8 insertions(+), 0 deletions(-)
t/t5550-http-fetch.sh +8 -0
index 05b1b62..0e69324 100755
@@ -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

--
1.6.3.1

#7 lore raw

PATCH 7/23 http-push, http-walker: style fixes

In reply to a message not in the archive

- 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)
*/

---
 http-push.c   |   48 ++++++++++++++++++++-------------
 http-walker.c |   83 ++++++++++++++++++++++++++++++++++----------------------
 2 files changed, 79 insertions(+), 52 deletions(-)
http-push.c +29 -19
index 1d84f3b..6ac0d63 100644
@@ -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)
http-walker.c +50 -33

201 lines not on this page · raw message

--
1.6.3.1

#8 lore raw

PATCH 8/23 http-walker: verify remote packs

In reply to a message not in the archive

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.

---
 http-walker.c |   33 ++++++++++++++++++++++++++++++---
 1 files changed, 30 insertions(+), 3 deletions(-)
http-walker.c +30 -3
index 6ac1783..bbc3023 100644
@@ -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;
 

--
1.6.3.1

#9 lore raw

PATCH 9/23 http*: copy string returned by sha1_to_hex

In reply to a message not in the archive

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.

---
 http-push.c   |   47 ++++++++++++++++++++++++-----------------------
 http-walker.c |   45 +++++++++++++++++++++++----------------------
 2 files changed, 47 insertions(+), 45 deletions(-)
http-push.c +24 -23
index 6ac0d63..c8cb6e4 100644
@@ -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)
http-walker.c +23 -22

81 lines not on this page · raw message

--
1.6.3.1

#10 lore raw

PATCH 10/23 http-push: do not SEGV after fetching a bad pack idx file

In reply to a message not in the archive

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.

---
 http-push.c |    2 ++
 1 files changed, 2 insertions(+), 0 deletions(-)
http-push.c +2 -0
index c8cb6e4..a6ca2ac 100644
@@ -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;

--
1.6.3.1

#11 lore raw

PATCH 11/23 Don't expect verify_pack() callers to set pack_size

In reply to a message not in the archive

From: Mike Hommey <[email protected]>
Date: Sun, 18 Jan 2009 09:04:26 +0100

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).

---
 pack-check.c |    8 +++++---
 1 files changed, 5 insertions(+), 3 deletions(-)
pack-check.c +5 -3
index 90c33b1..166ca70 100644
@@ -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))

--
1.6.3.1

#12 lore raw

PATCH 12/23 transport.c::get_refs_via_curl(): do not leak refs_url

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

---

This is based on Mike's earlier patch:

Subject: [WIP Patch 02/12] Some cleanup in get_refs_via_curl()
Date: Sun, 18 Jan 2009 09:04:27 +0100

I've removed the http_cleanup() invocation, after Johannes reported that it breaks http clone. I've also renamed it, for clarity's sake.

 transport.c |    1 +
 1 files changed, 1 insertions(+), 0 deletions(-)
transport.c +1 -0
index efecb65..9edd5aa 100644
@@ -519,6 +519,7 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push)
 		free(ref);
 	}
 
+	free(refs_url);
 	return refs;
 }
 

--
1.6.3.1

#13 lore raw

PATCH 13/23 http*: move common variables and macros to http.[ch]

In reply to a message not in the archive

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).

---
 http-push.c   |    8 +-------
 http-walker.c |   18 +++++-------------
 http.c        |    9 +++++++++
 http.h        |    5 +++++
 4 files changed, 20 insertions(+), 20 deletions(-)
http-push.c +1 -7
index a6ca2ac..b01db75 100644
@@ -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;
@@ -2251,6 +2248,7 @@ int main(int argc, char **argv)
 			}
 			if (!strcmp(arg, "--verbose")) {
 				push_verbosely = 1;
+				http_is_verbose = 1;
 				continue;
 			}
 			if (!strcmp(arg, "-d")) {
@@ -2300,8 +2298,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);
@@ -2504,8 +2500,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;
http-walker.c +5 -13
index d6cc622..032a7be 100644
@@ -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);
http.c +9 -0
index 2e3d649..3ca60bb 100644
@@ -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;
http.h +5 -0
index 26abebe..1ef7dc1 100644
@@ -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];
 

--
1.6.3.1

#14 lore raw

PATCH 14/23 http: create function end_url_with_slash

In reply to a message not in the archive

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.

---
 http.c |   12 +++++++++---
 1 files changed, 9 insertions(+), 3 deletions(-)
http.c +9 -3
index 3ca60bb..75fce9e 100644
@@ -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))

--
1.6.3.1

#15 lore raw

PATCH 15/23 http.c: new functions for the http API

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

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).

---

This patch is based on Mike's earlier patch:

Subject: [WIP Patch 03/12] Two new functions for the http API
Date: Sun, 18 Jan 2009 09:04:28 +0100

I've added a new return status code for http_request, HTTP_START_FAILED, since some callers need to know when start_active_slot() fails.

http_request() will emit an error message when start_active_slot() fails; for other cases, no error message is emitted.

I've added http_error(). (see description above)

I've added the list of new functions to the commit message body, as suggested by Jakub.

 http.c |  104 ++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h |   30 ++++++++++++++++++
 2 files changed, 134 insertions(+), 0 deletions(-)
http.c +104 -0
index 75fce9e..df22180 100644
@@ -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;
http.h +30 -0
index 1ef7dc1..3d878d5 100644
@@ -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 */

--
1.6.3.1

#16 lore raw

PATCH 16/23 transport.c::get_refs_via_curl(): use the new http API

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

---

This is based on Mike's earlier patch:

Subject: [WIP Patch 05/12] Use the new http API in get_refs_via_curl()
Date: Sun, 18 Jan 2009 09:04:30 +0100

I added back the die()'s like in the original, which Mike had removed.

 transport.c |   35 +++++++++++++----------------------
 1 files changed, 13 insertions(+), 22 deletions(-)
transport.c +13 -22
index 9edd5aa..af75f7b 100644
@@ -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,8 @@ static struct ref *get_refs_via_curl(struct transport *transport, int for_push)
 		free(ref);
 	}
 
+cleanup:
+	strbuf_release(&buffer);
 	free(refs_url);
 	return refs;
 }

--
1.6.3.1

#17 lore raw

PATCH 17/23 http.c::http_fetch_ref(): use the new http API

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

The error message ("Unable to start request") has been removed, since the http API already prints it.

---

This is based on Mike's earlier patch:

Subject: [WIP Patch 04/12] Use the new http API in http_fetch_ref()
Date: Sun, 18 Jan 2009 09:04:29 +0100

 http.c |   33 ++++++++-------------------------
 1 files changed, 8 insertions(+), 25 deletions(-)
http.c +8 -25
index df22180..2a49372 100644
@@ -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);

--
1.6.3.1

#18 lore raw

PATCH 18/23 http-push.c::remote_exists(): use the new http API

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

---

This is based on Mike's earlier patch:

Subject: [WIP Patch 08/12] Use the new http API in update_remote_info_refs()
Date: Sun, 18 Jan 2009 09:04:33 +0100

 http-push.c |   31 ++++++++++++-------------------
 1 files changed, 12 insertions(+), 19 deletions(-)
http-push.c +12 -19
index b01db75..455e0aa 100644
@@ -2005,29 +2005,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;
 }

--
1.6.3.1

#19 lore raw

PATCH 19/23 http-push.c::fetch_symref(): use the new http API

In reply to a message not in the archive

From: Mike Hommey <[email protected]>

---

This is based on Mike's earlier patch:

Subject: [WIP Patch 09/12] Use the new http API in fetch_symref()
Date: Sun, 18 Jan 2009 09:04:34 +0100

 http-push.c |   20 +++-----------------
 1 files changed, 3 insertions(+), 17 deletions(-)
http-push.c +3 -17
index 455e0aa..1714e4e 100644
@@ -2029,27 +2029,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);

--
1.6.3.1

#20 lore raw

PATCH 20/23 http*: add http_get_info_packs

In reply to a message not in the archive

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.

---

Clemens, I've corrected the return values for http-push.c::fetch_indices(). I won't touch repo->has_info_packs yet, though, as I've already mentioned.

 http-push.c   |  179 +++----------------------------------------------------
 http-walker.c |  184 +++------------------------------------------------------
 http.c        |  164 ++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h        |    4 +
 4 files changed, 186 insertions(+), 345 deletions(-)
http-push.c +9 -170
index 1714e4e..d6f3b8e 100644
@@ -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)
http-walker.c +9 -175

205 lines not on this page · raw message

http.c +164 -0

167 lines not on this page · raw message

http.h +4 -0
index 3d878d5..180b1c2 100644
@@ -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 */

--
1.6.3.1

#21 lore raw

PATCH 21/23 http: use new http API in fetch_index()

In reply to a message not in the archive
---
 http.c |   81 ++++++---------------------------------------------------------
 1 files changed, 8 insertions(+), 73 deletions(-)
http.c +8 -73
index 96d83d5..0701a6f 100644
@@ -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;

--
1.6.3.1

#22 lore raw

PATCH 22/23 http*: add helper methods for fetching packs

In reply to a message not in the archive

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.

---
 http-push.c   |  110 +++++++++++++-------------------------------------------
 http-walker.c |   85 +++++++++----------------------------------
 http.c        |  106 ++++++++++++++++++++++++++++++++++++++++++++++++++++++
 http.h        |   17 +++++++++
 4 files changed, 167 insertions(+), 151 deletions(-)
http-push.c +26 -84
index d6f3b8e..71d9967 100644
@@ -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);
-
-			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;
+			preq = (struct http_pack_request *)request->userData;
+
+			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;
http-walker.c +18 -67

115 lines not on this page · raw message

http.c +106 -0

113 lines not on this page · raw message

http.h +17 -0
index 180b1c2..511c0c4 100644
@@ -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 */

--
1.6.3.1

#23 lore raw

PATCH 23/23 http*: add helper methods for fetching objects (loose)

In reply to a message not in the archive

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.

---
 http-push.c   |  213 +++------------------------------------------
 http-walker.c |  267 +++++++++------------------------------------------------
 http.c        |  250 +++++++++++++++++++++++++++++++++++++++++++++++++++++-
 http.h        |   37 +++++++-
 4 files changed, 336 insertions(+), 431 deletions(-)
http-push.c +15 -198
index 71d9967..0d83465 100644
@@ -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;
-
-		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);
-			}
-		}
+		obj_req = (struct http_object_request *)request->userData;
+
+		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;
http-walker.c +40 -227

388 lines not on this page · raw message

http.c +248 -2

271 lines not on this page · raw message

http.h +33 -4
index 511c0c4..4c4e99c 100644
@@ -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 */

--
1.6.3.1