Re: [PATCH] clone: do not segfault when specifying a nonexistent branch

[Ralf Thielow <ralf.thielow@gmail.com>]

On Sat, Oct 5, 2013 at 1:55 AM, Duy Nguyen <pclouds@gmail.com> wrote:
> On Fri, Oct 4, 2013 at 9:20 PM, Stefan Beller
> <stefanbeller@googlemail.com> wrote:
>> I think we should emit a warning additionally?
>>
>> Signed-off-by: Stefan Beller <stefanbeller@googlemail.com>
>
> I think it's nice to credit Robert for reporting the fault in the
> commit message (something like "reported-by:" or "noticed-by:"...)
>
>> ---
>>  builtin/clone.c | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/builtin/clone.c b/builtin/clone.c
>> index 0aff974..b764ad0 100644
>> --- a/builtin/clone.c
>> +++ b/builtin/clone.c
>> @@ -688,7 +688,7 @@ static void write_refspec_config(const char* src_ref_prefix,
>>
>>         if (option_mirror || !option_bare) {
>>                 if (option_single_branch && !option_mirror) {
>> -                       if (option_branch) {
>> +                       if (option_branch && our_head_points_at) {
>>                                 if (strstr(our_head_points_at->name, "refs/tags/"))
>>                                         strbuf_addf(&value, "+%s:%s", our_head_points_at->name,
>>                                                 our_head_points_at->name);
>
> This prevents the segfault, but what about remote.*.fetch? Should we
> setup standard refspec for fetch or..?
> --
> Duy

This segfault only happens when cloning an empty repository and only with option
"--single-branch". Or do I miss something?

If we call "git clone" for a non-empty repository with a non-existing branch
using "[--single-branch] --branch foo" then Git will abort with a message that
the branch doesn't exist in upstream.

In an empty upstream repo the branch doesn't exist, either. So why not
abort with
the same message? That would be consistent. Otherwise I'd just
override the options
"--single-branch" and "--branch" to "not set".

Ralf