Re: [PATCH 1/4] stash: convert apply to builtin

[Joel Teichroeb]

On Sun, Mar 25, 2018 at 9:43 AM, Thomas Gummerer  wrote:
> On 03/24, Joel Teichroeb wrote:
>> ---
>
> Missing sign-off?  I saw it's missing in the other patches as well.
>

Thanks! I always forget to add a sign-off.

>> [...]
>> +
>> + 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(, "read-tree");
>> + argv_array_push(, sha1_to_hex(info->u_tree.hash));
>> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
>> stash_index_path);
>> +
>> + cp2.git_cmd = 1;
>> + argv_array_pushl(, "checkout-index", "--all", NULL);
>> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
>> stash_index_path);
>> +
>> + res = run_command() || run_command();
>> + 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.
>

I'm not sure about that. The shell script has a trap near the start in
order to clean up the temp index, unless I'm understanding the shell
script incorrectly.

Re: [PATCH 1/4] stash: convert apply to builtin

[Christian Couder]

On Sun, Mar 25, 2018 at 6:51 PM, Joel Teichroeb  wrote:
> On Sun, Mar 25, 2018 at 1:09 AM, Christian Couder
>  wrote:
>> It seems to me that the apply_stash() shell function is also used in
>> pop_stash() and in apply_to_branch(). Can the new helper be used there
>> too instead of apply_stash()? And then could apply_stash() be remove?
>
> I wasn't really sure if I should remove code from the .sh file as it
> seems in the past the old .sh files have been kept around as examples.

Yeah, some original shell scripts that have been converted are kept in
contrib/examples/, but the shell code has still been removed from the
.sh files when they were being converted.

> Has that been done for previous conversions?

I don't think there were some cases when the shell code was not
removed. I haven't looked at all the conversions in details though.

Re: [PATCH 1/4] stash: convert apply to builtin

[Thomas Gummerer]

On 03/24, Joel Teichroeb wrote:
> ---
> [...]
> +
> +static const char *ref_stash = "refs/stash";
> +static int quiet;
> +static char stash_index_path[PATH_MAX];
> +
> +struct stash_info {
> + struct object_id w_commit;
> + struct object_id b_commit;
> + struct object_id i_commit;
> + struct object_id u_commit;
> + struct object_id w_tree;
> + struct object_id b_tree;
> + struct object_id i_tree;
> + struct object_id u_tree;
> + const char *message;
> + const char *revision;
> + int is_stash_ref;
> + int has_u;
> + const char *patch;
> +};
> +
> +static int get_stash_info(struct stash_info *info, const char *commit)
> +{
> + struct strbuf w_commit_rev = STRBUF_INIT;
> + struct strbuf b_commit_rev = STRBUF_INIT;
> + struct strbuf w_tree_rev = STRBUF_INIT;
> + struct strbuf b_tree_rev = STRBUF_INIT;
> + struct strbuf i_tree_rev = STRBUF_INIT;
> + struct strbuf u_tree_rev = STRBUF_INIT;
> + struct strbuf commit_buf = STRBUF_INIT;
> + struct strbuf symbolic = STRBUF_INIT;
> + struct strbuf out = STRBUF_INIT;
> + int ret;
> + const char *revision = commit;
> + char *end_of_rev;
> + struct child_process cp = CHILD_PROCESS_INIT;
> + info->is_stash_ref = 0;
> +
> + if (commit == NULL) {
> + strbuf_addf(_buf, "%s@{0}", ref_stash);
> + revision = commit_buf.buf;

Before setting up the revisions here, as is done below, we used to
check if a stash even exists, if no commit was given.  So in a
repository with no stashes we would die with "No stash entries found",
while now we die with "error: refs/stash@{0} is not a valid
reference".  I think the error message we had previously was slightly
nicer, and we should try to keep it.

> + } else if (strspn(commit, "0123456789") == strlen(commit)) {
> + strbuf_addf(_buf, "%s@{%s}", ref_stash, commit);
> + revision = commit_buf.buf;
> + }
> + info->revision = revision;
> +
> + strbuf_addf(_commit_rev, "%s", revision);
> + strbuf_addf(_commit_rev, "%s^1", revision);
> + strbuf_addf(_tree_rev, "%s:", revision);
> + strbuf_addf(_tree_rev, "%s^1:", revision);
> + strbuf_addf(_tree_rev, "%s^2:", revision);
> +
> + ret = !get_oid(w_commit_rev.buf, >w_commit) &&
> + !get_oid(b_commit_rev.buf, >b_commit) &&
> + !get_oid(w_tree_rev.buf, >w_tree) &&
> + !get_oid(b_tree_rev.buf, >b_tree) &&
> + !get_oid(i_tree_rev.buf, >i_tree);
> +
> + strbuf_release(_commit_rev);
> + strbuf_release(_commit_rev);
> + strbuf_release(_tree_rev);
> + strbuf_release(_tree_rev);
> + strbuf_release(_tree_rev);
> +
> + if (!ret)
> + return error(_("%s is not a valid reference"), revision);

We used to distinguish between "not a valid reference" and "not a
stash-like commit" here.  I think just doing the first 'get_oid'
before the others, and returning the error if that fails, and then
doing the rest and returning the "not a stash-like commit" if one of
the other 'get_oid' calls fails would work, although I did not test it.

> +
> + strbuf_addf(_tree_rev, "%s^3:", revision);
> +
> + info->has_u = !get_oid(u_tree_rev.buf, >u_tree);
> +
> + strbuf_release(_tree_rev);
> +
> + end_of_rev = strchrnul(revision, '@');
> + strbuf_add(, revision, end_of_rev - revision);
> + cp.git_cmd = 1;
> + argv_array_pushl(, "rev-parse", "--symbolic-full-name", NULL);
> + argv_array_pushf(, "%s", symbolic.buf);
> + strbuf_release();
> + pipe_command(, NULL, 0, , 0, NULL, 0);
> +
> + if (out.len - 1 == strlen(ref_stash))
> + info->is_stash_ref = !strncmp(out.buf, ref_stash, out.len - 1);
> + strbuf_release();
> +
> + return 0;
> +}
> +
> [...]

