{"thread":{"id":"10620","subject":"[PATCH] builtin-reset: do not call \"ls-files --unmerged\"","startedAt":"2007-11-03T14:33:01Z","lastAt":"2007-11-03T15:21:21Z","messageCount":2,"participants":["Johannes Schindelin"],"isPatch":true,"patchVersion":1,"patchTotal":null},"messages":[{"id":"58139","messageId":"Pine.LNX.4.64.0711031417440.4362@racer.site","threadId":"10620","inReplyTo":"7v3axlodw9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] builtin-reset: do not call \"ls-files --unmerged\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-03T14:33:01Z","receivedAt":"2007-11-03T14:33:01Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nSince reset is a builtin now, it can use the full power of libgit.a\nand check for unmerged entries itself.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Tue, 11 Sep 2007, Junio C Hamano wrote:\n\n\t> Carlos Rica <jasampler@gmail.com> writes:\n\t> \n\t> > +static int unmerged_files(void)\n\t> > +{\n\t> > [...]\n\t> > +}\n\t> \n\t> Not that git-reset is so performance sensitive, but you could do\n\t> this from built-in without exec, by just reading the index and\n\t> checking if you have a higher-stage entry yourself.\n\n\tHereby done.\n\n builtin-reset.c  |   28 ++++++++--------------------\n t/t7102-reset.sh |    9 +++++++++\n 2 files changed, 17 insertions(+), 20 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 5467e36..79792ee 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -46,26 +46,14 @@ static inline int is_merge(void)\n \n static int unmerged_files(void)\n {\n-\tchar b;\n-\tssize_t len;\n-\tstruct child_process cmd;\n-\tconst char *argv_ls_files[] = {\"ls-files\", \"--unmerged\", NULL};\n-\n-\tmemset(&cmd, 0, sizeof(cmd));\n-\tcmd.argv = argv_ls_files;\n-\tcmd.git_cmd = 1;\n-\tcmd.out = -1;\n-\n-\tif (start_command(&cmd))\n-\t\tdie(\"Could not run sub-command: git ls-files\");\n-\n-\tlen = xread(cmd.out, &b, 1);\n-\tif (len < 0)\n-\t\tdie(\"Could not read output from git ls-files: %s\",\n-\t\t\t\t\t\tstrerror(errno));\n-\tfinish_command(&cmd);\n-\n-\treturn len;\n+\tint i;\n+\tread_cache();\n+\tfor (i = 0; i < active_nr; i++) {\n+\t\tstruct cache_entry *ce = active_cache[i];\n+\t\tif (ce_stage(ce))\n+\t\t\treturn 1;\n+\t}\n+\treturn 0;\n }\n \n static int reset_index_file(const unsigned char *sha1, int is_hard_reset)\ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex cea9afb..506767d 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -59,6 +59,15 @@ test_expect_success 'giving a non existing revision should fail' '\n \tcheck_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc\n '\n \n+test_expect_success 'reset --soft with unmerged index should fail' '\n+\ttouch .git/MERGE_HEAD &&\n+\techo \"100644 44c5b5884550c17758737edcced463447b91d42b 1\tun\" |\n+\t\tgit update-index --index-info &&\n+\t! git reset --soft HEAD &&\n+\trm .git/MERGE_HEAD &&\n+\tgit rm --cached -- un\n+'\n+\n test_expect_success \\\n \t'giving paths with options different than --mixed should fail' '\n \t! git reset --soft -- first &&\n-- \n1.5.3.5.1506.g83995\n"},{"id":"58146","messageId":"Pine.LNX.4.64.0711031437220.4362@racer.site","threadId":"10620","inReplyTo":"7v3axlodw9.fsf@gitster.siamese.dyndns.org","subject":"[PATCH] builtin-reset: avoid forking \"update-index --refresh\"","fromName":"Johannes Schindelin","fromEmail":"johannes.schindelin@gmx.de","sentAt":"2007-11-03T15:21:21Z","receivedAt":"2007-11-03T15:21:21Z","isPatch":true,"sender":{"key":"johannes.schindelin@gmx.de","avatar":"https://avatars.githubusercontent.com/u/127790?v=4"},"body":"\nInstead of forking update-index, call refresh_cache() directly.\n\nSigned-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>\n---\n\n\tOn Tue, 11 Sep 2007, Junio C Hamano wrote:\n\n\t> Carlos Rica <jasampler@gmail.com> writes:\n\t> \n\t> > +static int update_index_refresh(void)\n\t> > +{\n\t> > +\tconst char *argv_update_index[] = {\"update-index\", \"--refresh\", NULL};\n\t> > +\treturn run_command_v_opt(argv_update_index, RUN_GIT_CMD);\n\t> > +}\n\t> \n\t> Instead of making a call to this one after read_from_tree()\n\t> returns, immediately before writing the index out at the end of\n\t> read_from_tree(), you have the index in core.  I think you can\n\t> call read-cache.c::refresh_index() at that point and then do the\n\t> write_cache(), no?\n\n\tA little more complicated now, since this is (obviously) on top of \n\tthe bug fix I sent out earlier.\n\n\tI just realised that it partially undoes the changes of that \n\tcommit.  If you want to, I will redo this mini-series, changing \n\tthe first commit to something more similar to this patch.\n\n builtin-reset.c  |   53 +++++++++++++++++++++++++++++------------------------\n t/t7102-reset.sh |   10 ++++++++++\n 2 files changed, 39 insertions(+), 24 deletions(-)\n\ndiff --git a/builtin-reset.c b/builtin-reset.c\nindex 79792ee..44582f2 100644\n--- a/builtin-reset.c\n+++ b/builtin-reset.c\n@@ -95,26 +95,34 @@ static void print_new_head_line(struct commit *commit)\n \t\tprintf(\"\\n\");\n }\n \n-static int update_index_refresh(void)\n+static int update_index_refresh(int fd, struct lock_file *index_lock)\n {\n-\tconst char *argv_update_index[] = {\"update-index\", \"--refresh\", NULL};\n-\treturn run_command_v_opt(argv_update_index, RUN_GIT_CMD);\n-}\n+\tint result;\n+\n+\tif (!index_lock) {\n+\t\tindex_lock = xcalloc(1, sizeof(struct lock_file));\n+\t\tfd = hold_locked_index(index_lock, 1);\n+\t}\n \n-struct update_cb_data {\n-\tint index_fd;\n-\tstruct lock_file *lock;\n-\tint exit_code;\n-};\n+\tif (read_cache() < 0)\n+\t\treturn error(\"Could not read index\");\n+\tresult = refresh_cache(0) ? 1 : 0;\n+\tif (write_cache(fd, active_cache, active_nr) ||\n+\t\t\tclose(fd) ||\n+\t\t\tcommit_locked_index(index_lock))\n+\t\treturn error (\"Could not refresh index\");\n+\treturn result;\n+}\n \n static void update_index_from_diff(struct diff_queue_struct *q,\n \t\tstruct diff_options *opt, void *data)\n {\n \tint i;\n-\tstruct update_cb_data *cb = data;\n+\tint *discard_flag = data;\n \n \t/* do_diff_cache() mangled the index */\n \tdiscard_cache();\n+\t*discard_flag = 1;\n \tread_cache();\n \n \tfor (i = 0; i < q->nr; i++) {\n@@ -128,34 +136,33 @@ static void update_index_from_diff(struct diff_queue_struct *q,\n \t\t} else\n \t\t\tremove_file_from_cache(one->path);\n \t}\n-\n-\tcb->exit_code = write_cache(cb->index_fd, active_cache, active_nr) ||\n-\t\tclose(cb->index_fd) ||\n-\t\tcommit_locked_index(cb->lock);\n }\n \n static int read_from_tree(const char *prefix, const char **argv,\n \t\tunsigned char *tree_sha1)\n {\n+\tstruct lock_file *lock = xcalloc(1, sizeof(struct lock_file));\n+\tint index_fd, index_was_discarded = 0;\n \tstruct diff_options opt;\n-\tstruct update_cb_data cb;\n \n \tmemset(&opt, 0, sizeof(opt));\n \tdiff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);\n \topt.output_format = DIFF_FORMAT_CALLBACK;\n \topt.format_callback = update_index_from_diff;\n-\topt.format_callback_data = &cb;\n+\topt.format_callback_data = &index_was_discarded;\n \n-\tcb.lock = xcalloc(1, sizeof(struct lock_file));\n-\tcb.index_fd = hold_locked_index(cb.lock, 1);\n-\tcb.exit_code = 0;\n+\tindex_fd = hold_locked_index(lock, 1);\n+\tindex_was_discarded = 0;\n \tread_cache();\n \tif (do_diff_cache(tree_sha1, &opt))\n \t\treturn 1;\n \tdiffcore_std(&opt);\n \tdiff_flush(&opt);\n \n-\treturn cb.exit_code;\n+\tif (!index_was_discarded)\n+\t\t/* The index is still clobbered from do_diff_cache() */\n+\t\tdiscard_cache();\n+\treturn update_index_refresh(index_fd, lock);\n }\n \n static void prepend_reflog_action(const char *action, char *buf, size_t size)\n@@ -225,9 +232,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \t\telse if (reset_type != NONE)\n \t\t\tdie(\"Cannot do %s reset with paths.\",\n \t\t\t\t\treset_type_names[reset_type]);\n-\t\tif (read_from_tree(prefix, argv + i, sha1))\n-\t\t\treturn 1;\n-\t\treturn update_index_refresh() ? 1 : 0;\n+\t\treturn read_from_tree(prefix, argv + i, sha1);\n \t}\n \tif (reset_type == NONE)\n \t\treset_type = MIXED; /* by default */\n@@ -264,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)\n \tcase SOFT: /* Nothing else to do. */\n \t\tbreak;\n \tcase MIXED: /* Report what has not been updated. */\n-\t\tupdate_index_refresh();\n+\t\tupdate_index_refresh(0, NULL);\n \t\tbreak;\n \t}\n \ndiff --git a/t/t7102-reset.sh b/t/t7102-reset.sh\nindex 506767d..e5c9f30 100755\n--- a/t/t7102-reset.sh\n+++ b/t/t7102-reset.sh\n@@ -418,4 +418,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '\n \tgit diff-index --cached --exit-code HEAD\n '\n \n+cat > expect << EOF\n+file2: needs update\n+EOF\n+\n+test_expect_success '--mixed refreshes the index' '\n+\techo 123 >> file2 &&\n+\tgit reset --mixed HEAD > output &&\n+\tgit diff --exit-code expect output\n+'\n+\n test_done\n-- \n1.5.3.5.1506.g83995\n"}]}