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

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

[Sasha Khapyorsky]

This adds trivial support for cloning and fetching via ftp://.

Signed-off-by: Sasha Khapyorsky <sashak@voltaire.com>
---
 git-clone.sh     |    2 +-
 git-fetch.sh     |    4 ++--
 git-ls-remote.sh |    2 +-
 3 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/git-clone.sh b/git-clone.sh
index 7060bda..e1b3bf3 100755
--- a/git-clone.sh
+++ b/git-clone.sh
@@ -298,7 +298,7 @@ yes,yes)
 		fi
 		git-ls-remote "$repo" >"$GIT_DIR/CLONE_HEAD" || exit 1
 		;;
-	https://*|http://*)
+	https://*|http://*|ftp://*)
 		if test -z "@@NO_CURL@@"
 		then
 			clone_dumb_http "$repo" "$D"
diff --git a/git-fetch.sh b/git-fetch.sh
index c2eebee..09a5d6c 100755
--- a/git-fetch.sh
+++ b/git-fetch.sh
@@ -286,7 +286,7 @@ fetch_main () {
 
       # There are transports that can fetch only one head at a time...
       case "$remote" in
-      http://* | https://*)
+      http://* | https://* | ftp://*)
 	  if [ -n "$GIT_SSL_NO_VERIFY" ]; then
 	      curl_extra_args="-k"
 	  fi
@@ -350,7 +350,7 @@ fetch_main () {
   done
 
   case "$remote" in
-  http://* | https://* | rsync://* )
+  http://* | https://* | ftp://* | rsync://* )
       ;; # we are already done.
   *)
     ( : subshell because we muck with IFS
diff --git a/git-ls-remote.sh b/git-ls-remote.sh
index 2fdcaf7..2c0b521 100755
--- a/git-ls-remote.sh
+++ b/git-ls-remote.sh
@@ -49,7 +49,7 @@ trap "rm -fr $tmp-*" 0 1 2 3 15
 tmpdir=$tmp-d
 
 case "$peek_repo" in
-http://* | https://* )
+http://* | https://* | ftp://* )
         if [ -n "$GIT_SSL_NO_VERIFY" ]; then
             curl_extra_args="-k"
         fi
-- 
1.4.2.gffe87-dirty

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

[Junio C Hamano]

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.

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

[Sasha Khapyorsky]

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)",

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

[Junio C Hamano]

Sasha Khapyorsky <sashak@voltaire.com> writes:

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

I think without you would have, just with extra error messages
that http codepath filters out.

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

Here you do the same as the code would for HTTP 404 when you get
550 _and_ RETR failure...

> @@ -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;

... but you say, while the original code says "declare error if
it is not HTTP 404", "oh by the way, if it is 550 _or_ if it
is RETR failure then do not trigger this if()".  I suspect you
meant to say this?

	    (slot->http_code != 550 ||
	     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {

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

[Sasha Khapyorsky]

On 02:12 Sat 16 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> > Something like this?
> >
> > With this change I'm able to clone
> > ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
> 
> I think without you would have, just with extra error messages
> that http codepath filters out.

No, not really, without change it fails later:

$ git-clone ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
error: RETR response: 550 (curl_result = 19, http_code = 550, sha1 = 63b98080daa35f0d682db04f4fb7ada010888752)
Getting pack list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Getting alternates list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Also look at ftp://ftp.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
Getting pack list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
Getting index for pack 477061883bee3d10bece6e3432355b61ba02e594
error: Unable to find 63b98080daa35f0d682db04f4fb7ada010888752 under ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
Cannot obtain needed none 63b98080daa35f0d682db04f4fb7ada010888752
while processing commit 0000000000000000000000000000000000000000.

> 
> > 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)) {
> 
> Here you do the same as the code would for HTTP 404 when you get
> 550 _and_ RETR failure...
> 
> > @@ -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;
> 
> ... but you say, while the original code says "declare error if
> it is not HTTP 404", "oh by the way, if it is 550 _or_ if it
> is RETR failure then do not trigger this if()".  I suspect you
> meant to say this?
> 
> 	    (slot->http_code != 550 ||
> 	     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {

I think with less strict checking this could be done so, but with _and_
this also ensures that we are really in FTP mode.

Sasha

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

[Sasha Khapyorsky]

On 13:01 Sat 16 Sep     , Sasha Khapyorsky wrote:
> On 02:12 Sat 16 Sep     , Junio C Hamano wrote:
> > Sasha Khapyorsky <sashak@voltaire.com> writes:
> > 
> > > Something like this?
> > >
> > > With this change I'm able to clone
> > > ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
> > 
> > I think without you would have, just with extra error messages
> > that http codepath filters out.
> 
> No, not really, without change it fails later:
> 
> $ git-clone ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git
> error: RETR response: 550 (curl_result = 19, http_code = 550, sha1 = 63b98080daa35f0d682db04f4fb7ada010888752)
> Getting pack list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> Getting alternates list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> Also look at ftp://ftp.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
> Getting pack list for ftp://ftp.kernel.org/pub/scm/linux/kernel/git/torvalds/linux-2.6.git/
> Getting index for pack 477061883bee3d10bece6e3432355b61ba02e594
> error: Unable to find 63b98080daa35f0d682db04f4fb7ada010888752 under ftp://ftp.kernel.org/pub/scm/linux/kernel/git/paulus/powerpc.git/
> Cannot obtain needed none 63b98080daa35f0d682db04f4fb7ada010888752
> while processing commit 0000000000000000000000000000000000000000.
> 
> > 
> > > 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)) {
> > 
> > Here you do the same as the code would for HTTP 404 when you get
> > 550 _and_ RETR failure...
> > 
> > > @@ -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;
> > 
> > ... but you say, while the original code says "declare error if
> > it is not HTTP 404", "oh by the way, if it is 550 _or_ if it
> > is RETR failure then do not trigger this if()".  I suspect you
> > meant to say this?
> > 
> > 	    (slot->http_code != 550 ||
> > 	     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {
> 
> I think with less strict checking this could be done so, but with _and_
> this also ensures that we are really in FTP mode.

Hmm, saying this I see that original code doesn't do it for specific
case. So for this case we could do:

 	    !(slot->http_code == 550 &&
 	     slot->curl_result == CURLE_FTP_COULDNT_RETR_FILE)) {

Sasha

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

[Junio C Hamano]

Sasha Khapyorsky <sashak@voltaire.com> writes:

>> > 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)) {
>> 
>> Here you do the same as the code would for HTTP 404 when you get
>> 550 _and_ RETR failure...
>> 
>> > @@ -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;
>> 
>> ... but you say, while the original code says "declare error if
>> it is not HTTP 404", "oh by the way, if it is 550 _or_ if it
>> is RETR failure then do not trigger this if()".  I suspect you
>> meant to say this?
>> 
>> 	    (slot->http_code != 550 ||
>> 	     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {
>
> I think with less strict checking this could be done so, but with _and_
> this also ensures that we are really in FTP mode.

I was merely pointing out that in one place you have:

	(http_code == 550 && result == ERETR)

and another place that tries to say the opposite you have:

	(http_code != 550 && result != ERETR)

which is not the same thing as

	!(http_code == 550 && result == ERETR)

I understood, from the former "Use alternates if necessary"
part, that you wanted to make sure that 550 is really from
FTP_RETR and not other random HTTP error message, and I think
that is a reasonable thing to do.

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

[Sasha Khapyorsky]

On 10:29 Sat 16 Sep     , Junio C Hamano wrote:
> Sasha Khapyorsky <sashak@voltaire.com> writes:
> 
> >> > 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)) {
> >> 
> >> Here you do the same as the code would for HTTP 404 when you get
> >> 550 _and_ RETR failure...
> >> 
> >> > @@ -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;
> >> 
> >> ... but you say, while the original code says "declare error if
> >> it is not HTTP 404", "oh by the way, if it is 550 _or_ if it
> >> is RETR failure then do not trigger this if()".  I suspect you
> >> meant to say this?
> >> 
> >> 	    (slot->http_code != 550 ||
> >> 	     slot->curl_result != CURLE_FTP_COULDNT_RETR_FILE)) {
> >
> > I think with less strict checking this could be done so, but with _and_
> > this also ensures that we are really in FTP mode.
> 
> I was merely pointing out that in one place you have:
> 
> 	(http_code == 550 && result == ERETR)
> 
> and another place that tries to say the opposite you have:
> 
> 	(http_code != 550 && result != ERETR)
> 
> which is not the same thing as
> 
> 	!(http_code == 550 && result == ERETR)
> 
> I understood, from the former "Use alternates if necessary"
> part, that you wanted to make sure that 550 is really from
> FTP_RETR and not other random HTTP error message, and I think
> that is a reasonable thing to do.

Good. Am I need to send the patch or you will integrate it?

Sasha

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

[Junio C Hamano]

Sasha Khapyorsky <sashak@voltaire.com> writes:

> Good. Am I need to send the patch or you will integrate it?

Actually, I am thinking of doing this in two steps.

The attached is the first "clean-up" step, which should be
obvious enough.

And you already know what the second one that would come on top
of this should look like ;-).

-- >8 --
http-fetch.c: consolidate code to detect missing fetch target

At a handful places we check two error codes from curl library
to see if the file we asked was missing from the remote (e.g.
we asked for a loose object when it is in a pack) to decide what
to do next.  This consolidates the check into a single function.

Signed-off-by: Junio C Hamano <junkio@cox.net>
---
diff --git a/http-fetch.c b/http-fetch.c
index a113bb8..bc74f30 100644
--- a/http-fetch.c
+++ b/http-fetch.c
@@ -144,6 +144,19 @@ static size_t fwrite_sha1_file(void *ptr
 	return size;
 }
 
+static int missing__target(int code, int result)
+{
+	return	/* file:// URL -- do we ever use one??? */
+		(result == CURLE_FILE_COULDNT_READ_FILE) ||
+		/* http:// and https:// URL */
+		(code == 404 && result == CURLE_HTTP_RETURNED_ERROR) ||
+		/* ftp:// URL */
+		(code == 550 && result == CURLE_FTP_COULDNT_RETR_FILE)
+		;
+}
+
+#define missing_target(a) missing__target((a)->http_code, (a)->curl_result)
+
 static void fetch_alternates(const char *base);
 
 static void process_object_response(void *callback_data);
@@ -323,8 +336,7 @@ static void process_object_response(void
 	obj_req->state = COMPLETE;
 
 	/* Use alternates if necessary */
-	if (obj_req->http_code == 404 ||
-	    obj_req->curl_result == CURLE_FILE_COULDNT_READ_FILE) {
+	if (missing_target(obj_req)) {
 		fetch_alternates(alt->base);
 		if (obj_req->repo->next != NULL) {
 			obj_req->repo =
@@ -537,8 +549,7 @@ static void process_alternates_response(
 			return;
 		}
 	} else if (slot->curl_result != CURLE_OK) {
-		if (slot->http_code != 404 &&
-		    slot->curl_result != CURLE_FILE_COULDNT_READ_FILE) {
+		if (!missing_target(slot)) {
 			got_alternates = -1;
 			return;
 		}
@@ -941,8 +952,7 @@ #endif
 	if (start_active_slot(slot)) {
 		run_active_slot(slot);
 		if (results.curl_result != CURLE_OK) {
-			if (results.http_code == 404 ||
-			    results.curl_result == CURLE_FILE_COULDNT_READ_FILE) {
+			if (missing_target(&results)) {
 				repo->got_indices = 1;
 				free(buffer.buffer);
 				return 0;
@@ -1123,8 +1133,7 @@ #endif
 		ret = error("Request for %s aborted", hex);
 	} 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)
+		if (missing_target(obj_req))
 			ret = -1; /* Be silent, it is probably in a pack. */
 		else
 			ret = error("%s (curl_result = %d, http_code = %ld, sha1 = %s)",

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

[Junio C Hamano]

Junio C Hamano <junkio@cox.net> writes:

> Sasha Khapyorsky <sashak@voltaire.com> writes:
>
>> Good. Am I need to send the patch or you will integrate it?
>
> Actually, I am thinking of doing this in two steps.
>
> The attached is the first "clean-up" step, which should be
> obvious enough.
>
> And you already know what the second one that would come on top
> of this should look like ;-).

Oops, thinko.  I sent a rolled-up one out.

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

[Sasha Khapyorsky]

On 11:00 Sat 16 Sep     , Junio C Hamano wrote:
> Junio C Hamano <junkio@cox.net> writes:
> 
> > Sasha Khapyorsky <sashak@voltaire.com> writes:
> >
> >> Good. Am I need to send the patch or you will integrate it?
> >
> > Actually, I am thinking of doing this in two steps.
> >
> > The attached is the first "clean-up" step, which should be
> > obvious enough.
> >
> > And you already know what the second one that would come on top
> > of this should look like ;-).
> 
> Oops, thinko.  I sent a rolled-up one out.

Yes, and it looks good. Thanks :)

Sasha