Re: [PATCH v9 07/21] stash: convert apply to builtin
On 09/26, Paul-Sebastian Ungureanu wrote:
> From: Joel Teichroeb
>
> Add a builtin helper for performing stash commands. Converting
> all at once proved hard to review, so starting with just apply
> lets conversion get started without the other commands 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
> Signed-off-by: Paul-Sebastian Ungureanu
> ---
> .gitignore | 1 +
> Makefile| 1 +
> builtin.h | 1 +
> builtin/stash--helper.c | 452
> git-stash.sh| 78 +--
> git.c | 1 +
> 6 files changed, 463 insertions(+), 71 deletions(-)
> create mode 100644 builtin/stash--helper.c
>
> diff --git a/.gitignore b/.gitignore
> index ffceea7d59..b59661cb88 100644
> --- a/.gitignore
> +++ b/.gitignore
> @@ -157,6 +157,7 @@
> /git-show-ref
> /git-stage
> /git-stash
> +/git-stash--helper
> /git-status
> /git-stripspace
> /git-submodule
> diff --git a/Makefile b/Makefile
> index d03df31c2a..f900c68e69 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -1093,6 +1093,7 @@ BUILTIN_OBJS += builtin/shortlog.o
> BUILTIN_OBJS += builtin/show-branch.o
> BUILTIN_OBJS += builtin/show-index.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 99206df4bd..317bc338f7 100644
> --- a/builtin.h
> +++ b/builtin.h
> @@ -223,6 +223,7 @@ 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_show_index(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..7819dae332
> --- /dev/null
> +++ b/builtin/stash--helper.c
> @@ -0,0 +1,452 @@
> +#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"
> +#include "rerere.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 struct strbuf stash_index_path = STRBUF_INIT;
> +
> +/*
> + * w_commit is set to the commit containing the working tree
> + * b_commit is set to the base commit
> + * i_commit is set to the commit containing the index tree
> + * u_commit is set to the commit containing the untracked files tree
> + * w_tree is set to the working tree
> + * b_tree is set to the base tree
> + * i_tree is set to the index tree
> + * u_tree is set to the untracked files tree
> + */
> +
> +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;
> + struct strbuf revision;
> + int is_stash_ref;
> + int has_u;
> +};
> +
> +static void free_stash_info(struct stash_info *info)
> +{
> + strbuf_release(>revision);
> +}
> +
> +static void assert_stash_like(struct stash_info *info, const char *revision)
> +{
> + if (get_oidf(>b_commit, "%s^1", revision) ||
> + get_oidf(>w_tree, "%s:", revision) ||
> + get_oidf(>b_tree, "%s^1:", revision) ||
> + get_oidf(>i_tree, "%s^2:", revision)) {
> + free_stash_info(info);
> + error(_("'%s' is not a stash-like commit"), revision);
> + exit(128);
This seems to just emulate 'die()'. Can we just use that directly?
The only reason I could imagine for not doing that would be to keep
the same exit code, or the exact same message we had in the shell
script. But we're doing neither here. The
[PATCH v9 07/21] stash: convert apply to builtin
From: Joel Teichroeb
Add a builtin helper for performing stash commands. Converting
all at once proved hard to review, so starting with just apply
lets conversion get started without the other commands 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
Signed-off-by: Paul-Sebastian Ungureanu
---
.gitignore | 1 +
Makefile| 1 +
builtin.h | 1 +
builtin/stash--helper.c | 452
git-stash.sh| 78 +--
git.c | 1 +
6 files changed, 463 insertions(+), 71 deletions(-)
create mode 100644 builtin/stash--helper.c
diff --git a/.gitignore b/.gitignore
index ffceea7d59..b59661cb88 100644
--- a/.gitignore
+++ b/.gitignore
@@ -157,6 +157,7 @@
/git-show-ref
/git-stage
/git-stash
+/git-stash--helper
/git-status
/git-stripspace
/git-submodule
diff --git a/Makefile b/Makefile
index d03df31c2a..f900c68e69 100644
--- a/Makefile
+++ b/Makefile
@@ -1093,6 +1093,7 @@ BUILTIN_OBJS += builtin/shortlog.o
BUILTIN_OBJS += builtin/show-branch.o
BUILTIN_OBJS += builtin/show-index.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 99206df4bd..317bc338f7 100644
--- a/builtin.h
+++ b/builtin.h
@@ -223,6 +223,7 @@ 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_show_index(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..7819dae332
--- /dev/null
+++ b/builtin/stash--helper.c
@@ -0,0 +1,452 @@
+#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"
+#include "rerere.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 struct strbuf stash_index_path = STRBUF_INIT;
+
+/*
+ * w_commit is set to the commit containing the working tree
+ * b_commit is set to the base commit
+ * i_commit is set to the commit containing the index tree
+ * u_commit is set to the commit containing the untracked files tree
+ * w_tree is set to the working tree
+ * b_tree is set to the base tree
+ * i_tree is set to the index tree
+ * u_tree is set to the untracked files tree
+ */
+
+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;
+ struct strbuf revision;
+ int is_stash_ref;
+ int has_u;
+};
+
+static void free_stash_info(struct stash_info *info)
+{
+ strbuf_release(>revision);
+}
+
+static void assert_stash_like(struct stash_info *info, const char *revision)
+{
+ if (get_oidf(>b_commit, "%s^1", revision) ||
+ get_oidf(>w_tree, "%s:", revision) ||
+ get_oidf(>b_tree, "%s^1:", revision) ||
+ get_oidf(>i_tree, "%s^2:", revision)) {
+ free_stash_info(info);
+ error(_("'%s' is not a stash-like commit"), revision);
+ exit(128);
+ }
+}
+
+static int get_stash_info(struct stash_info *info, int argc, const char **argv)
+{
+ int ret;
+ char *end_of_rev;
+ char *expanded_ref;
+ const char *revision;
+ const char *commit = NULL;
+ struct object_id dummy;
+ struct strbuf symbolic = STRBUF_INIT;
+
+ if (argc > 1) {
+ int i;
+ struct strbuf refs_msg = STRBUF_INIT;
+ for (i = 0; i < argc; i++)
+ strbuf_addf(_msg, " '%s'", argv[i]);
+
