[PATCH] builtin-reset: do not call "ls-files --unmerged"

[PATCH] builtin-reset: do not call "ls-files --unmerged"

[Johannes Schindelin]

Since reset is a builtin now, it can use the full power of libgit.a
and check for unmerged entries itself.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 11 Sep 2007, Junio C Hamano wrote:

	> Carlos Rica <jasampler@gmail.com> writes:
	> 
	> > +static int unmerged_files(void)
	> > +{
	> > [...]
	> > +}
	> 
	> Not that git-reset is so performance sensitive, but you could do
	> this from built-in without exec, by just reading the index and
	> checking if you have a higher-stage entry yourself.

	Hereby done.

 builtin-reset.c  |   28 ++++++++--------------------
 t/t7102-reset.sh |    9 +++++++++
 2 files changed, 17 insertions(+), 20 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 5467e36..79792ee 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -46,26 +46,14 @@ static inline int is_merge(void)
 
 static int unmerged_files(void)
 {
-	char b;
-	ssize_t len;
-	struct child_process cmd;
-	const char *argv_ls_files[] = {"ls-files", "--unmerged", NULL};
-
-	memset(&cmd, 0, sizeof(cmd));
-	cmd.argv = argv_ls_files;
-	cmd.git_cmd = 1;
-	cmd.out = -1;
-
-	if (start_command(&cmd))
-		die("Could not run sub-command: git ls-files");
-
-	len = xread(cmd.out, &b, 1);
-	if (len < 0)
-		die("Could not read output from git ls-files: %s",
-						strerror(errno));
-	finish_command(&cmd);
-
-	return len;
+	int i;
+	read_cache();
+	for (i = 0; i < active_nr; i++) {
+		struct cache_entry *ce = active_cache[i];
+		if (ce_stage(ce))
+			return 1;
+	}
+	return 0;
 }
 
 static int reset_index_file(const unsigned char *sha1, int is_hard_reset)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index cea9afb..506767d 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -59,6 +59,15 @@ test_expect_success 'giving a non existing revision should fail' '
 	check_changes 3ec39651e7f44ea531a5de18a9fa791c0fd370fc
 '
 
+test_expect_success 'reset --soft with unmerged index should fail' '
+	touch .git/MERGE_HEAD &&
+	echo "100644 44c5b5884550c17758737edcced463447b91d42b 1	un" |
+		git update-index --index-info &&
+	! git reset --soft HEAD &&
+	rm .git/MERGE_HEAD &&
+	git rm --cached -- un
+'
+
 test_expect_success \
 	'giving paths with options different than --mixed should fail' '
 	! git reset --soft -- first &&
-- 
1.5.3.5.1506.g83995

[PATCH] builtin-reset: avoid forking "update-index --refresh"

[Johannes Schindelin]

Instead of forking update-index, call refresh_cache() directly.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---

	On Tue, 11 Sep 2007, Junio C Hamano wrote:

	> Carlos Rica <jasampler@gmail.com> writes:
	> 
	> > +static int update_index_refresh(void)
	> > +{
	> > +	const char *argv_update_index[] = {"update-index", "--refresh", NULL};
	> > +	return run_command_v_opt(argv_update_index, RUN_GIT_CMD);
	> > +}
	> 
	> Instead of making a call to this one after read_from_tree()
	> returns, immediately before writing the index out at the end of
	> read_from_tree(), you have the index in core.  I think you can
	> call read-cache.c::refresh_index() at that point and then do the
	> write_cache(), no?

	A little more complicated now, since this is (obviously) on top of 
	the bug fix I sent out earlier.

	I just realised that it partially undoes the changes of that 
	commit.  If you want to, I will redo this mini-series, changing 
	the first commit to something more similar to this patch.

 builtin-reset.c  |   53 +++++++++++++++++++++++++++++------------------------
 t/t7102-reset.sh |   10 ++++++++++
 2 files changed, 39 insertions(+), 24 deletions(-)

diff --git a/builtin-reset.c b/builtin-reset.c
index 79792ee..44582f2 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -95,26 +95,34 @@ static void print_new_head_line(struct commit *commit)
 		printf("\n");
 }
 