Re: [PATCH 1/4] stash: convert apply to builtin

[Joel Teichroeb]

On Sun, Mar 25, 2018 at 1:09 AM, Christian Couder
 wrote:
> It seems to me that the apply_stash() shell function is also used in
> pop_stash() and in apply_to_branch(). Can the new helper be used there
> too instead of apply_stash()? And then could apply_stash() be remove?

I wasn't really sure if I should remove code from the .sh file as it
seems in the past the old .sh files have been kept around as examples.
Has that been done for previous conversions?

Re: [PATCH 1/4] stash: convert apply to builtin

[Thomas Gummerer]

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(>b_tree, >i_tree) || !oidcmp(_tree, 
> >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(, "diff-tree", "--binary", 
> NULL);
> + argv_array_pushf(, "%s^2^..%s^2", 
> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
> + if (pipe_command(, NULL, 0, , 0, NULL, 0))
> + return -1;
> +
> + child_process_init();
> + cp.git_cmd = 1;
> + argv_array_pushl(, "apply", "--cached", NULL);
> + if (pipe_command(, out.buf, out.len, NULL, 0, NULL, 
> 0))
> + return -1;
> +
> + strbuf_release();
> + discard_cache();
> + read_cache();
> + if (write_cache_as_tree(index_tree.hash, 0, NULL))
> + return -1;
> +
> + argv_array_push(, "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(, "read-tree");
> + argv_array_push(, sha1_to_hex(info->u_tree.hash));
> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
> stash_index_path);
> +
> + cp2.git_cmd = 1;
> + argv_array_pushl(, "checkout-index", "--all", NULL);
> + argv_array_pushf(_array, "GIT_INDEX_FILE=%s", 
> stash_index_path);
> +
> + res = run_command() || run_command();
> + 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.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] = >b_tree;
> +
> + ret = merge_recursive_generic(, _tree, >w_tree, bases_count, 
> bases, );
> + if (ret != 0) {
> + struct argv_array args = ARGV_ARRAY_INIT;
> + argv_array_push(, "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;
> + }
> +
> [...]

Re: [PATCH 1/4] stash: convert apply to builtin

[Christian Couder]

On Sun, Mar 25, 2018 at 8:40 AM, Eric Sunshine  wrote:
> On Sat, Mar 24, 2018 at 1:37 PM, Joel Teichroeb  wrote:

>> +static int do_apply_stash(const char *prefix, struct stash_info *info, int 
>> index)
>> +{
>> +   if (index) {
>> +   if (!oidcmp(>b_tree, >i_tree) || 
>> !oidcmp(_tree, >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(, "diff-tree", "--binary", 
>> NULL);
>> +   argv_array_pushf(, "%s^2^..%s^2", 
>> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
>> +   if (pipe_command(, NULL, 0, , 0, NULL, 0))
>> +   return -1;
>
> Leaking 'out'?
>
>> +
>> +   child_process_init();
>> +   cp.git_cmd = 1;
>> +   argv_array_pushl(, "apply", "--cached", 
>> NULL);
>> +   if (pipe_command(, out.buf, out.len, NULL, 0, 
>> NULL, 0))
>> +   return -1;
>
> Leaking 'out'.

It might be a good idea to have small functions encapsulating the
forks of git commands. For example:

static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)
{
struct child_process cp = CHILD_PROCESS_INIT;
const char *w_commit_hex = oid_to_hex(w_commit);

cp.git_cmd = 1;
argv_array_pushl(, "diff-tree", "--binary", NULL);
argv_array_pushf(, "%s^2^..%s^2", w_commit_hex, w_commit_hex);

return pipe_command(, NULL, 0, out, 0, NULL, 0);
}

static int apply_cached(struct strbuf *out)
{
struct child_process cp = CHILD_PROCESS_INIT;

cp.git_cmd = 1;
argv_array_pushl(, "apply", "--cached", NULL);
return pipe_command(, out->buf, out->len, NULL, 0, NULL, 0);
}

This could help find leaks and maybe later make it easier to call
libified code instead of forking a git command.

Re: [PATCH 1/4] stash: convert apply to builtin

[Christian Couder]

On Sat, Mar 24, 2018 at 6:37 PM, Joel Teichroeb  wrote:
> diff --git a/git-stash.sh b/git-stash.sh
> index fc8f8ae640..92c084eb17 100755
> --- a/git-stash.sh
> +++ b/git-stash.sh
> @@ -711,7 +711,8 @@ push)
> ;;
>  apply)
> shift
> -   apply_stash "$@"
> +   cd "$START_DIR"
> +   git stash--helper apply "$@"
> ;;
>  clear)
> shift

It seems to me that the apply_stash() shell function is also used in
pop_stash() and in apply_to_branch(). Can the new helper be used there
too instead of apply_stash()? And then could apply_stash() be remove?

Re: [PATCH 1/4] stash: convert apply to builtin

[Eric Sunshine]

On Sat, Mar 24, 2018 at 1:37 PM, Joel Teichroeb  wrote:
> diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
> @@ -0,0 +1,339 @@
> +static int get_stash_info(struct stash_info *info, const char *commit)
> +{
> +   struct strbuf w_commit_rev = STRBUF_INIT;
> +   struct strbuf b_commit_rev = STRBUF_INIT;
> +   struct strbuf w_tree_rev = STRBUF_INIT;
> +   struct strbuf b_tree_rev = STRBUF_INIT;
> +   struct strbuf i_tree_rev = STRBUF_INIT;
> +   struct strbuf u_tree_rev = STRBUF_INIT;
> +   struct strbuf commit_buf = STRBUF_INIT;
> +   struct strbuf symbolic = STRBUF_INIT;
> +   struct strbuf out = STRBUF_INIT;

'commit_buf' is being leaked. All the others seem to be covered (even
in the case of early 'return').

> +   if (commit == NULL) {
> +   strbuf_addf(_buf, "%s@{0}", ref_stash);
> +   revision = commit_buf.buf;
> +   } else if (strspn(commit, "0123456789") == strlen(commit)) {
> +   strbuf_addf(_buf, "%s@{%s}", ref_stash, commit);
> +   revision = commit_buf.buf;
> +   }
> +static int do_apply_stash(const char *prefix, struct stash_info *info, int 
> index)
> +{
> +   if (index) {
> +   if (!oidcmp(>b_tree, >i_tree) || !oidcmp(_tree, 
> >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(, "diff-tree", "--binary", 
> NULL);
> +   argv_array_pushf(, "%s^2^..%s^2", 
> sha1_to_hex(info->w_commit.hash), sha1_to_hex(info->w_commit.hash));
> +   if (pipe_command(, NULL, 0, , 0, NULL, 0))
> +   return -1;

Leaking 'out'?

> +
> +   child_process_init();
> +   cp.git_cmd = 1;
> +   argv_array_pushl(, "apply", "--cached", NULL);
> +   if (pipe_command(, out.buf, out.len, NULL, 0, 
> NULL, 0))
> +   return -1;

Leaking 'out'.

> +
> +   strbuf_release();
> +   discard_cache();
> +   read_cache();
> +   if (write_cache_as_tree(index_tree.hash, 0, NULL))
> +   return -1;
> +
> +   argv_array_push(, "reset");
> +   cmd_reset(args.argc, args.argv, prefix);
> +   }
> +   }
> +   if (has_index) {
> +   if (reset_tree(index_tree, 0, 0))
> +   return -1;
> +   } else {
> +   struct child_process cp = CHILD_PROCESS_INIT;
> +   struct strbuf out = STRBUF_INIT;
> +   cp.git_cmd = 1;
> +   argv_array_pushl(, "diff-index", "--cached", 
> "--name-only", "--diff-filter=A", NULL);
> +   argv_array_push(, sha1_to_hex(c_tree.hash));
> +   ret = pipe_command(, NULL, 0, , 0, NULL, 0);
> +   if (ret)
> +   return -1;
> +
> +   if (reset_tree(c_tree, 0, 1))
> +   return -1;

Leaking 'out' at these two 'return's?

> +   child_process_init();
> +   cp.git_cmd = 1;
> +   argv_array_pushl(, "update-index", "--add", 
> "--stdin", NULL);
> +   ret = pipe_command(, out.buf, out.len, NULL, 0, NULL, 0);
> +   if (ret)
> +   return -1;

And here.

> +
> +   strbuf_release();
> +   discard_cache();
> +   }
> +
> +   if (!quiet) {
> +   struct argv_array args = ARGV_ARRAY_INIT;
> +   argv_array_push(, "status");
> +   cmd_status(args.argc, args.argv, prefix);
> +   }
> +
> +   return 0;
> +}
> +
> +static int apply_stash(int argc, const char **argv, const char *prefix)
> +{
> +   const char *commit = NULL;
> +   int index = 0;
> +   struct stash_info info;
> +   struct option options[] = {
> +   OPT__QUIET(, N_("be quiet, only report errors")),
> +   OPT_BOOL(0, "index", ,
> +   N_("attempt to ininstate the index")),

"ininstate"??

> +   OPT_END()
> +   };
> +
> +   argc = parse_options(argc, argv, prefix, options,
> +   git_stash_helper_apply_usage, 0);
> +
> +   if (argc == 1) {
> +   commit = argv[0];
> +   }
> +
> +   if (get_stash_info(, commit))
> +   return -1;
> +
> +
> +   return do_apply_stash(prefix, , index);
> +}

Re: [PATCH 1/4] stash: convert apply to builtin

[Christian Couder]

> +   if (unpack_trees(nr_trees, t, ))
> +   return -1;
> +
> +   if (write_locked_index(_index, _file, COMMIT_LOCK)) {
> +   error(_("unable to write new index file"));
> +   return -1;

Maybe: return error(...);

> +   }
> +
> +   return 0;
> +}

[...]

> +   argc = parse_options(argc, argv, prefix, options,
> +   git_stash_helper_apply_usage, 0);
> +
> +   if (argc == 1) {
> +   commit = argv[0];
> +   }

The brackets are not needed.

> +   if (get_stash_info(, commit))
> +   return -1;
> +
> +

Spurious new line.

> +   return do_apply_stash(prefix, , index);
> +}

[PATCH 1/4] stash: convert apply to builtin

[Joel Teichroeb]

---
 .gitignore  |   1 +
 Makefile|   1 +
 builtin.h   |   1 +
 builtin/stash--helper.c | 339 
 git-stash.sh|   3 +-
 git.c   |   1 +
 6 files changed, 345 insertions(+), 1 deletion(-)
 create mode 100644 builtin/stash--helper.c

diff --git a/.gitignore b/.gitignore
index 833ef3b0b7..296d5f376d 100644
--- a/.gitignore
+++ b/.gitignore
@@ -152,6 +152,7 @@
 /git-show-ref
 /git-stage
 /git-stash
+/git-stash--helper
 /git-status
 /git-stripspace
 /git-submodule
diff --git a/Makefile b/Makefile
index a1d8775adb..8ca361c57a 100644
--- a/Makefile
+++ b/Makefile
@@ -1020,6 +1020,7 @@ BUILTIN_OBJS += builtin/send-pack.o
 BUILTIN_OBJS += builtin/shortlog.o
 BUILTIN_OBJS += builtin/show-branch.o
 BUILTIN_OBJS += builtin/show-ref.o
+BUILTIN_OBJS += builtin/stash--helper.o
 BUILTIN_OBJS += builtin/stripspace.o
 BUILTIN_OBJS += builtin/submodule--helper.o
 BUILTIN_OBJS += builtin/symbolic-ref.o
diff --git a/builtin.h b/builtin.h
index 42378f3aa4..a14fd85b0e 100644
--- a/builtin.h
+++ b/builtin.h
@@ -219,6 +219,7 @@ extern int cmd_shortlog(int argc, const char **argv, const 
char *prefix);
 extern int cmd_show(int argc, const char **argv, const char *prefix);
 extern int cmd_show_branch(int argc, const char **argv, const char *prefix);
 extern int cmd_status(int argc, const char **argv, const char *prefix);
+extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);
 extern int cmd_stripspace(int argc, const char **argv, const char *prefix);
 extern int cmd_submodule__helper(int argc, const char **argv, const char 
*prefix);
 extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);
diff --git a/builtin/stash--helper.c b/builtin/stash--helper.c
new file mode 100644
index 00..e9a9574f40
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,339 @@
+#include "builtin.h"
+#include "config.h"
+#include "parse-options.h"
+#include "refs.h"
+#include "lockfile.h"
+#include "cache-tree.h"
+#include "unpack-trees.h"
+#include "merge-recursive.h"
+#include "argv-array.h"
+#include "run-command.h"
+#include "dir.h"
+
+static const char * const git_stash_helper_usage[] = {
+   N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   NULL
+};
+
+static const char * const git_stash_helper_apply_usage[] = {
+   N_("git stash--helper apply [--index] [-q|--quiet] []"),
+   NULL
+};
+
+static const char *ref_stash = "refs/stash";
+static int quiet;
+static char stash_index_path[PATH_MAX];
+
+struct stash_info {
+   struct object_id w_commit;
+   struct object_id b_commit;
+   struct object_id i_commit;
+   struct object_id u_commit;
+   struct object_id w_tree;
+   struct object_id b_tree;
+   struct object_id i_tree;
+   struct object_id u_tree;
+   const char *message;
+   const char *revision;
+   int is_stash_ref;
+   int has_u;
+   const char *patch;
+};
+
+static int get_stash_info(struct stash_info *info, const char *commit)
+{
+   struct strbuf w_commit_rev = STRBUF_INIT;
+   struct strbuf b_commit_rev = STRBUF_INIT;
+   struct strbuf w_tree_rev = STRBUF_INIT;
+   struct strbuf b_tree_rev = STRBUF_INIT;
+   struct strbuf i_tree_rev = STRBUF_INIT;
+   struct strbuf u_tree_rev = STRBUF_INIT;
+   struct strbuf commit_buf = STRBUF_INIT;
+   struct strbuf symbolic = STRBUF_INIT;
+   struct strbuf out = STRBUF_INIT;
+   int ret;
+   const char *revision = commit;
+   char *end_of_rev;
+   struct child_process cp = CHILD_PROCESS_INIT;
+   info->is_stash_ref = 0;
+
+   if (commit == NULL) {
+   strbuf_addf(_buf, "%s@{0}", ref_stash);
+   revision = commit_buf.buf;
+   } else if (strspn(commit, "0123456789") == strlen(commit)) {
+   strbuf_addf(_buf, "%s@{%s}", ref_stash, commit);
+   revision = commit_buf.buf;
+   }
+   info->revision = revision;
+
+   strbuf_addf(_commit_rev, "%s", revision);
+   strbuf_addf(_commit_rev, "%s^1", revision);
+   strbuf_addf(_tree_rev, "%s:", revision);
+   strbuf_addf(_tree_rev, "%s^1:", revision);
+   strbuf_addf(_tree_rev, "%s^2:", revision);
+
+   ret = !get_oid(w_commit_rev.buf, >w_commit) &&
+   !get_oid(b_commit_rev.buf, >b_commit) &&
+   !get_oid(w_tree_rev.buf, >w_tree) &&
+   !get_oid(b_tree_rev.buf, >b_tree) &&
+   !get_oid(i_tree_rev.buf, >i_tree);
+
+   strbuf_release(_commit_rev);
+   strbuf_release(_commit_rev);
+   strbuf_release(_tree_rev);
+   strbuf_release(_tree_rev);
+   strbuf_release(_tree_rev);
+
+   if (!ret)
+   return error(_("%s is not a valid reference"), revision);
+
+   strbuf_addf(_tree_rev, "%s^3:", revision);
+
+   info->has_u = !get_oid(u_tree_rev.buf, >u_tree);
+
+   strbuf_release(_tree_rev);
+
+