Re: [PATCH] Trivial support for cloning and fetching via ftp://.

[Sasha Khapyorsky <sashak@voltaire.com>]

On 23:57 Wed 13 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> > This adds trivial support for cloning and fetching via ftp://.
> 
> Interesting.
> 
> I was wondering myself if our use of curl libraries in
> http-fetch allows us to do this when I was looking at the
> alternates breakage yesterday.
> 
> At a few places we do look at http error code that is returned
> from the curl library, and change our behaviour based on that.
> But it appears the difference between error code from ftp and
> http has no bad effect on us.  In an empty repository, we can
> run this:
> 
> 	$ git-http-fetch -a -v heads/merge \
> 	  ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
> 
> (of course, this should normally be with http://www.kernel.org).
> We notice that we get an error from a request for one object,
> and switch to pack & alternates transfer.  The only difference
> between http://www and ftp://ftp is that for the former we know
> error code 404 and supress the error message but for the latter
> we do not treat error 550 from RETR response any specially and
> show an error message.  We still fall back to retrieve packs,
> hoping that the missing object is in a pack.
> 
> I'd take this patch as is, but we might want to add some error
> message supression logic just like we do for http.

Something like this?

With this change I'm able to clone
ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git


diff --git a/http-fetch.c b/http-fetch.c
index a113bb8..46d6029 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -324,7 +324,9 @@ static void process_object_response(void
 
 	/* Use alternates if necessary */
 	if (obj_req->http_code == 404 ||
-	    obj_req->curl_result == CURLE_FILE_COULDNT_READ_FILE) {
+	    obj_req->curl_result == CURLE_FILE_COULDNT_READ_FILE ||
+	    (obj_req->http_code == 550 &&
+	     obj_req->curl_result == CURLE_FTP_COULDNT_RETR_FILE)) {
 		fetch_alternates(alt->base);
 		if (obj_req->repo->next != NULL) {
 			obj_req->repo =
@@ -538,7 +540,9 @@ static void process_alternates_response(
 		}
 	} else if (slot->curl_result != CURLE_OK) {
 		if (slot->http_code != 404 &&
-		    slot->curl_result != CURLE_FILE_COULDNT_READ_FILE) {
+		    slot->curl_result != CURLE_FILE_COULDNT_READ_FILE &&
+		    (slot->http_code != 550 &&
+		     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {
 			got_alternates = -1;
 			return;
 		}
@@ -942,7 +946,9 @@ #endif
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
 			if (results.http_code == 404 ||
-			    results.curl_result == CURLE_FILE_COULDNT_READ_FILE) {
+			    results.curl_result == CURLE_FILE_COULDNT_READ_FILE ||
+			    (results.http_code == 550 &&
+			     results.curl_result == CURLE_FTP_COULDNT_RETR_FILE)) {
 				repo->got_indices = 1;
 				free(buffer.buffer);
 				return 0;
@@ -1124,7 +1130,9 @@ #endif
 	} else if (obj_req->curl_result != CURLE_OK &&
 		   obj_req->http_code != 416) {
 		if (obj_req->http_code == 404 ||
-		    obj_req->curl_result == CURLE_FILE_COULDNT_READ_FILE)
+		    obj_req->curl_result == CURLE_FILE_COULDNT_READ_FILE ||
+		    (obj_req->http_code == 550 &&
+		     obj_req->curl_result == CURLE_FTP_COULDNT_RETR_FILE))
 			ret = -1; /* Be silent, it is probably in a pack. */
 		else
 			ret = error("%s (curl_result = %d, http_code = %ld, sha1 = %s)",