On 03/24, Joel Teichroeb wrote:
> ---
Missing sign-off? I saw it's missing in the other patches as well.
> [...]
> +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.hash, 0, NULL) || reset_tree(c_tree, 0,
> 0))
> + 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 child_process cp = CHILD_PROCESS_INIT;
> + struct strbuf out = STRBUF_INIT;
> + struct argv_array args = ARGV_ARRAY_INIT;
> + cp.git_cmd = 1;
> + argv_array_pushl(&cp.args, "diff-tree", "--binary",
> NULL);
> + argv_array_pushf(&cp.args, "%s^2^..%s^2",
> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
> + if (pipe_command(&cp, NULL, 0, &out, 0, NULL, 0))
> + return -1;
> +
> + child_process_init(&cp);
> + cp.git_cmd = 1;
> + argv_array_pushl(&cp.args, "apply", "--cached", NULL);
> + if (pipe_command(&cp, out.buf, out.len, NULL, 0, NULL,
> 0))
> + return -1;
> +
> + strbuf_release(&out);
> + discard_cache();
> + read_cache();
> + if (write_cache_as_tree(index_tree.hash, 0, NULL))
> + return -1;
> +
> + argv_array_push(&args, "reset");
> + cmd_reset(args.argc, args.argv, prefix);
> + }
> + }
> +
> + if (info->has_u) {
> + struct child_process cp = CHILD_PROCESS_INIT;
> + struct child_process cp2 = CHILD_PROCESS_INIT;
> + int res;
> +
> + cp.git_cmd = 1;
> + argv_array_push(&cp.args, "read-tree");
> + argv_array_push(&cp.args, sha1_to_hex(info->u_tree.hash));
> + argv_array_pushf(&cp.env_array, "GIT_INDEX_FILE=%s",
> stash_index_path);
> +
> + cp2.git_cmd = 1;
> + argv_array_pushl(&cp2.args, "checkout-index", "--all", NULL);
> + argv_array_pushf(&cp2.env_array, "GIT_INDEX_FILE=%s",
> stash_index_path);
> +
> + res = run_command(&cp) || run_command(&cp2);
> + remove_path(stash_index_path);
> + if (res)
> + return error(_("Could not restore untracked files from
> stash"));
A minor change in behaviour here is that we are removing the temporary
index file unconditionally here, while we would previously only remove
it if both 'read-tree' and 'checkout-index' would succeed.
I don't think that's a bad thing, we probably don't want users to try
and use that index file in any way, and I doubt that's part of anyones
workflow, so I think cleaning it up makes sense.
> + }
> +
> + init_merge_options(&o);
> +
> + o.branch1 = "Updated upstream";
> + o.branch2 = "Stashed changes";
> +
> + if (!hashcmp(info->b_tree.hash, c_tree.hash))
> + o.branch1 = "Version stash was based on";
> +
> + if (quiet)
> + o.verbosity = 0;
> +
> + if (o.verbosity >= 3)
> + printf_ln(_("Merging %s with %s"), o.branch1, o.branch2);
> +
> + bases[0] = &info->b_tree;
> +
> + ret = merge_recursive_generic(&o, &c_tree, &info->w_tree, bases_count,
> bases, &result);
> + if (ret != 0) {
> + struct argv_array args = ARGV_ARRAY_INIT;
> + argv_array_push(&args, "rerere");
> + cmd_rerere(args.argc, args.argv, prefix);
> + if (index)
> + printf_ln(_("Index was not unstashed."));
Minor nit: I think the above should be 'fprintf_ln(stderr, ...)' to
match what we currently have.
> +
> + return ret;
> + }
> +
> [...]
