Re: [PATCH 1/4] refs: add referent parameter to refs_resolve_ref_unsafe

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

John Cai <johncai86@gmail.com> writes:

> Yes, so for 3/4 I was exploring doing the same thing. However, each_repo_fn goes
> pretty deep in the callstack and to provide an alternate set of functions that
> use something like each_repo_referent_fn would still lead to a relatively large
> blast radius, eg, something like:

Hmph.  You only care about teaching referent to calls to
refs_for_each_ref_in() and friends in apply_ref_filter()
and nowhere else.  So for example, to preserve the current
calling pattern for this pair of caller/callback (and you have
dozens more exactly like this one you touch in [3/4]):

	static int register_ref(const char *refname,
                                const struct object_id *oid,
				int flags,
				void *cb_data);

	static int read_bisect_refs(void)
	{
		return refs_for_each_ref_in(get_main_ref_store(the_repository),
					    "refs/bisect", register_ref, NULL);
	}

you could arrange your _new_ API the way you want, e.g.,

	typedef int each_ref_with_referent_fn(const char *refname,
	                                      const char *referent,
                                              const struct object_id *oid,
					      int flags,
					      void *cb_data);

	int refs_for_each_ref_with_referent_in(struct ref_store *refs,
					       const char *prefix,
                                               each_ref_with_referent_fn fn,
					       void *cb_data);

to help the single caller, i.e., apply_ref_filter(), and rebuild the
existing API on top as a thin wrapper, e.g.

	/* This cannot change without butchering existing callers */
	typedef int each_ref_fn(const char *refname,
                                const struct object_id *oid,
				int flags,
				void *cb_data);

	/* Hence we introduce an adapter */
	struct each_ref_fn_adapter_cbdata {
		each_ref_fn user_each_ref_fn;
		void *user_cb_data;
	};

	/* This is designed to work as an each_ref_with_referent_fn */
	static int each_ref_adapter_fn(const char *refname,
				       const char *referent,
                                       const struct object_id *oid,
				       int flags,
				       void *cb_data)
	{
		struct each_ref_fn_adapter_cbdata *adapter_cbdata = cbdata;

		/* the callers have no need for referent */
                return adapter_cbdata->user_each_ref_fn(refname, oid, flags,
					                adapter_cbdata->user_cbdata);
	}

	/*
         * The function signature must stay the same to help existing,
         * callers, but the implementation is now a thin wrapper.
	 */
	int refs_for_each_ref_in(struct ref_store *refs,
				 const char *prefix,
                                 each_ref_fn fn,
				 void *cb_data)
        {
		struct each_ref_fn_adapter_cbdata adapter_cbdata = {
                	.user_each_ref_fn = fn,
			.user_cb_data = cb_data,
		};
		return refs_for_each_ref_with_referetnt_in(refs, prefix,
							   each_ref_adapter_fn,
                                                           &adapter_cbdata);
	}

no?

You'd need to pass through the new parameter "referent" through the
code paths that implement refs_for_each_ref_in() and friends to
update them to refs_for_each_ref_with_referent_in() and friends no
matter what, but there are limited number of the top-level
refs_for_each_ref_in() and friends that are unaware of the
"referent", and each of them would need the above ~20 lines (couting
the comment) adapter function that all can share the single
each_ref_adapter_fn() callback function.

Or am I missing some other intricacy in the existing for-each-* API?