Re: [PATCH] clone: ignore invalid local refs in remote

[Junio C Hamano <gitster@pobox.com>]

"Derrick Stolee via GitGitGadget" <gitgitgadget@gmail.com> writes:

> From: Derrick Stolee <derrickstolee@github.com>
> Subject: Re: [PATCH] clone: ignore invalid local refs in remote

After seeing the title, I expected that cloning from such a
repository with cruft in .git/refs/ directory would issue a warning
and succeed without these non-ref files.

But that is not what is happening here?

> +test_expect_success 'local clone from repo with corrupt refs fails gracefully' '
> +	git init corrupt &&
> +	test_commit -C corrupt one &&
> +	echo a >corrupt/.git/refs/heads/topic &&
> +
> +	test_must_fail git clone corrupt working 2>err &&
> +	grep "has a null OID" err
> +'
> +

We keep expecting that clone _will_ fail.

So the net change is that we still do not tolerate a corrupt
repository and do not let corruption to propagate through cloning,
but we diagnose this breakage as an error by calling die(), which 
is appropriate for dealing with runtime data error, instead of
hitting a BUG(), which is reserved for program errors.

I agree with the fixed behaviour and implementation.  It just is
that "ignore" on the title seems misleading.  Other than that,
thanks for a good finding and a clean fix.

> -	if (!new_oid || is_null_oid(new_oid))
> -		BUG("create called without valid new_oid");
> +	if (!new_oid || is_null_oid(new_oid)) {
> +		strbuf_addf(err, "'%s' has a null OID", refname);
> +		return 1;
> +	}