Re: [PATCH v19 5/7] branch: add --delete-merged <branch>

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

"Harald Nordgren via GitGitGadget" <gitgitgadget@gmail.com> writes:

> +struct spare_data {
> +	struct strset *deletable;
> +	struct strset *spared;
> +};
> +
> +/*
> + * A surviving branch stacked on a deletion candidate would lose its
> + * upstream, so drop that candidate from the delete set and remember it
> + * in "spared" so its own upstream can be tidied up afterwards.
> + */
> +static int spare_stacked_base(const struct reference *ref, void *cb_data)
> +{
> +	struct spare_data *data = cb_data;
> +	struct branch *branch;
> +	const char *upstream, *up_short;
> +
> +	if (strset_contains(data->deletable, ref->name))
> +		return 0;
> +	branch = branch_get(ref->name);
> +	upstream = branch_get_upstream(branch, NULL);
> +	if (!upstream || !skip_prefix(upstream, "refs/heads/", &up_short) ||
> +	    !strset_contains(data->deletable, up_short))
> +		return 0;
> +
> +	strset_remove(data->deletable, up_short);
> +	strset_add(data->spared, up_short);
> +	return 0;
> +}
> +
> +/*
> + * Keep any branch that a surviving branch tracks as its upstream, so we
> + * never delete a branch out from under one stacked on top of it.  Such a
> + * base is itself merged, so when its own upstream is also going away
> + * (no surviving branch tracks it), clear the base's now-stale upstream.
> + */
> +static void spare_stacked_bases(struct ref_store *refs, struct strset *deletable)
> +{
> +	struct strset spared = STRSET_INIT;
> +	struct spare_data data = { .deletable = deletable, .spared = &spared };
> +	struct strbuf key = STRBUF_INIT;
> +	struct hashmap_iter iter;
> +	struct strmap_entry *entry;
> +
> +	refs_for_each_branch_ref(refs, spare_stacked_base, &data);

Hmph.  Wouldn't this implicitly make whether a stacked branch is
spared or has its upstream configuration cleared depends on the
order in which the branches are visited by the callback function of
refs_for_each_branch_ref(), which presumably is alphabetical?

For example, if 'a_tip' (unmerged) tracks 'b_mid' (merged), which in
turn tracks 'c_lower' (merged), visiting them in alphabetical order
('a_tip', 'b_mid', 'c_lower') would spare both 'b_mid' and
'c_lower'.  If they, however, were named 'tip' (unmerged), which
tracks 'mid' (merged), which in turn tracks 'lower' (merged), they
would be visited in the order 'lower', 'mid', 'tip'.  This would
result in 'lower' being deleted and 'mid' being spared with its
upstream configuration cleared, even though the relationship among
these three branches is exactly the same.

Since the branches are visited in a fixed alphabetical order, it
might not be an unpredictable order that yields unrepeatable
results.  Nonetheless, the behavior should be consistent and
independent of branch names, as long as the inter-relationship
among the branches involved is identical, no?

Or am I grossly misreading the code?

Thanks.