-static int update_index_refresh(void)
+static int update_index_refresh(int fd, struct lock_file *index_lock)
 {
-	const char *argv_update_index[] = {"update-index", "--refresh", NULL};
-	return run_command_v_opt(argv_update_index, RUN_GIT_CMD);
-}
+	int result;
+
+	if (!index_lock) {
+		index_lock = xcalloc(1, sizeof(struct lock_file));
+		fd = hold_locked_index(index_lock, 1);
+	}
 
-struct update_cb_data {
-	int index_fd;
-	struct lock_file *lock;
-	int exit_code;
-};
+	if (read_cache() < 0)
+		return error("Could not read index");
+	result = refresh_cache(0) ? 1 : 0;
+	if (write_cache(fd, active_cache, active_nr) ||
+			close(fd) ||
+			commit_locked_index(index_lock))
+		return error ("Could not refresh index");
+	return result;
+}
 
 static void update_index_from_diff(struct diff_queue_struct *q,
 		struct diff_options *opt, void *data)
 {
 	int i;
-	struct update_cb_data *cb = data;
+	int *discard_flag = data;
 
 	/* do_diff_cache() mangled the index */
 	discard_cache();
+	*discard_flag = 1;
 	read_cache();
 
 	for (i = 0; i < q->nr; i++) {
@@ -128,34 +136,33 @@ static void update_index_from_diff(struct diff_queue_struct *q,
 		} else
 			remove_file_from_cache(one->path);
 	}
-
-	cb->exit_code = write_cache(cb->index_fd, active_cache, active_nr) ||
-		close(cb->index_fd) ||
-		commit_locked_index(cb->lock);
 }
 
 static int read_from_tree(const char *prefix, const char **argv,
 		unsigned char *tree_sha1)
 {
+	struct lock_file *lock = xcalloc(1, sizeof(struct lock_file));
+	int index_fd, index_was_discarded = 0;
 	struct diff_options opt;
-	struct update_cb_data cb;
 
 	memset(&opt, 0, sizeof(opt));
 	diff_tree_setup_paths(get_pathspec(prefix, (const char **)argv), &opt);
 	opt.output_format = DIFF_FORMAT_CALLBACK;
 	opt.format_callback = update_index_from_diff;
-	opt.format_callback_data = &cb;
+	opt.format_callback_data = &index_was_discarded;
 
-	cb.lock = xcalloc(1, sizeof(struct lock_file));
-	cb.index_fd = hold_locked_index(cb.lock, 1);
-	cb.exit_code = 0;
+	index_fd = hold_locked_index(lock, 1);
+	index_was_discarded = 0;
 	read_cache();
 	if (do_diff_cache(tree_sha1, &opt))
 		return 1;
 	diffcore_std(&opt);
 	diff_flush(&opt);
 
-	return cb.exit_code;
+	if (!index_was_discarded)
+		/* The index is still clobbered from do_diff_cache() */
+		discard_cache();
+	return update_index_refresh(index_fd, lock);
 }
 
 static void prepend_reflog_action(const char *action, char *buf, size_t size)
@@ -225,9 +232,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 		else if (reset_type != NONE)
 			die("Cannot do %s reset with paths.",
 					reset_type_names[reset_type]);
-		if (read_from_tree(prefix, argv + i, sha1))
-			return 1;
-		return update_index_refresh() ? 1 : 0;
+		return read_from_tree(prefix, argv + i, sha1);
 	}
 	if (reset_type == NONE)
 		reset_type = MIXED; /* by default */
@@ -264,7 +269,7 @@ int cmd_reset(int argc, const char **argv, const char *prefix)
 	case SOFT: /* Nothing else to do. */
 		break;
 	case MIXED: /* Report what has not been updated. */
-		update_index_refresh();
+		update_index_refresh(0, NULL);
 		break;
 	}
 
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index 506767d..e5c9f30 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -418,4 +418,14 @@ test_expect_success 'resetting an unmodified path is a no-op' '
 	git diff-index --cached --exit-code HEAD
 '
 
+cat > expect << EOF
+file2: needs update
+EOF
+
+test_expect_success '--mixed refreshes the index' '
+	echo 123 >> file2 &&
+	git reset --mixed HEAD > output &&
+	git diff --exit-code expect output
+'
+
 test_done
-- 
1.5.3.5.1506.g83995