Re: [PATCH] clear error message for clone a gitweb URL
- From
Junio C Hamano <junkio@cox.net>
- Date
- Nov 8, 2006, 20:42 UTC
- Message-ID
- <7v7iy5sma9.fsf@assigned-by-dhcp.cox.net>
- In-Reply-To
- <4551864D.3010301@gmail.com>
Liu Yubao <yubao.liu@gmail.com> writes:
Show 18 quoted lines
> When clone a gitweb URL, git reports "Can't lock ref", it's not clear for users,
> this patch adds clear error message for this case.
>
> diff --git a/fetch.c b/fetch.c
> index c426c04..40c5183 100644
> --- a/fetch.c
> +++ b/fetch.c
> @@ -266,6 +266,14 @@ int pull(int targets, char **target, con
> if (!write_ref || !write_ref[i])
> continue;
>
> + if (*write_ref[i] == '\0') {
> + if (strncmp(write_ref_log_details, "http", 4) == 0)
> + error("Can't feed empty ref, seems you are fetching from a gitweb URL, "
> + "check it in web browser for git URL.");
> + else
> + error("Can't feed empty ref");
> + goto unlock_and_fail;You might have got that error by feeding an URL for gitweb, but I do not think the code, even with your additions, knows enough to tell that the user's mistake isn't other kinds of errors.
I am afraid that it would cause the user to waste time going wild goose chase if you say "seems you are...". The phrasing makes it sound as if the tool _knows_ with some certainty that it is more plausible cause of the error than other kinds, while it certainly doesn't.
I think the reason it does not notice the breakage much earlier is that git-clone does not notice that gitweb URL gives nonsense to requests to "http://host/gitweb.cgi/$project/info/refs", so your patch to git-clone.sh is probably touching the right place, but I still feel the wording is a bit too strong and definitive than it should be.
Perhaps...
diff --git a/git-clone.sh b/git-clone.sh index 3f006d1..7ae69d9 100755 --- a/git-clone.sh +++ b/git-clone.sh @@ -46,15 +46,18 @@ Perhaps git-update-server-info needs to do name=`expr "z$refname" : 'zrefs/\(.*\)'` && case "$name" in - *^*) continue;; - esac + *^*) continue ;; + '') false ;; + esac && if test -n "$use_separate_remote" && branch_name=`expr "z$name" : 'zheads/\(.*\)'` then tname="remotes/$origin/$branch_name" else tname=$name - fi + fi || { + die "info/refs has nonsense $sha1 $refname, are you pulling from the right repository URL?" + } git-http-fetch -v -a -w "$tname" "$name" "$1/" || exit 1 done <"$clone_tmp/refs" rm -fr "$clone_tmp"