{"thread":{"id":"14868","subject":"[PATCH] Fail properly when cloning from invalid HTTP URL","startedAt":"2008-08-07T00:06:30Z","lastAt":"2008-08-07T14:10:35Z","messageCount":3,"participants":["pasky@suse.cz","Junio C Hamano","Daniel Barkalow"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"86381","messageId":"1218067590-31590-1-git-send-email-pasky@suse.cz","threadId":"14868","inReplyTo":null,"subject":"[PATCH] Fail properly when cloning from invalid HTTP URL","fromName":"","fromEmail":"pasky@suse.cz","sentAt":"2008-08-07T00:06:30Z","receivedAt":"2008-08-07T00:06:30Z","isPatch":true,"sender":{"key":"pasky@ucw.cz","avatar":"https://avatars.githubusercontent.com/u/18439?v=4"},"body":"From: Petr Baudis \n\nCurrently, when cloning from invalid HTTP URL, git clone will possibly\nreturn curl error, then a confusing message about remote HEAD and then\nreturn success and leave an empty repository behind, confusing either\nthe end-user or the automated service calling it (think repo.or.cz).\n\nThis patch changes the error() calls in get_refs_via_curl() to die()s,\nakin to the other get_refs_*() functions.\n\nCc: Daniel Barkalow \nSigned-off-by: Petr Baudis \n---\n transport.c | 7 +++----\n 1 files changed, 3 insertions(+), 4 deletions(-)\n\ndiff --git a/transport.c b/transport.c\nindex 6eb65b8..b88b89b 100644\n--- a/transport.c\n+++ b/transport.c\n@@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n \t\tif (results.curl_result != CURLE_OK) {\n \t\t\tstrbuf_release(&buffer);\n \t\t\tif (missing_target(&results)) {\n+\t\t\t\tdie(\"%s not found: did you run git update-server-info on the server?\", refs_url);\n \t\t\t\treturn NULL;\n \t\t\t} else {\n-\t\t\t\terror(\"%s\", curl_errorstr);\n-\t\t\t\treturn NULL;\n+\t\t\t\tdie(\"%s download error - %s\", refs_url, curl_errorstr);\n \t\t\t}\n \t\t}\n \t} else {\n \t\tstrbuf_release(&buffer);\n-\t\terror(\"Unable to start request\");\n-\t\treturn NULL;\n+\t\tdie(\"Unable to start HTTP request\");\n \t}\n \n \tdata = buffer.buf;\n-- \n1.5.6.3.392.g292f1\n"},{"id":"86384","messageId":"7v63qdn03a.fsf@gitster.siamese.dyndns.org","threadId":"14868","inReplyTo":"1218067590-31590-1-git-send-email-pasky@suse.cz","subject":"Re: [PATCH] Fail properly when cloning from invalid HTTP URL","fromName":"Junio C Hamano","fromEmail":"gitster@pobox.com","sentAt":"2008-08-07T00:24:41Z","receivedAt":"2008-08-07T00:24:41Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"pasky@suse.cz writes:\n\n> diff --git a/transport.c b/transport.c\n> index 6eb65b8..b88b89b 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> \t\tif (results.curl_result != CURLE_OK) {\n> \t\t\tstrbuf_release(&buffer);\n> \t\t\tif (missing_target(&results)) {\n> +\t\t\t\tdie(\"%s not found: did you run git update-server-info on the server?\", refs_url);\n> \t\t\t\treturn NULL;\n\nHmm, I do not see a deletion of a line here...\n\nEverybody else in the get_refs_via_ family just die()s, and the caller of\nthem do not notice nor treat NULL as anything special; I think your patch\nis a sensible thing to do.\n"},{"id":"86415","messageId":"alpine.LNX.1.00.0808071007440.19665@iabervon.org","threadId":"14868","inReplyTo":"1218067590-31590-1-git-send-email-pasky@suse.cz","subject":"Re: [PATCH] Fail properly when cloning from invalid HTTP URL","fromName":"Daniel Barkalow","fromEmail":"barkalow@iabervon.org","sentAt":"2008-08-07T14:10:35Z","receivedAt":"2008-08-07T14:10:35Z","isPatch":true,"sender":{"key":"barkalow@iabervon.org","avatar":"https://avatars.githubusercontent.com/u/55364219?v=4"},"body":"On Thu, 7 Aug 2008, pasky@suse.cz wrote:\n\n> From: Petr Baudis \n> \n> Currently, when cloning from invalid HTTP URL, git clone will possibly\n> return curl error, then a confusing message about remote HEAD and then\n> return success and leave an empty repository behind, confusing either\n> the end-user or the automated service calling it (think repo.or.cz).\n> \n> This patch changes the error() calls in get_refs_via_curl() to die()s,\n> akin to the other get_refs_*() functions.\n> \n> Cc: Daniel Barkalow \n> Signed-off-by: Petr Baudis \n> ---\n> transport.c | 7 +++----\n> 1 files changed, 3 insertions(+), 4 deletions(-)\n> \n> diff --git a/transport.c b/transport.c\n> index 6eb65b8..b88b89b 100644\n> --- a/transport.c\n> +++ b/transport.c\n> @@ -464,16 +464,15 @@ static struct ref *get_refs_via_curl(struct transport *transport)\n> \t\tif (results.curl_result != CURLE_OK) {\n> \t\t\tstrbuf_release(&buffer);\n> \t\t\tif (missing_target(&results)) {\n> +\t\t\t\tdie(\"%s not found: did you run git update-server-info on the server?\", refs_url);\n> \t\t\t\treturn NULL;\n\nThe \"return NULL\" isn't necessary any more. Otherwise,\n\nAcked-by: Daniel Barkalow \n\n> \t\t\t} else {\n> -\t\t\t\terror(\"%s\", curl_errorstr);\n> -\t\t\t\treturn NULL;\n> +\t\t\t\tdie(\"%s download error - %s\", refs_url, curl_errorstr);\n> \t\t\t}\n> \t\t}\n> \t} else {\n> \t\tstrbuf_release(&buffer);\n> -\t\terror(\"Unable to start request\");\n> -\t\treturn NULL;\n> +\t\tdie(\"Unable to start HTTP request\");\n> \t}\n> \n> \tdata = buffer.buf;\n> -- \n> 1.5.6.3.392.g292f1\n> \n> --\n> To unsubscribe from this list: send the line \"unsubscribe git\" in\n> the body of a message to majordomo@vger.kernel.org\n> More majordomo info at http://vger.kernel.org/majordomo-info.html\n> \n"}]}