From: Liu Yubao <yubao.liu@gmail•com>
To: Junio C Hamano <junkio@cox•net>
Cc: git@vger•kernel.org
Subject: Re: [PATCH] clear error message for clone a gitweb URL
Date: Thu, 09 Nov 2006 09:49:09 +0800 [thread overview]
Message-ID: <45528915.8090608@gmail.com> (raw)
In-Reply-To: <7v7iy5sma9.fsf@assigned-by-dhcp.cox.net>
Junio C Hamano wrote:
> Liu Yubao <yubao.liu@gmail•com> writes:
>
>> 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 agree with you, it's not a fault of fetch.c.
> 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"
>
It works well. Maybe it's better to say "info/refs has nonsense sha1($sha1)
prev parent reply other threads:[~2006-11-09 1:50 UTC|newest]
Thread overview: 3+ messages / expand[flat|nested] mbox.gz Atom feed top
2006-11-08 7:25 [PATCH] clear error message for clone a gitweb URL Liu Yubao
2006-11-08 20:42 ` Junio C Hamano
2006-11-09 1:49 ` Liu Yubao [this message]
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=45528915.8090608@gmail.com \
--to=yubao.liu@gmail$(echo .)com \
--cc=git@vger$(echo .)kernel.org \
--cc=junkio@cox$(echo .)net \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox