Re: [PATCH v4 3/5] stash: convert drop and clear to builtin

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

Joel Teichroeb <joel@teichroeb.net> writes:

> +static int do_clear_stash(void)
> +{
> +	struct object_id obj;
> +	if (get_oid(ref_stash, &obj))
> +		return 0;
> +	return delete_ref(NULL, ref_stash, &obj, 0);
> +}

Here you see if the "refs/stash" is there, and learn what its
current value is, using get_oid(), so that you can call delete_ref()
with it.

> +static int do_drop_stash(const char *prefix, struct stash_info *info)
> +{
> +	...
> +	cp.git_cmd = 1;
> +	/* Even though --quiet is specified, rev-parse still outputs the hash */
> +	cp.no_stdout = 1;
> +	argv_array_pushl(&cp.args, "rev-parse", "--verify", "--quiet", NULL);
> +	argv_array_pushf(&cp.args, "%s@{0}", ref_stash);
> +	ret = run_command(&cp);
> +	if (ret)
> +		do_clear_stash();

Here you call out to rev-parse as an external process.  Isn't doing
the same get_oid() sufficient?

Not limited to the above examples, the conversion in this series
feels somewhat unbalanced---doing easy things like this by forking
an external process and then doing a relatively heavyweight thing
like merge operation (in 2/5) in-process feels the other way around.

Thanks.