Re: [PATCH v3 2/5] stash: convert apply to builtin

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Joel,

On Mon, 26 Mar 2018, Joel Teichroeb wrote:

> Add a bulitin helper for performing stash commands. Converting
> all at once proved hard to review, so starting with just apply
> let conversion get started without the other command being
> finished.
> 
> The helper is being implemented as a drop in replacement for
> stash so that when it is complete it can simply be renamed and
> the shell script deleted.
> 
> Delete the contents of the apply_stash shell function and replace
> it with a call to stash--helper apply until pop is also
> converted.
> 
> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>

Makes sense.

I need a couple of adjustments before it compiles on Windows:

> [...]
> +
> +static int do_apply_stash(const char *prefix, struct stash_info *info, int index)
> +{
> +	struct merge_options o;
> +	struct object_id c_tree;
> +	struct object_id index_tree;
> +	const struct object_id *bases[1];
> +	int bases_count = 1;
> +	struct commit *result;
> +	int ret;
> +	int has_index = index;
> +
> +	read_cache_preload(NULL);
> +	if (refresh_cache(REFRESH_QUIET))
> +		return -1;
> +
> +	if (write_cache_as_tree(&c_tree, 0, NULL) || reset_tree(&c_tree, 0, 0))

When applied on top of current `master`, I need to replace the &c_tree by
c_tree.hash.

Likewise...

> +		return error(_("Cannot apply a stash in the middle of a merge"));
> +
> +	if (index) {
> +		if (!oidcmp(&info->b_tree, &info->i_tree) || !oidcmp(&c_tree, &info->i_tree)) {
> +			has_index = 0;
> +		} else {
> +			struct strbuf out = STRBUF_INIT;
> +
> +			if (diff_tree_binary(&out, &info->w_commit)) {
> +				strbuf_release(&out);
> +				return -1;
> +			}
> +
> +			ret = apply_cached(&out);
> +			strbuf_release(&out);
> +			if (ret)
> +				return -1;
> +
> +			discard_cache();
> +			read_cache();
> +			if (write_cache_as_tree(&index_tree, 0, NULL))

... &index_tree -> index_tree.hash.

These are probably changed to use object_id's already in `pu`, I guess.

I also need this change:

> [...]
> +
> +	index_file = get_index_file();
> +	xsnprintf(stash_index_path, PATH_MAX, "%s.stash.%d", index_file, pid);

Since `pid_t` is `unsigned long long` on Windows, I changed the %d" to
%"PRIuMAX and cast `pid` to `(uintmax_t)`.

With those changes, the entire patch series compiles here.

BTW t3903 runs in 13m30s here with this patch series, 14m30s otherwise.
That might not seem like much, until you realize that t3903 *still*
performs a metric ton of Unix shell scripting outside of `git stash` (and
that is the reason for the slowness).

Ciao,
Dscho