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

From: Junio C Hamano <gitster@pobox.com>
Date: Mar 21, 2022, 20:33 UTC
Message-ID: <xmqqzgljvyf3.fsf@gitster.g>
In-Reply-To: <7283f826198aaec94c847f0b26e228ace9a38433.1647880211.git.me@ttaylorr.com>

Taylor Blau <me@ttaylorr.com> 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?

Show 26 quoted lines

>  	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?