{"thread":{"id":"48146","subject":"[PATCH v2 0/6] Convert some stash functionality to a builtin","startedAt":"2018-03-26T01:14:37Z","lastAt":"2018-03-28T03:32:46Z","messageCount":10,"participants":["Joel Teichroeb","Christian Couder","Johannes Schindelin"],"isPatch":true,"patchVersion":2,"patchTotal":6},"messages":[{"id":"342950","messageId":"20180326011426.19159-1-joel@teichroeb.net","threadId":"48146","inReplyTo":null,"subject":"[PATCH v2 0/6] Convert some stash functionality to a builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:20Z","receivedAt":"2018-03-26T01:14:37Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"I've been working on converting all of git stash to be a\nbuiltin, however it's hard to get it all working at once with\nlimited time, so I've moved around half of it to a new\nstash--helper builtin and called these functions from the shell\nscript. Once this is stabalized, it should be easier to convert\nthe rest of the commands one at a time without breaking\nanything.\n\nI've sent most of this code before, but that was targetting a\nfull replacement of stash. The code is overall the same, but\nwith some code review changes and updates for internal api\nchanges.\n\nSince there seems to be interest from GSOC students who want to\nwork on converting builtins, I figured I should finish what I\nhave that works now so they could build on top of it.\n\nPrevious threads:\nv1: https://public-inbox.org/git/20180325173916.GE10909@hank/T/\n\nChanges from v1:\n - Fixed memory leaks\n - Fixed formatting\n - Fixed typos\n - Removed old .sh code that has been replaced\n - Fix differences in error messages\n - Added tests to ensure argument handling work when specifiying\n   multiple refs, or no arguments for branch\n - Refactored the code a bit\n - Reordered the patches so they compile and test successfully\n\nJoel Teichroeb (6):\n  stash: Add tests for passing in too many refs\n  stash: Add test for branch with no arguments\n  stash: convert apply to builtin\n  stash: convert drop and clear to builtin\n  stash: convert branch to builtin\n  stash: convert pop to builtin\n\n .gitignore              |   1 +\n Makefile                |   1 +\n builtin.h               |   1 +\n builtin/stash--helper.c | 620 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            | 125 +---------\n git.c                   |   1 +\n t/t3903-stash.sh        |  26 ++\n 7 files changed, 658 insertions(+), 117 deletions(-)\n create mode 100644 builtin/stash--helper.c\n\n-- \n2.16.2\n\n"},{"id":"342951","messageId":"20180326011426.19159-2-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 1/6] stash: Add tests for passing in too many refs","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:21Z","receivedAt":"2018-03-26T01:14:42Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n t/t3903-stash.sh | 22 ++++++++++++++++++++++\n 1 file changed, 22 insertions(+)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex aefde7b172..7146e27bb5 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -45,6 +45,12 @@ test_expect_success 'applying bogus stash does nothing' '\n \ttest_cmp expect file\n '\n \n+test_expect_success 'applying with too many agruments does nothing' '\n+\ttest_must_fail git stash apply stash@{0} bar &&\n+\techo 1 >expect &&\n+\ttest_cmp expect file\n+'\n+\n test_expect_success 'apply does not need clean working directory' '\n \techo 4 >other-file &&\n \tgit stash apply &&\n@@ -97,6 +103,10 @@ test_expect_success 'stash drop complains of extra options' '\n \ttest_must_fail git stash drop --foo\n '\n \n+test_expect_success 'stash drop complains with too many refs' '\n+\ttest_must_fail git stash drop stash@{1} stash@{2}\n+'\n+\n test_expect_success 'drop top stash' '\n \tgit reset --hard &&\n \tgit stash list > stashlist1 &&\n@@ -160,6 +170,10 @@ test_expect_success 'stash pop' '\n \ttest 0 = $(git stash list | wc -l)\n '\n \n+test_expect_success 'stash pop complains with too many refs' '\n+\ttest_must_fail git stash pop stash@{1} stash@{2}\n+'\n+\n cat > expect << EOF\n diff --git a/file2 b/file2\n new file mode 100644\n@@ -479,6 +493,10 @@ test_expect_success 'stash branch - stashes on stack, stash-like argument' '\n \ttest $(git ls-files --modified | wc -l) -eq 1\n '\n \n+test_expect_success 'stash branch complains with too many refs' '\n+\ttest_must_fail git stash branch stash-branch stash@{1} stash@{2}\n+'\n+\n test_expect_success 'stash show format defaults to --stat' '\n \tgit stash clear &&\n \ttest_when_finished \"git reset --hard HEAD\" &&\n@@ -567,6 +585,10 @@ test_expect_success 'stash show -p - no stashes on stack, stash-like argument' '\n \ttest_cmp expected actual\n '\n \n+test_expect_success 'stash show complains with too many refs' '\n+\ttest_must_fail git stash show stash@{1} stash@{2}\n+'\n+\n test_expect_success 'stash drop - fail early if specified stash is not a stash reference' '\n \tgit stash clear &&\n \ttest_when_finished \"git reset --hard HEAD && git stash clear\" &&\n-- \n2.16.2\n\n"},{"id":"342952","messageId":"20180326011426.19159-3-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 2/6] stash: Add test for branch with no arguments","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:22Z","receivedAt":"2018-03-26T01:14:45Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n t/t3903-stash.sh | 4 ++++\n 1 file changed, 4 insertions(+)\n\ndiff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\nindex 7146e27bb5..261571d02a 100755\n--- a/t/t3903-stash.sh\n+++ b/t/t3903-stash.sh\n@@ -497,6 +497,10 @@ test_expect_success 'stash branch complains with too many refs' '\n \ttest_must_fail git stash branch stash-branch stash@{1} stash@{2}\n '\n \n+test_expect_success 'stash branch complains with no arguments' '\n+\ttest_must_fail git stash branch\n+'\n+\n test_expect_success 'stash show format defaults to --stat' '\n \tgit stash clear &&\n \ttest_when_finished \"git reset --hard HEAD\" &&\n-- \n2.16.2\n\n"},{"id":"342953","messageId":"20180326011426.19159-5-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 4/6] stash: convert drop and clear to builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:24Z","receivedAt":"2018-03-26T01:14:48Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n builtin/stash--helper.c | 101 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            |   4 +-\n 2 files changed, 103 insertions(+), 2 deletions(-)\n\ndiff --git a/builtin/stash--helper.c b/builtin/stash--helper.c\nindex 050de415b4..640de6d0aa 100644\n--- a/builtin/stash--helper.c\n+++ b/builtin/stash--helper.c\n@@ -11,7 +11,14 @@\n #include \"dir.h\"\n \n static const char * const git_stash_helper_usage[] = {\n+\tN_(\"git stash--helper drop [-q|--quiet] [<stash>]\"),\n \tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n+\tN_(\"git stash--helper clear\"),\n+\tNULL\n+};\n+\n+static const char * const git_stash_helper_drop_usage[] = {\n+\tN_(\"git stash--helper drop [-q|--quiet] [<stash>]\"),\n \tNULL\n };\n \n@@ -20,6 +27,11 @@ static const char * const git_stash_helper_apply_usage[] = {\n \tNULL\n };\n \n+static const char * const git_stash_helper_clear_usage[] = {\n+\tN_(\"git stash--helper clear\"),\n+\tNULL\n+};\n+\n static const char *ref_stash = \"refs/stash\";\n static int quiet;\n static char stash_index_path[PATH_MAX];\n@@ -166,6 +178,29 @@ static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n \treturn 0;\n }\n \n+static int do_clear_stash(void)\n+{\n+\tstruct object_id obj;\n+\tif (get_oid(ref_stash, &obj))\n+\t\treturn 0;\n+\n+\treturn delete_ref(NULL, ref_stash, &obj, 0);\n+}\n+\n+static int clear_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options, git_stash_helper_clear_usage, PARSE_OPT_STOP_AT_NON_OPTION);\n+\n+\tif (argc != 0)\n+\t\treturn error(_(\"git stash--helper clear with parameters is unimplemented\"));\n+\n+\treturn do_clear_stash();\n+}\n+\n static int reset_tree(struct object_id *i_tree, int update, int reset)\n {\n \tstruct unpack_trees_options opts;\n@@ -399,6 +434,68 @@ static int apply_stash(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n+static int do_drop_stash(const char *prefix, struct stash_info *info)\n+{\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tint ret;\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\targv_array_pushl(&args, \"reflog\", \"delete\", \"--updateref\", \"--rewrite\", NULL);\n+\targv_array_push(&args, info->revision.buf);\n+\tret = cmd_reflog(args.argc, args.argv, prefix);\n+\tif (!ret) {\n+\t\tif (!quiet)\n+\t\t\tprintf(_(\"Dropped %s (%s)\\n\"), info->revision.buf, oid_to_hex(&info->w_commit));\n+\t} else {\n+\t\treturn error(_(\"%s: Could not drop stash entry\"), info->revision.buf);\n+\t}\n+\n+\tcp.git_cmd = 1;\n+\t/* Even though --quiet is specified, rev-parse still outputs the hash */\n+\tcp.no_stdout = 1;\n+\targv_array_pushl(&cp.args, \"rev-parse\", \"--verify\", \"--quiet\", NULL);\n+\targv_array_pushf(&cp.args, \"%s@{0}\", ref_stash);\n+\tret = run_command(&cp);\n+\n+\tif (ret)\n+\t\tdo_clear_stash();\n+\n+\treturn 0;\n+}\n+\n+static int assert_stash_ref(struct stash_info *info)\n+{\n+\tif (!info->is_stash_ref)\n+\t\treturn error(_(\"'%s' is not a stash reference\"), info->revision.buf);\n+\n+\treturn 0;\n+}\n+\n+static int drop_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tstruct stash_info info;\n+\tint ret;\n+\tstruct option options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tgit_stash_helper_drop_usage, 0);\n+\n+\tif (get_stash_info(&info, argc, argv))\n+\t\treturn -1;\n+\n+\tif (assert_stash_ref(&info)) {\n+\t\tdestroy_stash_info(&info);\n+\t\treturn -1;\n+\t}\n+\n+\tret = do_drop_stash(prefix, &info);\n+\tdestroy_stash_info(&info);\n+\treturn ret;\n+}\n+\n int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n {\n \tint result = 0;\n@@ -421,6 +518,10 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n \t\tusage_with_options(git_stash_helper_usage, options);\n \telse if (!strcmp(argv[0], \"apply\"))\n \t\tresult = apply_stash(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"clear\"))\n+\t\tresult = clear_stash(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"drop\"))\n+\t\tresult = drop_stash(argc, argv, prefix);\n \telse {\n \t\terror(_(\"unknown subcommand: %s\"), argv[0]);\n \t\tusage_with_options(git_stash_helper_usage, options);\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 0b5d1f3743..0b8f07b38a 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -652,7 +652,7 @@ apply)\n \t;;\n clear)\n \tshift\n-\tclear_stash \"$@\"\n+\tgit stash--helper clear \"$@\"\n \t;;\n create)\n \tshift\n@@ -664,7 +664,7 @@ store)\n \t;;\n drop)\n \tshift\n-\tdrop_stash \"$@\"\n+\tgit stash--helper drop \"$@\"\n \t;;\n pop)\n \tshift\n-- \n2.16.2\n\n"},{"id":"342954","messageId":"20180326011426.19159-6-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 5/6] stash: convert branch to builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:25Z","receivedAt":"2018-03-26T01:14:50Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n builtin/stash--helper.c | 47 +++++++++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            | 17 ++---------------\n 2 files changed, 49 insertions(+), 15 deletions(-)\n\ndiff --git a/builtin/stash--helper.c b/builtin/stash--helper.c\nindex 640de6d0aa..4226c13be3 100644\n--- a/builtin/stash--helper.c\n+++ b/builtin/stash--helper.c\n@@ -13,6 +13,7 @@\n static const char * const git_stash_helper_usage[] = {\n \tN_(\"git stash--helper drop [-q|--quiet] [<stash>]\"),\n \tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n+\tN_(\"git stash--helper branch <branchname> [<stash>]\"),\n \tN_(\"git stash--helper clear\"),\n \tNULL\n };\n@@ -27,6 +28,11 @@ static const char * const git_stash_helper_apply_usage[] = {\n \tNULL\n };\n \n+static const char * const git_stash_helper_branch_usage[] = {\n+\tN_(\"git stash--helper branch <branchname> [<stash>]\"),\n+\tNULL\n+};\n+\n static const char * const git_stash_helper_clear_usage[] = {\n \tN_(\"git stash--helper clear\"),\n \tNULL\n@@ -496,6 +502,45 @@ static int drop_stash(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n+static int branch_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tconst char *branch = NULL;\n+\tint ret;\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\tstruct stash_info info;\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tgit_stash_helper_branch_usage, 0);\n+\n+\tif (argc == 0)\n+\t\treturn error(_(\"No branch name specified\"));\n+\n+\tbranch = argv[0];\n+\n+\tif (get_stash_info(&info, argc - 1, argv + 1))\n+\t\treturn -1;\n+\n+\targv_array_pushl(&args, \"checkout\", \"-b\", NULL);\n+\targv_array_push(&args, branch);\n+\targv_array_push(&args, oid_to_hex(&info.b_commit));\n+\tret = cmd_checkout(args.argc, args.argv, prefix);\n+\tif (ret) {\n+\t\tdestroy_stash_info(&info);\n+\t\treturn -1;\n+\t}\n+\n+\tret = do_apply_stash(prefix, &info, 1);\n+\tif (!ret && info.is_stash_ref)\n+\t\tret = do_drop_stash(prefix, &info);\n+\n+\tdestroy_stash_info(&info);\n+\n+\treturn ret;\n+}\n+\n int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n {\n \tint result = 0;\n@@ -522,6 +567,8 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n \t\tresult = clear_stash(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"drop\"))\n \t\tresult = drop_stash(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"branch\"))\n+\t\tresult = branch_stash(argc, argv, prefix);\n \telse {\n \t\terror(_(\"unknown subcommand: %s\"), argv[0]);\n \t\tusage_with_options(git_stash_helper_usage, options);\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 0b8f07b38a..c5fd4c6c44 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -598,20 +598,6 @@ drop_stash () {\n \tclear_stash\n }\n \n-apply_to_branch () {\n-\ttest -n \"$1\" || die \"$(gettext \"No branch name specified\")\"\n-\tbranch=$1\n-\tshift 1\n-\n-\tset -- --index \"$@\"\n-\tassert_stash_like \"$@\"\n-\n-\tgit checkout -b $branch $REV^ &&\n-\tapply_stash \"$@\" && {\n-\t\ttest -z \"$IS_STASH_REF\" || drop_stash \"$@\"\n-\t}\n-}\n-\n test \"$1\" = \"-p\" && set \"push\" \"$@\"\n \n PARSE_CACHE='--not-parsed'\n@@ -672,7 +658,8 @@ pop)\n \t;;\n branch)\n \tshift\n-\tapply_to_branch \"$@\"\n+\tcd \"$START_DIR\"\n+\tgit stash--helper branch \"$@\"\n \t;;\n *)\n \tcase $# in\n-- \n2.16.2\n\n"},{"id":"342955","messageId":"20180326011426.19159-7-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 6/6] stash: convert pop to builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:26Z","receivedAt":"2018-03-26T01:14:54Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n builtin/stash--helper.c | 41 +++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            | 39 ++++-----------------------------------\n 2 files changed, 45 insertions(+), 35 deletions(-)\n\ndiff --git a/builtin/stash--helper.c b/builtin/stash--helper.c\nindex 4226c13be3..36e7e660a0 100644\n--- a/builtin/stash--helper.c\n+++ b/builtin/stash--helper.c\n@@ -12,6 +12,7 @@\n \n static const char * const git_stash_helper_usage[] = {\n \tN_(\"git stash--helper drop [-q|--quiet] [<stash>]\"),\n+\tN_(\"git stash--helper pop [--index] [-q|--quiet] [<stash>]\"),\n \tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n \tN_(\"git stash--helper branch <branchname> [<stash>]\"),\n \tN_(\"git stash--helper clear\"),\n@@ -23,6 +24,11 @@ static const char * const git_stash_helper_drop_usage[] = {\n \tNULL\n };\n \n+static const char * const git_stash_helper_pop_usage[] = {\n+\tN_(\"git stash--helper pop [--index] [-q|--quiet] [<stash>]\"),\n+\tNULL\n+};\n+\n static const char * const git_stash_helper_apply_usage[] = {\n \tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n \tNULL\n@@ -502,6 +508,39 @@ static int drop_stash(int argc, const char **argv, const char *prefix)\n \treturn ret;\n }\n \n+static int pop_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tint index = 0, ret;\n+\tstruct stash_info info;\n+\tstruct option options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n+\t\tOPT_BOOL(0, \"index\", &index,\n+\t\t\tN_(\"attempt to recreate the index\")),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tgit_stash_helper_pop_usage, 0);\n+\n+\tif (get_stash_info(&info, argc, argv))\n+\t\treturn -1;\n+\n+\tif (assert_stash_ref(&info)) {\n+\t\tdestroy_stash_info(&info);\n+\t\treturn -1;\n+\t}\n+\n+\tif (do_apply_stash(prefix, &info, index)) {\n+\t\tprintf_ln(_(\"The stash entry is kept in case you need it again.\"));\n+\t\tdestroy_stash_info(&info);\n+\t\treturn -1;\n+\t}\n+\n+\tret = do_drop_stash(prefix, &info);\n+\tdestroy_stash_info(&info);\n+\treturn ret;\n+}\n+\n static int branch_stash(int argc, const char **argv, const char *prefix)\n {\n \tconst char *branch = NULL;\n@@ -567,6 +606,8 @@ int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n \t\tresult = clear_stash(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"drop\"))\n \t\tresult = drop_stash(argc, argv, prefix);\n+\telse if (!strcmp(argv[0], \"pop\"))\n+\t\tresult = pop_stash(argc, argv, prefix);\n \telse if (!strcmp(argv[0], \"branch\"))\n \t\tresult = branch_stash(argc, argv, prefix);\n \telse {\ndiff --git a/git-stash.sh b/git-stash.sh\nindex c5fd4c6c44..1b8c7c4f63 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -565,39 +565,6 @@ assert_stash_ref() {\n \t}\n }\n \n-apply_stash () {\n-\tcd \"$START_DIR\"\n-\tgit stash--helper apply \"$@\"\n-\tres=$?\n-\tcd_to_toplevel\n-\treturn $res\n-}\n-\n-pop_stash() {\n-\tassert_stash_ref \"$@\"\n-\n-\tif apply_stash \"$@\"\n-\tthen\n-\t\tdrop_stash \"$@\"\n-\telse\n-\t\tstatus=$?\n-\t\tsay \"$(gettext \"The stash entry is kept in case you need it again.\")\"\n-\t\texit $status\n-\tfi\n-}\n-\n-drop_stash () {\n-\tassert_stash_ref \"$@\"\n-\n-\tgit reflog delete --updateref --rewrite \"${REV}\" &&\n-\t\tsay \"$(eval_gettext \"Dropped \\${REV} (\\$s)\")\" ||\n-\t\tdie \"$(eval_gettext \"\\${REV}: Could not drop stash entry\")\"\n-\n-\t# clear_stash if we just dropped the last stash entry\n-\tgit rev-parse --verify --quiet \"$ref_stash@{0}\" >/dev/null ||\n-\tclear_stash\n-}\n-\n test \"$1\" = \"-p\" && set \"push\" \"$@\"\n \n PARSE_CACHE='--not-parsed'\n@@ -634,7 +601,8 @@ push)\n \t;;\n apply)\n \tshift\n-\tapply_stash \"$@\"\n+\tcd \"$START_DIR\"\n+\tgit stash--helper apply \"$@\"\n \t;;\n clear)\n \tshift\n@@ -654,7 +622,8 @@ drop)\n \t;;\n pop)\n \tshift\n-\tpop_stash \"$@\"\n+\tcd \"$START_DIR\"\n+\tgit stash--helper pop \"$@\"\n \t;;\n branch)\n \tshift\n-- \n2.16.2\n\n"},{"id":"342956","messageId":"20180326011426.19159-4-joel@teichroeb.net","threadId":"48146","inReplyTo":"20180326011426.19159-1-joel@teichroeb.net","subject":"[PATCH v2 3/6] stash: convert apply to builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-26T01:14:23Z","receivedAt":"2018-03-26T01:15:00Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n---\n .gitignore              |   1 +\n Makefile                |   1 +\n builtin.h               |   1 +\n builtin/stash--helper.c | 431 ++++++++++++++++++++++++++++++++++++++++++++++++\n git-stash.sh            |  75 +--------\n git.c                   |   1 +\n 6 files changed, 440 insertions(+), 70 deletions(-)\n create mode 100644 builtin/stash--helper.c\n\ndiff --git a/.gitignore b/.gitignore\nindex 833ef3b0b7..296d5f376d 100644\n--- a/.gitignore\n+++ b/.gitignore\n@@ -152,6 +152,7 @@\n /git-show-ref\n /git-stage\n /git-stash\n+/git-stash--helper\n /git-status\n /git-stripspace\n /git-submodule\ndiff --git a/Makefile b/Makefile\nindex 96f6138f63..6cfdbe9a32 100644\n--- a/Makefile\n+++ b/Makefile\n@@ -1028,6 +1028,7 @@ BUILTIN_OBJS += builtin/send-pack.o\n BUILTIN_OBJS += builtin/shortlog.o\n BUILTIN_OBJS += builtin/show-branch.o\n BUILTIN_OBJS += builtin/show-ref.o\n+BUILTIN_OBJS += builtin/stash--helper.o\n BUILTIN_OBJS += builtin/stripspace.o\n BUILTIN_OBJS += builtin/submodule--helper.o\n BUILTIN_OBJS += builtin/symbolic-ref.o\ndiff --git a/builtin.h b/builtin.h\nindex 42378f3aa4..a14fd85b0e 100644\n--- a/builtin.h\n+++ b/builtin.h\n@@ -219,6 +219,7 @@ extern int cmd_shortlog(int argc, const char **argv, const char *prefix);\n extern int cmd_show(int argc, const char **argv, const char *prefix);\n extern int cmd_show_branch(int argc, const char **argv, const char *prefix);\n extern int cmd_status(int argc, const char **argv, const char *prefix);\n+extern int cmd_stash__helper(int argc, const char **argv, const char *prefix);\n extern int cmd_stripspace(int argc, const char **argv, const char *prefix);\n extern int cmd_submodule__helper(int argc, const char **argv, const char *prefix);\n extern int cmd_symbolic_ref(int argc, const char **argv, const char *prefix);\ndiff --git a/builtin/stash--helper.c b/builtin/stash--helper.c\nnew file mode 100644\nindex 0000000000..050de415b4\n--- /dev/null\n+++ b/builtin/stash--helper.c\n@@ -0,0 +1,431 @@\n+#include \"builtin.h\"\n+#include \"config.h\"\n+#include \"parse-options.h\"\n+#include \"refs.h\"\n+#include \"lockfile.h\"\n+#include \"cache-tree.h\"\n+#include \"unpack-trees.h\"\n+#include \"merge-recursive.h\"\n+#include \"argv-array.h\"\n+#include \"run-command.h\"\n+#include \"dir.h\"\n+\n+static const char * const git_stash_helper_usage[] = {\n+\tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n+\tNULL\n+};\n+\n+static const char * const git_stash_helper_apply_usage[] = {\n+\tN_(\"git stash--helper apply [--index] [-q|--quiet] [<stash>]\"),\n+\tNULL\n+};\n+\n+static const char *ref_stash = \"refs/stash\";\n+static int quiet;\n+static char stash_index_path[PATH_MAX];\n+\n+struct stash_info {\n+\tstruct object_id w_commit;\n+\tstruct object_id b_commit;\n+\tstruct object_id i_commit;\n+\tstruct object_id u_commit;\n+\tstruct object_id w_tree;\n+\tstruct object_id b_tree;\n+\tstruct object_id i_tree;\n+\tstruct object_id u_tree;\n+\tstruct strbuf revision;\n+\tint is_stash_ref;\n+\tint has_u;\n+};\n+\n+static int get_symbolic_name(const char *symbolic, struct strbuf *out)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\tcp.git_cmd = 1;\n+\targv_array_pushl(&cp.args, \"rev-parse\", \"--symbolic-full-name\", NULL);\n+\targv_array_pushf(&cp.args, \"%s\", symbolic);\n+\treturn pipe_command(&cp, NULL, 0, out, 0, NULL, 0);\n+}\n+\n+static int have_stash(void)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\n+\tcp.git_cmd = 1;\n+\tcp.no_stdout = 1;\n+\targv_array_pushl(&cp.args, \"rev-parse\", \"--verify\", \"--quiet\", NULL);\n+\targv_array_pushf(&cp.args, \"%s\", ref_stash);\n+\treturn pipe_command(&cp, NULL, 0, NULL, 0, NULL, 0);\n+}\n+\n+static void destroy_stash_info(struct stash_info *info)\n+{\n+\tstrbuf_release(&info->revision);\n+}\n+\n+static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n+{\n+\tstruct strbuf w_commit_rev = STRBUF_INIT;\n+\tstruct strbuf b_commit_rev = STRBUF_INIT;\n+\tstruct strbuf w_tree_rev = STRBUF_INIT;\n+\tstruct strbuf b_tree_rev = STRBUF_INIT;\n+\tstruct strbuf i_tree_rev = STRBUF_INIT;\n+\tstruct strbuf u_tree_rev = STRBUF_INIT;\n+\tstruct strbuf symbolic = STRBUF_INIT;\n+\tstruct strbuf out = STRBUF_INIT;\n+\tint ret;\n+\tconst char *revision;\n+\tconst char *commit = NULL;\n+\tchar *end_of_rev;\n+\tinfo->is_stash_ref = 0;\n+\n+\tif (argc > 1) {\n+\t\tint i;\n+\t\tfprintf(stderr, _(\"Too many revisions specified:\"));\n+\t\tfor (i = 0; i < argc; ++i) {\n+\t\t\tfprintf(stderr, \" '%s'\", argv[i]);\n+\t\t}\n+\t\tfprintf(stderr, \"\\n\");\n+\n+\t\treturn -1;\n+\t}\n+\n+\tif (argc == 1)\n+\t\tcommit = argv[0];\n+\n+\tstrbuf_init(&info->revision, 0);\n+\tif (commit == NULL) {\n+\t\tif (have_stash()) {\n+\t\t\tdestroy_stash_info(info);\n+\t\t\treturn error(_(\"No stash entries found.\"));\n+\t\t}\n+\n+\t\tstrbuf_addf(&info->revision, \"%s@{0}\", ref_stash);\n+\t} else if (strspn(commit, \"0123456789\") == strlen(commit)) {\n+\t\tstrbuf_addf(&info->revision, \"%s@{%s}\", ref_stash, commit);\n+\t} else {\n+\t\tstrbuf_addstr(&info->revision, commit);\n+\t}\n+\n+\trevision = info->revision.buf;\n+\n+\tstrbuf_addf(&w_commit_rev, \"%s\", revision);\n+\n+\n+\tret = !get_oid(w_commit_rev.buf, &info->w_commit);\n+\n+\tstrbuf_release(&w_commit_rev);\n+\n+\tif (!ret) {\n+\t\tdestroy_stash_info(info);\n+\t\treturn error(_(\"%s is not a valid reference\"), revision);\n+\t}\n+\n+\tstrbuf_addf(&b_commit_rev, \"%s^1\", revision);\n+\tstrbuf_addf(&w_tree_rev, \"%s:\", revision);\n+\tstrbuf_addf(&b_tree_rev, \"%s^1:\", revision);\n+\tstrbuf_addf(&i_tree_rev, \"%s^2:\", revision);\n+\n+\tret = !get_oid(b_commit_rev.buf, &info->b_commit) &&\n+\t\t!get_oid(w_tree_rev.buf, &info->w_tree) &&\n+\t\t!get_oid(b_tree_rev.buf, &info->b_tree) &&\n+\t\t!get_oid(i_tree_rev.buf, &info->i_tree);\n+\n+\tstrbuf_release(&b_commit_rev);\n+\tstrbuf_release(&w_tree_rev);\n+\tstrbuf_release(&b_tree_rev);\n+\tstrbuf_release(&i_tree_rev);\n+\n+\tif (!ret) {\n+\t\tdestroy_stash_info(info);\n+\t\treturn error(_(\"'%s' is not a stash-like commit\"), revision);\n+\t}\n+\n+\tstrbuf_addf(&u_tree_rev, \"%s^3:\", revision);\n+\n+\tinfo->has_u = !get_oid(u_tree_rev.buf, &info->u_tree);\n+\n+\tstrbuf_release(&u_tree_rev);\n+\n+\tend_of_rev = strchrnul(revision, '@');\n+\tstrbuf_add(&symbolic, revision, end_of_rev - revision);\n+\n+\tret = get_symbolic_name(symbolic.buf, &out);\n+\tstrbuf_release(&symbolic);\n+\tif (ret) {\n+\t\tdestroy_stash_info(info);\n+\t\tstrbuf_release(&out);\n+\t\treturn -1;\n+\t}\n+\n+\tif (out.len - 1 == strlen(ref_stash))\n+\t\tinfo->is_stash_ref = !strncmp(out.buf, ref_stash, out.len - 1);\n+\tstrbuf_release(&out);\n+\n+\treturn 0;\n+}\n+\n+static int reset_tree(struct object_id *i_tree, int update, int reset)\n+{\n+\tstruct unpack_trees_options opts;\n+\tint nr_trees = 1;\n+\tstruct tree_desc t[MAX_UNPACK_TREES];\n+\tstruct tree *tree;\n+\tstruct lock_file lock_file = LOCK_INIT;\n+\n+\tread_cache_preload(NULL);\n+\tif (refresh_cache(REFRESH_QUIET))\n+\t\treturn -1;\n+\n+\thold_locked_index(&lock_file, LOCK_DIE_ON_ERROR);\n+\n+\tmemset(&opts, 0, sizeof(opts));\n+\n+\ttree = parse_tree_indirect(i_tree);\n+\tif (parse_tree(tree))\n+\t\treturn -1;\n+\n+\tinit_tree_desc(t, tree->buffer, tree->size);\n+\n+\topts.head_idx = 1;\n+\topts.src_index = &the_index;\n+\topts.dst_index = &the_index;\n+\topts.merge = 1;\n+\topts.reset = reset;\n+\topts.update = update;\n+\topts.fn = oneway_merge;\n+\n+\tif (unpack_trees(nr_trees, t, &opts))\n+\t\treturn -1;\n+\n+\tif (write_locked_index(&the_index, &lock_file, COMMIT_LOCK))\n+\t\treturn error(_(\"unable to write new index file\"));\n+\n+\treturn 0;\n+}\n+\n+static int diff_tree_binary(struct strbuf *out, struct object_id *w_commit)\n+{\n+    struct child_process cp = CHILD_PROCESS_INIT;\n+    const char *w_commit_hex = oid_to_hex(w_commit);\n+\n+    cp.git_cmd = 1;\n+    argv_array_pushl(&cp.args, \"diff-tree\", \"--binary\", NULL);\n+    argv_array_pushf(&cp.args, \"%s^2^..%s^2\", w_commit_hex, w_commit_hex);\n+\n+    return pipe_command(&cp, NULL, 0, out, 0, NULL, 0);\n+}\n+\n+static int apply_cached(struct strbuf *out)\n+{\n+    struct child_process cp = CHILD_PROCESS_INIT;\n+\n+    cp.git_cmd = 1;\n+    argv_array_pushl(&cp.args, \"apply\", \"--cached\", NULL);\n+    return pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);\n+}\n+\n+static int reset_head(const char *prefix)\n+{\n+\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\n+\targv_array_push(&args, \"reset\");\n+\treturn cmd_reset(args.argc, args.argv, prefix);\n+}\n+\n+static int diff_cached_index(struct strbuf *out, struct object_id *c_tree)\n+{\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+    const char *c_tree_hex = oid_to_hex(c_tree);\n+\n+\tcp.git_cmd = 1;\n+\targv_array_pushl(&cp.args, \"diff-index\", \"--cached\", \"--name-only\", \"--diff-filter=A\", NULL);\n+\targv_array_push(&cp.args, c_tree_hex);\n+\treturn pipe_command(&cp, NULL, 0, out, 0, NULL, 0);\n+}\n+\n+static int update_index(struct strbuf *out) {\n+\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\tcp.git_cmd = 1;\n+\targv_array_pushl(&cp.args, \"update-index\", \"--add\", \"--stdin\", NULL);\n+\treturn pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);\n+}\n+\n+static int do_apply_stash(const char *prefix, struct stash_info *info, int index)\n+{\n+\tstruct merge_options o;\n+\tstruct object_id c_tree;\n+\tstruct object_id index_tree;\n+\tconst struct object_id *bases[1];\n+\tint bases_count = 1;\n+\tstruct commit *result;\n+\tint ret;\n+\tint has_index = index;\n+\n+\tread_cache_preload(NULL);\n+\tif (refresh_cache(REFRESH_QUIET))\n+\t\treturn -1;\n+\n+\tif (write_cache_as_tree(&c_tree, 0, NULL) || reset_tree(&c_tree, 0, 0))\n+\t\treturn error(_(\"Cannot apply a stash in the middle of a merge\"));\n+\n+\tif (index) {\n+\t\tif (!oidcmp(&info->b_tree, &info->i_tree) || !oidcmp(&c_tree, &info->i_tree)) {\n+\t\t\thas_index = 0;\n+\t\t} else {\n+\t\t\tstruct strbuf out = STRBUF_INIT;\n+\n+\t\t\tif (diff_tree_binary(&out, &info->w_commit)) {\n+\t\t\t\tstrbuf_release(&out);\n+\t\t\t\treturn -1;\n+\t\t\t}\n+\n+\t\t\tret = apply_cached(&out);\n+\t\t\tstrbuf_release(&out);\n+\t\t\tif (ret)\n+\t\t\t\treturn -1;\n+\n+\t\t\tdiscard_cache();\n+\t\t\tread_cache();\n+\t\t\tif (write_cache_as_tree(&index_tree, 0, NULL))\n+\t\t\t\treturn -1;\n+\n+\t\t\treset_head(prefix);\n+\t\t}\n+\t}\n+\n+\tif (info->has_u) {\n+\t\tstruct child_process cp = CHILD_PROCESS_INIT;\n+\t\tstruct child_process cp2 = CHILD_PROCESS_INIT;\n+\t\tint res;\n+\n+\t\tcp.git_cmd = 1;\n+\t\targv_array_push(&cp.args, \"read-tree\");\n+\t\targv_array_push(&cp.args, oid_to_hex(&info->u_tree));\n+\t\targv_array_pushf(&cp.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n+\n+\t\tcp2.git_cmd = 1;\n+\t\targv_array_pushl(&cp2.args, \"checkout-index\", \"--all\", NULL);\n+\t\targv_array_pushf(&cp2.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n+\n+\t\tres = run_command(&cp) || run_command(&cp2);\n+\t\tremove_path(stash_index_path);\n+\t\tif (res)\n+\t\t\treturn error(_(\"Could not restore untracked files from stash\"));\n+\t}\n+\n+\tinit_merge_options(&o);\n+\n+\to.branch1 = \"Updated upstream\";\n+\to.branch2 = \"Stashed changes\";\n+\n+\tif (!oidcmp(&info->b_tree, &c_tree))\n+\t\to.branch1 = \"Version stash was based on\";\n+\n+\tif (quiet)\n+\t\to.verbosity = 0;\n+\n+\tif (o.verbosity >= 3)\n+\t\tprintf_ln(_(\"Merging %s with %s\"), o.branch1, o.branch2);\n+\n+\tbases[0] = &info->b_tree;\n+\n+\tret = merge_recursive_generic(&o, &c_tree, &info->w_tree, bases_count, bases, &result);\n+\tif (ret != 0) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\t\targv_array_push(&args, \"rerere\");\n+\t\tcmd_rerere(args.argc, args.argv, prefix);\n+\n+\t\tif (index)\n+\t\t\tfprintf_ln(stderr, _(\"Index was not unstashed.\"));\n+\n+\t\treturn ret;\n+\t}\n+\n+\tif (has_index) {\n+\t\tif (reset_tree(&index_tree, 0, 0))\n+\t\t\treturn -1;\n+\t} else {\n+\t\tstruct strbuf out = STRBUF_INIT;\n+\n+\t\tif (diff_cached_index(&out, &c_tree)) {\n+\t\t\tstrbuf_release(&out);\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tif (reset_tree(&c_tree, 0, 1)) {\n+\t\t\tstrbuf_release(&out);\n+\t\t\treturn -1;\n+\t\t}\n+\n+\t\tret = update_index(&out);\n+\t\tstrbuf_release(&out);\n+\t\tif (ret)\n+\t\t\treturn -1;\n+\n+\t\tdiscard_cache();\n+\t}\n+\n+\tif (!quiet) {\n+\t\tstruct argv_array args = ARGV_ARRAY_INIT;\n+\t\targv_array_push(&args, \"status\");\n+\t\tcmd_status(args.argc, args.argv, prefix);\n+\t}\n+\n+\treturn 0;\n+}\n+\n+static int apply_stash(int argc, const char **argv, const char *prefix)\n+{\n+\tint index = 0;\n+\tstruct stash_info info;\n+\tint ret;\n+\tstruct option options[] = {\n+\t\tOPT__QUIET(&quiet, N_(\"be quiet, only report errors\")),\n+\t\tOPT_BOOL(0, \"index\", &index,\n+\t\t\tN_(\"attempt to recreate the index\")),\n+\t\tOPT_END()\n+\t};\n+\n+\targc = parse_options(argc, argv, prefix, options,\n+\t\t\tgit_stash_helper_apply_usage, 0);\n+\n+\tif (get_stash_info(&info, argc, argv))\n+\t\treturn -1;\n+\n+\tret = do_apply_stash(prefix, &info, index);\n+\tdestroy_stash_info(&info);\n+\treturn ret;\n+}\n+\n+int cmd_stash__helper(int argc, const char **argv, const char *prefix)\n+{\n+\tint result = 0;\n+\tpid_t pid = getpid();\n+\tconst char *index_file;\n+\n+\tstruct option options[] = {\n+\t\tOPT_END()\n+\t};\n+\n+\tgit_config(git_default_config, NULL);\n+\n+\targc = parse_options(argc, argv, prefix, options, git_stash_helper_usage,\n+\t\tPARSE_OPT_KEEP_UNKNOWN|PARSE_OPT_KEEP_DASHDASH);\n+\n+\tindex_file = get_index_file();\n+\txsnprintf(stash_index_path, PATH_MAX, \"%s.stash.%d\", index_file, pid);\n+\n+\tif (argc < 1)\n+\t\tusage_with_options(git_stash_helper_usage, options);\n+\telse if (!strcmp(argv[0], \"apply\"))\n+\t\tresult = apply_stash(argc, argv, prefix);\n+\telse {\n+\t\terror(_(\"unknown subcommand: %s\"), argv[0]);\n+\t\tusage_with_options(git_stash_helper_usage, options);\n+\t\tresult = 1;\n+\t}\n+\n+\treturn result;\n+}\ndiff --git a/git-stash.sh b/git-stash.sh\nindex 94793c1a91..0b5d1f3743 100755\n--- a/git-stash.sh\n+++ b/git-stash.sh\n@@ -566,76 +566,11 @@ assert_stash_ref() {\n }\n \n apply_stash () {\n-\n-\tassert_stash_like \"$@\"\n-\n-\tgit update-index -q --refresh || die \"$(gettext \"unable to refresh index\")\"\n-\n-\t# current index state\n-\tc_tree=$(git write-tree) ||\n-\t\tdie \"$(gettext \"Cannot apply a stash in the middle of a merge\")\"\n-\n-\tunstashed_index_tree=\n-\tif test -n \"$INDEX_OPTION\" && test \"$b_tree\" != \"$i_tree\" &&\n-\t\t\ttest \"$c_tree\" != \"$i_tree\"\n-\tthen\n-\t\tgit diff-tree --binary $s^2^..$s^2 | git apply --cached\n-\t\ttest $? -ne 0 &&\n-\t\t\tdie \"$(gettext \"Conflicts in index. Try without --index.\")\"\n-\t\tunstashed_index_tree=$(git write-tree) ||\n-\t\t\tdie \"$(gettext \"Could not save index tree\")\"\n-\t\tgit reset\n-\tfi\n-\n-\tif test -n \"$u_tree\"\n-\tthen\n-\t\tGIT_INDEX_FILE=\"$TMPindex\" git read-tree \"$u_tree\" &&\n-\t\tGIT_INDEX_FILE=\"$TMPindex\" git checkout-index --all &&\n-\t\trm -f \"$TMPindex\" ||\n-\t\tdie \"$(gettext \"Could not restore untracked files from stash entry\")\"\n-\tfi\n-\n-\teval \"\n-\t\tGITHEAD_$w_tree='Stashed changes' &&\n-\t\tGITHEAD_$c_tree='Updated upstream' &&\n-\t\tGITHEAD_$b_tree='Version stash was based on' &&\n-\t\texport GITHEAD_$w_tree GITHEAD_$c_tree GITHEAD_$b_tree\n-\t\"\n-\n-\tif test -n \"$GIT_QUIET\"\n-\tthen\n-\t\tGIT_MERGE_VERBOSITY=0 && export GIT_MERGE_VERBOSITY\n-\tfi\n-\tif git merge-recursive $b_tree -- $c_tree $w_tree\n-\tthen\n-\t\t# No conflict\n-\t\tif test -n \"$unstashed_index_tree\"\n-\t\tthen\n-\t\t\tgit read-tree \"$unstashed_index_tree\"\n-\t\telse\n-\t\t\ta=\"$TMP-added\" &&\n-\t\t\tgit diff-index --cached --name-only --diff-filter=A $c_tree >\"$a\" &&\n-\t\t\tgit read-tree --reset $c_tree &&\n-\t\t\tgit update-index --add --stdin <\"$a\" ||\n-\t\t\t\tdie \"$(gettext \"Cannot unstage modified files\")\"\n-\t\t\trm -f \"$a\"\n-\t\tfi\n-\t\tsquelch=\n-\t\tif test -n \"$GIT_QUIET\"\n-\t\tthen\n-\t\t\tsquelch='>/dev/null 2>&1'\n-\t\tfi\n-\t\t(cd \"$START_DIR\" && eval \"git status $squelch\") || :\n-\telse\n-\t\t# Merge conflict; keep the exit status from merge-recursive\n-\t\tstatus=$?\n-\t\tgit rerere\n-\t\tif test -n \"$INDEX_OPTION\"\n-\t\tthen\n-\t\t\tgettextln \"Index was not unstashed.\" >&2\n-\t\tfi\n-\t\texit $status\n-\tfi\n+\tcd \"$START_DIR\"\n+\tgit stash--helper apply \"$@\"\n+\tres=$?\n+\tcd_to_toplevel\n+\treturn $res\n }\n \n pop_stash() {\ndiff --git a/git.c b/git.c\nindex ceaa58ef40..6ffe6364ac 100644\n--- a/git.c\n+++ b/git.c\n@@ -466,6 +466,7 @@ static struct cmd_struct commands[] = {\n \t{ \"show-branch\", cmd_show_branch, RUN_SETUP },\n \t{ \"show-ref\", cmd_show_ref, RUN_SETUP },\n \t{ \"stage\", cmd_add, RUN_SETUP | NEED_WORK_TREE },\n+\t{ \"stash--helper\", cmd_stash__helper, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"status\", cmd_status, RUN_SETUP | NEED_WORK_TREE },\n \t{ \"stripspace\", cmd_stripspace },\n \t{ \"submodule--helper\", cmd_submodule__helper, RUN_SETUP | SUPPORT_SUPER_PREFIX},\n-- \n2.16.2\n\n"},{"id":"342966","messageId":"CAP8UFD0wc2i3W4FNZPcDs3ELJGJj=4NaP8mhd-zMAkdK=vXT1A@mail.gmail.com","threadId":"48146","inReplyTo":"20180326011426.19159-4-joel@teichroeb.net","subject":"Re: [PATCH v2 3/6] stash: convert apply to builtin","fromName":"Christian Couder","fromEmail":"christian.couder@gmail.com","sentAt":"2018-03-26T07:05:44Z","receivedAt":"2018-03-26T07:05:49Z","isPatch":true,"sender":{"key":"christian.couder@gmail.com","avatar":"https://avatars.githubusercontent.com/u/208954?v=4"},"body":"On Mon, Mar 26, 2018 at 3:14 AM, Joel Teichroeb <joel@teichroeb.net> wrote:\n> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n\nThe commit message in this patch and the following ones could be a bit\nmore verbose. It could at least tell that the end goal is to convert\ngit-stash.sh to a C builtin.\n\n> +static void destroy_stash_info(struct stash_info *info)\n> +{\n> +       strbuf_release(&info->revision);\n> +}\n\nNot sure if \"destroy\" is the right word in the function name. I would\nhave used \"free\" instead.\n\n> +static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n> +{\n> +       struct strbuf w_commit_rev = STRBUF_INIT;\n> +       struct strbuf b_commit_rev = STRBUF_INIT;\n> +       struct strbuf w_tree_rev = STRBUF_INIT;\n> +       struct strbuf b_tree_rev = STRBUF_INIT;\n> +       struct strbuf i_tree_rev = STRBUF_INIT;\n> +       struct strbuf u_tree_rev = STRBUF_INIT;\n> +       struct strbuf symbolic = STRBUF_INIT;\n> +       struct strbuf out = STRBUF_INIT;\n> +       int ret;\n> +       const char *revision;\n> +       const char *commit = NULL;\n> +       char *end_of_rev;\n> +       info->is_stash_ref = 0;\n> +\n> +       if (argc > 1) {\n> +               int i;\n> +               fprintf(stderr, _(\"Too many revisions specified:\"));\n> +               for (i = 0; i < argc; ++i) {\n> +                       fprintf(stderr, \" '%s'\", argv[i]);\n> +               }\n\nThe brackets are not needed.\n\n> +               fprintf(stderr, \"\\n\");\n> +\n> +               return -1;\n> +       }\n> +\n> +       if (argc == 1)\n> +               commit = argv[0];\n> +\n> +       strbuf_init(&info->revision, 0);\n> +       if (commit == NULL) {\n> +               if (have_stash()) {\n> +                       destroy_stash_info(info);\n> +                       return error(_(\"No stash entries found.\"));\n> +               }\n> +\n> +               strbuf_addf(&info->revision, \"%s@{0}\", ref_stash);\n> +       } else if (strspn(commit, \"0123456789\") == strlen(commit)) {\n> +               strbuf_addf(&info->revision, \"%s@{%s}\", ref_stash, commit);\n> +       } else {\n> +               strbuf_addstr(&info->revision, commit);\n> +       }\n> +\n> +       revision = info->revision.buf;\n> +\n> +       strbuf_addf(&w_commit_rev, \"%s\", revision);\n\nMaybe use strbuf_addstr()?\n\n> +\n> +\n\nSpurious new line.\n\n[...]\n\n> +static int diff_cached_index(struct strbuf *out, struct object_id *c_tree)\n> +{\n> +       struct child_process cp = CHILD_PROCESS_INIT;\n> +    const char *c_tree_hex = oid_to_hex(c_tree);\n\nIndent looks weird.\n\n> +\n> +       cp.git_cmd = 1;\n> +       argv_array_pushl(&cp.args, \"diff-index\", \"--cached\", \"--name-only\", \"--diff-filter=A\", NULL);\n> +       argv_array_push(&cp.args, c_tree_hex);\n> +       return pipe_command(&cp, NULL, 0, out, 0, NULL, 0);\n> +}\n> +\n> +static int update_index(struct strbuf *out) {\n\nThe opening bracket should be on its own line.\n\n> +       struct child_process cp = CHILD_PROCESS_INIT;\n\nMaybe add a new line here to be more consistent with other such functions.\n\n> +       cp.git_cmd = 1;\n> +       argv_array_pushl(&cp.args, \"update-index\", \"--add\", \"--stdin\", NULL);\n> +       return pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);\n> +}\n\n[...]\n\n> +       if (info->has_u) {\n> +               struct child_process cp = CHILD_PROCESS_INIT;\n> +               struct child_process cp2 = CHILD_PROCESS_INIT;\n> +               int res;\n> +\n> +               cp.git_cmd = 1;\n> +               argv_array_push(&cp.args, \"read-tree\");\n> +               argv_array_push(&cp.args, oid_to_hex(&info->u_tree));\n> +               argv_array_pushf(&cp.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n> +\n> +               cp2.git_cmd = 1;\n> +               argv_array_pushl(&cp2.args, \"checkout-index\", \"--all\", NULL);\n> +               argv_array_pushf(&cp2.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n\nMaybe use small functions for the above read-tree and checkout-index.\n\n> +               res = run_command(&cp) || run_command(&cp2);\n> +               remove_path(stash_index_path);\n> +               if (res)\n> +                       return error(_(\"Could not restore untracked files from stash\"));\n> +       }\n\nThanks.\n"},{"id":"343055","messageId":"nycvar.QRO.7.76.6.1803262110000.77@ZVAVAG-6OXH6DA.rhebcr.pbec.zvpebfbsg.pbz","threadId":"48146","inReplyTo":"20180326011426.19159-2-joel@teichroeb.net","subject":"Re: [PATCH v2 1/6] stash: Add tests for passing in too many refs","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2018-03-26T19:21:33Z","receivedAt":"2018-03-26T19:21:45Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"Hi Joel,\n\nOn Sun, 25 Mar 2018, Joel Teichroeb wrote:\n\n> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n\nI could imagine that the commit message would benefit from this body:\n\n\tIn preparation for converting the stash command incrementally to\n\ta builtin command, this patch improves test coverage of the option\n\tparsing.\n\n> diff --git a/t/t3903-stash.sh b/t/t3903-stash.sh\n> index aefde7b172..7146e27bb5 100755\n> --- a/t/t3903-stash.sh\n> +++ b/t/t3903-stash.sh\n> @@ -45,6 +45,12 @@ test_expect_success 'applying bogus stash does nothing' '\n>  \ttest_cmp expect file\n>  '\n>  \n> +test_expect_success 'applying with too many agruments does nothing' '\n> +\ttest_must_fail git stash apply stash@{0} bar &&\n> +\techo 1 >expect &&\n> +\ttest_cmp expect file\n> +'\n\nI suppose you encountered a problem where `stash apply a b` would modify\nthe file?\n\nAnd if you really want to verify that the command does nothing, I guess\nyou will have to use\n\n\ttest-chmtime =123456789 file &&\n\ttest_must_fail git stash apply stash@{0} bar &&\n\ttest 123456789 = $(test-chmtime -v +0 file | sed 's/[^0-9].*$//')\n\n> @@ -97,6 +103,10 @@ test_expect_success 'stash drop complains of extra options' '\n>  \ttest_must_fail git stash drop --foo\n>  '\n>  \n> +test_expect_success 'stash drop complains with too many refs' '\n> +\ttest_must_fail git stash drop stash@{1} stash@{2}\n\nI wonder whether you might want to verify that the error message is\nprinted, e.g. via\n\n\ttest_must_fail git stash drop stash@{1} stash@{2} 2>err &&\n\ttest_i18ngrep \"Too many\" err\n\nAlso, since the added tests look very similar, it might make sense to use\na loop (with fixed revision arguments).\n\nCiao,\nDscho\n"},{"id":"343202","messageId":"CA+CzEk8xDG39Fb5GU2gHDvsPo=xGXxwNCdBOg4D+eQ2wPVaTOg@mail.gmail.com","threadId":"48146","inReplyTo":"CAP8UFD0wc2i3W4FNZPcDs3ELJGJj=4NaP8mhd-zMAkdK=vXT1A@mail.gmail.com","subject":"Re: [PATCH v2 3/6] stash: convert apply to builtin","fromName":"Joel Teichroeb","fromEmail":"joel@teichroeb.net","sentAt":"2018-03-28T03:32:15Z","receivedAt":"2018-03-28T03:32:46Z","isPatch":true,"sender":{"key":"joel@teichroeb.net","avatar":"https://avatars.githubusercontent.com/u/240865?v=4"},"body":"On Mon, Mar 26, 2018 at 12:05 AM, Christian Couder\n<christian.couder@gmail.com> wrote:\n> On Mon, Mar 26, 2018 at 3:14 AM, Joel Teichroeb <joel@teichroeb.net> wrote:\n>> Signed-off-by: Joel Teichroeb <joel@teichroeb.net>\n>\n> The commit message in this patch and the following ones could be a bit\n> more verbose. It could at least tell that the end goal is to convert\n> git-stash.sh to a C builtin.\n>\n>> +static void destroy_stash_info(struct stash_info *info)\n>> +{\n>> +       strbuf_release(&info->revision);\n>> +}\n>\n> Not sure if \"destroy\" is the right word in the function name. I would\n> have used \"free\" instead.\n>\n>> +static int get_stash_info(struct stash_info *info, int argc, const char **argv)\n>> +{\n>> +       struct strbuf w_commit_rev = STRBUF_INIT;\n>> +       struct strbuf b_commit_rev = STRBUF_INIT;\n>> +       struct strbuf w_tree_rev = STRBUF_INIT;\n>> +       struct strbuf b_tree_rev = STRBUF_INIT;\n>> +       struct strbuf i_tree_rev = STRBUF_INIT;\n>> +       struct strbuf u_tree_rev = STRBUF_INIT;\n>> +       struct strbuf symbolic = STRBUF_INIT;\n>> +       struct strbuf out = STRBUF_INIT;\n>> +       int ret;\n>> +       const char *revision;\n>> +       const char *commit = NULL;\n>> +       char *end_of_rev;\n>> +       info->is_stash_ref = 0;\n>> +\n>> +       if (argc > 1) {\n>> +               int i;\n>> +               fprintf(stderr, _(\"Too many revisions specified:\"));\n>> +               for (i = 0; i < argc; ++i) {\n>> +                       fprintf(stderr, \" '%s'\", argv[i]);\n>> +               }\n>\n> The brackets are not needed.\n>\n>> +               fprintf(stderr, \"\\n\");\n>> +\n>> +               return -1;\n>> +       }\n>> +\n>> +       if (argc == 1)\n>> +               commit = argv[0];\n>> +\n>> +       strbuf_init(&info->revision, 0);\n>> +       if (commit == NULL) {\n>> +               if (have_stash()) {\n>> +                       destroy_stash_info(info);\n>> +                       return error(_(\"No stash entries found.\"));\n>> +               }\n>> +\n>> +               strbuf_addf(&info->revision, \"%s@{0}\", ref_stash);\n>> +       } else if (strspn(commit, \"0123456789\") == strlen(commit)) {\n>> +               strbuf_addf(&info->revision, \"%s@{%s}\", ref_stash, commit);\n>> +       } else {\n>> +               strbuf_addstr(&info->revision, commit);\n>> +       }\n>> +\n>> +       revision = info->revision.buf;\n>> +\n>> +       strbuf_addf(&w_commit_rev, \"%s\", revision);\n>\n> Maybe use strbuf_addstr()?\n>\n>> +\n>> +\n>\n> Spurious new line.\n>\n> [...]\n>\n>> +static int diff_cached_index(struct strbuf *out, struct object_id *c_tree)\n>> +{\n>> +       struct child_process cp = CHILD_PROCESS_INIT;\n>> +    const char *c_tree_hex = oid_to_hex(c_tree);\n>\n> Indent looks weird.\n>\n>> +\n>> +       cp.git_cmd = 1;\n>> +       argv_array_pushl(&cp.args, \"diff-index\", \"--cached\", \"--name-only\", \"--diff-filter=A\", NULL);\n>> +       argv_array_push(&cp.args, c_tree_hex);\n>> +       return pipe_command(&cp, NULL, 0, out, 0, NULL, 0);\n>> +}\n>> +\n>> +static int update_index(struct strbuf *out) {\n>\n> The opening bracket should be on its own line.\n>\n>> +       struct child_process cp = CHILD_PROCESS_INIT;\n>\n> Maybe add a new line here to be more consistent with other such functions.\n>\n>> +       cp.git_cmd = 1;\n>> +       argv_array_pushl(&cp.args, \"update-index\", \"--add\", \"--stdin\", NULL);\n>> +       return pipe_command(&cp, out->buf, out->len, NULL, 0, NULL, 0);\n>> +}\n>\n> [...]\n>\n>> +       if (info->has_u) {\n>> +               struct child_process cp = CHILD_PROCESS_INIT;\n>> +               struct child_process cp2 = CHILD_PROCESS_INIT;\n>> +               int res;\n>> +\n>> +               cp.git_cmd = 1;\n>> +               argv_array_push(&cp.args, \"read-tree\");\n>> +               argv_array_push(&cp.args, oid_to_hex(&info->u_tree));\n>> +               argv_array_pushf(&cp.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n>> +\n>> +               cp2.git_cmd = 1;\n>> +               argv_array_pushl(&cp2.args, \"checkout-index\", \"--all\", NULL);\n>> +               argv_array_pushf(&cp2.env_array, \"GIT_INDEX_FILE=%s\", stash_index_path);\n>\n> Maybe use small functions for the above read-tree and checkout-index.\n>\n>> +               res = run_command(&cp) || run_command(&cp2);\n>> +               remove_path(stash_index_path);\n>> +               if (res)\n>> +                       return error(_(\"Could not restore untracked files from stash\"));\n>> +       }\n>\n> Thanks.\n\nThanks for your detailed comments! I've fixed them all in my next patch set.\n"}]}