Subject: Re: [RFC PATCH] for-each-ref: respect GIT_REF_PARANOIA setting

From: Junio C Hamano <[email-masked]>
Date: Mon, 21 Mar 2022 13:33:04 -0700

Taylor Blau <[email-masked]> writes:

> +	ret = filter_refs(&array, &filter, FILTER_REFS_ALL);
> +	if (ret)
> +		goto cleanup;
>  	ref_array_sort(sorting, &array);

Fixing the "regression" would be a simple matter of removing the
early return from here, and instead show what we have collected?

>  	if (!maxcount || array.nr < maxcount)
> @@ -91,11 +93,12 @@ int cmd_for_each_ref(int argc, const char **argv, const char *prefix)
>  		putchar('\n');
>  	}
>  
> +cleanup:
>  	strbuf_release(&err);
>  	strbuf_release(&output);
>  	ref_array_clear(&array);
>  	free_commit_list(filter.with_commit);
>  	free_commit_list(filter.no_commit);
>  	ref_sorting_release(sorting);
> -	return 0;
> +	return ret;
>  }
> diff --git a/ref-filter.c b/ref-filter.c
> index 7838bd22b8..f9667f6ca4 100644
> --- a/ref-filter.c
> +++ b/ref-filter.c
> @@ -2249,7 +2249,7 @@ static int ref_filter_handler(const char *refname, const struct object_id *oid,
>  
>  	if (flag & REF_ISBROKEN) {
>  		warning(_("ignoring broken ref %s"), refname);
> -		return 0;
> +		return 1;
>  	}

Ah, no, not really.  This causes us to stop iterating prematurely.

If we are iterating because we want to find any breakage, such an
early stop in iteration makes good sense, but most of the time, we
are not, and it is questionable if such an early return makes much
sense.

I suspect that a handler may need to keep going, while recording a
bit for each ref it collects.  ref_array_item may or may not have (I
do not know offhand) already a bit in its flag word to signal a broken
ref that we can carry this information out to the callers?