{"thread":{"id":"43331","subject":"Re: [PATCH] clear error message for clone a gitweb URL","startedAt":"2006-11-08T07:25:01Z","lastAt":"2006-11-09T01:49:09Z","messageCount":3,"participants":["Junio C Hamano","Liu Yubao"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"298547","messageId":"4551864D.3010301@gmail.com","threadId":"43331","inReplyTo":null,"subject":"[PATCH] clear error message for clone a gitweb URL","fromName":"Liu Yubao","fromEmail":"yubao.liu@gmail.com","sentAt":"2006-11-08T07:25:01Z","receivedAt":"2006-11-08T07:25:01Z","isPatch":true,"sender":{"key":"yubao.liu@gmail.com","avatar":null},"body":"When clone a gitweb URL, git reports \"Can't lock ref\", it's not clear for users,\nthis patch adds clear error message for this case.\n\ndiff --git a/fetch.c b/fetch.c\nindex c426c04..40c5183 100644\n--- a/fetch.c\n+++ b/fetch.c\n@@ -266,6 +266,14 @@ int pull(int targets, char **target, con\n \t\tif (!write_ref || !write_ref[i])\n \t\t\tcontinue;\n \n+\t\tif (*write_ref[i] == '\\0') {\n+\t\t\tif (strncmp(write_ref_log_details, \"http\", 4) == 0)\n+\t\t\t\terror(\"Can't feed empty ref, seems you are fetching from a gitweb URL, \"\n+\t\t\t\t      \"check it in web browser for git URL.\");\n+\t\t\telse\n+\t\t\t\terror(\"Can't feed empty ref\");\n+\t\t\tgoto unlock_and_fail;\n+\t\t}\n \t\tlock[i] = lock_ref_sha1(write_ref[i], NULL);\n \t\tif (!lock[i]) {\n \t\t\terror(\"Can't lock ref %s\", write_ref[i]);\ndiff --git a/git-clone.sh b/git-clone.sh\nindex 3f006d1..c8274e0 100755\n--- a/git-clone.sh\n+++ b/git-clone.sh\n@@ -55,6 +55,10 @@ Perhaps git-update-server-info needs to\n \t\telse\n \t\t\ttname=$name\n \t\tfi\n+\t\tif [ -z \"$tname\" -o -z \"$name\" ]; then\n+\t\t\tdie \"Cannot feed empty ref or commit-id, seems you are fetching\n+from a gitweb URL, check it in web browser for git URL.\"\n+\t\tfi\n \t\tgit-http-fetch -v -a -w \"$tname\" \"$name\" \"$1/\" || exit 1\n \tdone <\"$clone_tmp/refs\"\n"},{"id":"294381","messageId":"7v7iy5sma9.fsf@assigned-by-dhcp.cox.net","threadId":"43331","inReplyTo":"4551864D.3010301@gmail.com","subject":"Re: [PATCH] clear error message for clone a gitweb URL","fromName":"Junio C Hamano","fromEmail":"junkio@cox.net","sentAt":"2006-11-08T20:42:22Z","receivedAt":"2006-11-08T20:42:22Z","isPatch":true,"sender":{"key":"gitster@pobox.com","avatar":"https://avatars.githubusercontent.com/u/54884?v=4"},"body":"Liu Yubao <yubao.liu@gmail.com> writes:\n\n> When clone a gitweb URL, git reports \"Can't lock ref\", it's not clear for users,\n> this patch adds clear error message for this case.\n>\n> diff --git a/fetch.c b/fetch.c\n> index c426c04..40c5183 100644\n> --- a/fetch.c\n> +++ b/fetch.c\n> @@ -266,6 +266,14 @@ int pull(int targets, char **target, con\n>  \t\tif (!write_ref || !write_ref[i])\n>  \t\t\tcontinue;\n>  \n> +\t\tif (*write_ref[i] == '\\0') {\n> +\t\t\tif (strncmp(write_ref_log_details, \"http\", 4) == 0)\n> +\t\t\t\terror(\"Can't feed empty ref, seems you are fetching from a gitweb URL, \"\n> +\t\t\t\t      \"check it in web browser for git URL.\");\n> +\t\t\telse\n> +\t\t\t\terror(\"Can't feed empty ref\");\n> +\t\t\tgoto unlock_and_fail;\n\nYou might have got that error by feeding an URL for gitweb, but\nI do not think the code, even with your additions, knows enough\nto tell that the user's mistake isn't other kinds of errors.\n\nI am afraid that it would cause the user to waste time going\nwild goose chase if you say \"seems you are...\".  The phrasing\nmakes it sound as if the tool _knows_ with some certainty that\nit is more plausible cause of the error than other kinds, while\nit certainly doesn't.\n\nI think the reason it does not notice the breakage much earlier\nis that git-clone does not notice that gitweb URL gives nonsense\nto requests to \"http://host/gitweb.cgi/$project/info/refs\", so\nyour patch to git-clone.sh is probably touching the right place,\nbut I still feel the wording is a bit too strong and definitive\nthan it should be.\n\nPerhaps...\n\ndiff --git a/git-clone.sh b/git-clone.sh\nindex 3f006d1..7ae69d9 100755\n--- a/git-clone.sh\n+++ b/git-clone.sh\n@@ -46,15 +46,18 @@ Perhaps git-update-server-info needs to\n \tdo\n \t\tname=`expr \"z$refname\" : 'zrefs/\\(.*\\)'` &&\n \t\tcase \"$name\" in\n-\t\t*^*)\tcontinue;;\n-\t\tesac\n+\t\t*^*)\tcontinue ;;\n+\t\t'')\tfalse ;;\n+\t\tesac &&\n \t\tif test -n \"$use_separate_remote\" &&\n \t\t   branch_name=`expr \"z$name\" : 'zheads/\\(.*\\)'`\n \t\tthen\n \t\t\ttname=\"remotes/$origin/$branch_name\"\n \t\telse\n \t\t\ttname=$name\n-\t\tfi\n+\t\tfi || {\n+\t\t\tdie \"info/refs has nonsense $sha1 $refname, are you pulling from the right repository URL?\"\n+\t\t}\n \t\tgit-http-fetch -v -a -w \"$tname\" \"$name\" \"$1/\" || exit 1\n \tdone <\"$clone_tmp/refs\"\n \trm -fr \"$clone_tmp\"\n\n\n"},{"id":"297323","messageId":"45528915.8090608@gmail.com","threadId":"43331","inReplyTo":"7v7iy5sma9.fsf@assigned-by-dhcp.cox.net","subject":"Re: [PATCH] clear error message for clone a gitweb URL","fromName":"Liu Yubao","fromEmail":"yubao.liu@gmail.com","sentAt":"2006-11-09T01:49:09Z","receivedAt":"2006-11-09T01:49:09Z","isPatch":true,"sender":{"key":"yubao.liu@gmail.com","avatar":null},"body":"Junio C Hamano wrote:\n> Liu Yubao <yubao.liu@gmail.com> writes:\n> \n>> When clone a gitweb URL, git reports \"Can't lock ref\", it's not clear for users,\n>> this patch adds clear error message for this case.\n>>\n>> diff --git a/fetch.c b/fetch.c\n>> index c426c04..40c5183 100644\n>> --- a/fetch.c\n>> +++ b/fetch.c\n>> @@ -266,6 +266,14 @@ int pull(int targets, char **target, con\n>>  \t\tif (!write_ref || !write_ref[i])\n>>  \t\t\tcontinue;\n>>  \n>> +\t\tif (*write_ref[i] == '\\0') {\n>> +\t\t\tif (strncmp(write_ref_log_details, \"http\", 4) == 0)\n>> +\t\t\t\terror(\"Can't feed empty ref, seems you are fetching from a gitweb URL, \"\n>> +\t\t\t\t      \"check it in web browser for git URL.\");\n>> +\t\t\telse\n>> +\t\t\t\terror(\"Can't feed empty ref\");\n>> +\t\t\tgoto unlock_and_fail;\n> \n> You might have got that error by feeding an URL for gitweb, but\n> I do not think the code, even with your additions, knows enough\n> to tell that the user's mistake isn't other kinds of errors.\n> \n> I am afraid that it would cause the user to waste time going\n> wild goose chase if you say \"seems you are...\".  The phrasing\n> makes it sound as if the tool _knows_ with some certainty that\n> it is more plausible cause of the error than other kinds, while\n> it certainly doesn't.\n> \n\nI agree with you, it's not a fault of fetch.c.\n\n> I think the reason it does not notice the breakage much earlier\n> is that git-clone does not notice that gitweb URL gives nonsense\n> to requests to \"http://host/gitweb.cgi/$project/info/refs\", so\n> your patch to git-clone.sh is probably touching the right place,\n> but I still feel the wording is a bit too strong and definitive\n> than it should be.\n> \n> Perhaps...\n> \n> diff --git a/git-clone.sh b/git-clone.sh\n> index 3f006d1..7ae69d9 100755\n> --- a/git-clone.sh\n> +++ b/git-clone.sh\n> @@ -46,15 +46,18 @@ Perhaps git-update-server-info needs to\n>  \tdo\n>  \t\tname=`expr \"z$refname\" : 'zrefs/\\(.*\\)'` &&\n>  \t\tcase \"$name\" in\n> -\t\t*^*)\tcontinue;;\n> -\t\tesac\n> +\t\t*^*)\tcontinue ;;\n> +\t\t'')\tfalse ;;\n> +\t\tesac &&\n>  \t\tif test -n \"$use_separate_remote\" &&\n>  \t\t   branch_name=`expr \"z$name\" : 'zheads/\\(.*\\)'`\n>  \t\tthen\n>  \t\t\ttname=\"remotes/$origin/$branch_name\"\n>  \t\telse\n>  \t\t\ttname=$name\n> -\t\tfi\n> +\t\tfi || {\n> +\t\t\tdie \"info/refs has nonsense $sha1 $refname, are you pulling from the right repository URL?\"\n> +\t\t}\n>  \t\tgit-http-fetch -v -a -w \"$tname\" \"$name\" \"$1/\" || exit 1\n>  \tdone <\"$clone_tmp/refs\"\n>  \trm -fr \"$clone_tmp\"\n> \nIt works well. Maybe it's better to say \"info/refs has nonsense sha1($sha1) \n"}]}