[PATCH] Fail properly when cloning from invalid HTTP URL

classic Classic list List threaded Threaded
3 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Fail properly when cloning from invalid HTTP URL

Petr Baudis
From: Petr Baudis <[hidden email]>

Currently, when cloning from invalid HTTP URL, git clone will possibly
return curl error, then a confusing message about remote HEAD and then
return success and leave an empty repository behind, confusing either
the end-user or the automated service calling it (think repo.or.cz).

This patch changes the error() calls in get_refs_via_curl() to die()s,
akin to the other get_refs_*() functions.

Cc: Daniel Barkalow <[hidden email]>
Signed-off-by: Petr Baudis <[hidden email]>
---
 transport.c |    7 +++----
 1 files changed, 3 insertions(+), 4 deletions(-)

diff --git a/transport.c b/transport.c
index 6eb65b8..b88b89b 100644
--- a/transport.c
+++ b/transport.c
@@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)
  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);
  return NULL;
  } else {
- error("%s", curl_errorstr);
- return NULL;
+ die("%s download error - %s", refs_url, curl_errorstr);
  }
  }
  } else {
  strbuf_release(&buffer);
- error("Unable to start request");
- return NULL;
+ die("Unable to start HTTP request");
  }
 
  data = buffer.buf;
--
1.5.6.3.392.g292f1

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fail properly when cloning from invalid HTTP URL

Junio C Hamano
[hidden email] writes:

> diff --git a/transport.c b/transport.c
> index 6eb65b8..b88b89b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)
>   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);
>   return NULL;

Hmm, I do not see a deletion of a line here...

Everybody else in the get_refs_via_ family just die()s, and the caller of
them do not notice nor treat NULL as anything special; I think your patch
is a sensible thing to do.

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Fail properly when cloning from invalid HTTP URL

Daniel Barkalow
In reply to this post by Petr Baudis
On Thu, 7 Aug 2008, [hidden email] wrote:

> From: Petr Baudis <[hidden email]>
>
> Currently, when cloning from invalid HTTP URL, git clone will possibly
> return curl error, then a confusing message about remote HEAD and then
> return success and leave an empty repository behind, confusing either
> the end-user or the automated service calling it (think repo.or.cz).
>
> This patch changes the error() calls in get_refs_via_curl() to die()s,
> akin to the other get_refs_*() functions.
>
> Cc: Daniel Barkalow <[hidden email]>
> Signed-off-by: Petr Baudis <[hidden email]>
> ---
>  transport.c |    7 +++----
>  1 files changed, 3 insertions(+), 4 deletions(-)
>
> diff --git a/transport.c b/transport.c
> index 6eb65b8..b88b89b 100644
> --- a/transport.c
> +++ b/transport.c
> @@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)
>   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);
>   return NULL;

The "return NULL" isn't necessary any more. Otherwise,

Acked-by: Daniel Barkalow <[hidden email]>

>   } else {
> - error("%s", curl_errorstr);
> - return NULL;
> + die("%s download error - %s", refs_url, curl_errorstr);
>   }
>   }
>   } else {
>   strbuf_release(&buffer);
> - error("Unable to start request");
> - return NULL;
> + die("Unable to start HTTP request");
>   }
>  
>   data = buffer.buf;
> --
> 1.5.6.3.392.g292f1
>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to [hidden email]
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to [hidden email]
More majordomo info at  http://vger.kernel.org/majordomo-info.html