Don't fetch objects that exist in the local repository

PATCH In review Nick Hengeveld 2 participants 4 messages Jun 15, 2016

Share & tools

Nick Hengeveld

author

2016-06-15 22:42 UTC #0 lore raw

PATCH 1/1 Don't fetch objects that exist in the local repository

Be sure not to fetch objects that already exist in the local repository. The main process loop no longer performs this check, http-fetch now checks prior to starting a new request queue entry and when fetch_object() is called, and local-fetch now checks when fetch_object() is called.

As discussed in this thread: http://marc.theaimsgroup.com/?t=112854890500001

Signed-off-by Nick Hengeveld <[email protected]>

---

 fetch.c       |    2 +-
 http-fetch.c  |   10 +++++++++-
 local-fetch.c |    5 ++++-
 3 files changed, 14 insertions(+), 3 deletions(-)

41b78748fe458224fc1d621f5f0a4df2a3ac3253

fetch.c +1 -1

@@ -165,7 +165,7 @@ static int loop(void)
 		 * the queue because we needed to fetch it first.
 		 */
 		if (! (obj->flags & TO_SCAN)) {
-			if (!has_sha1_file(obj->sha1) && fetch(obj->sha1)) {
+			if (fetch(obj->sha1)) {
 				report_missing(obj->type
 					       ? obj->type
 					       : "object", obj->sha1);

http-fetch.c +9 -1

@@ -489,7 +489,10 @@ void process_request_queue()
 
 	while (active_requests < max_requests && request != NULL) {
 		if (request->state == WAITING) {
-			start_request(request);
+			if (has_sha1_file(request->sha1))
+				release_request(request);
+			else
+				start_request(request);
 			curl_multi_perform(curlm, &num_transfers);
 		}
 		request = request->next;

@@ -890,6 +893,11 @@ static int fetch_object(struct alt_base 
 	if (request == NULL)
 		return error("Couldn't find request for %s in the queue", hex);
 
+	if (has_sha1_file(request->sha1)) {
+		release_request(request);
+		return 0;
+	}
+
 #ifdef USE_CURL_MULTI
 	int num_transfers;
 	while (request->state == WAITING) {

local-fetch.c +4 -1

@@ -166,7 +166,10 @@ static int fetch_file(const unsigned cha
 
 int fetch(unsigned char *sha1)
 {
-	return fetch_file(sha1) && fetch_pack(sha1);
+	if (has_sha1_file(sha1))
+		return 0;
+	else
+		return fetch_file(sha1) && fetch_pack(sha1);
 }
 
 int fetch_ref(char *ref, unsigned char *sha1)

Johannes Schindelin

2016-06-15 22:42 UTC #1 lore raw

Hi,

Quoted 1 line · Nick Hengeveld

Be sure not to fetch objects that already exist in the local repository.

Really? I seem to recall a dispute I had with Linus that this is unacceptable. He seems worried about incomplete fetches.

Only objects which are descendants of refs are safe, since the refs are written only after the objects were fetched successfully.

Ciao,
Dscho

Nick Hengeveld

author

2016-06-15 22:42 UTC #2 lore raw

Quoted 4 lines · Johannes Schindelin

Quoted 1 line
Be sure not to fetch objects that already exist in the local repository.
Really? I seem to recall a dispute I had with Linus that this is unacceptable. He seems worried about incomplete fetches.

This patch just moved an existing check from the process queue loop to the appropriate places in transport-specific code to prevent the transport from transferring an object that appeared in the local repository after it was prefetched (eg. via a pack), and to make sure that all objects that were prefetched are subsequently fetched so the transport can perform the appropriate cleanup.

Signature · 2 lines

For a successful technology, reality must take precedence over public
relations, for nature cannot be fooled.

Johannes Schindelin

2016-06-15 22:42 UTC #3 lore raw

Hi,

Quoted 7 lines · Nick Hengeveld

Quoted 4 lines · Johannes Schindelin
Quoted 1 line
Be sure not to fetch objects that already exist in the local repository.
Really? I seem to recall a dispute I had with Linus that this is unacceptable. He seems worried about incomplete fetches.

Quoted 6 lines

This patch just moved an existing check from the process queue loop to the appropriate places in transport-specific code to prevent the transport from transferring an object that appeared in the local repository after it was prefetched (eg. via a pack), and to make sure that all objects that were prefetched are subsequently fetched so the transport can perform the appropriate cleanup.

Ah! Okay, now I understand.

Thanks,
Dscho