threads / discuss / 10614

git reset -- path weirdness

Subject: git reset -- path weirdness

## tl;dr

3 messages between Nov 3, 2007 and Nov 3, 2007.

replies: 2people: 2as markdown or json

Björn Steinbrink· Nov 3, 2007, 11:17 UTC · lore
Hi,

I noticed some weirdness with git reset when a path is given. Basically it seems to cycle the file through 3 states: unstaged, unmerged, deleted(!) which is IMHO weird at best. A bisection showed that the behaviour was introduced with the shell -> conversion of git-reset.

Extra annoyance is caused by the fact that "git status" tells you to add the file when it is in the unmerged state, but that doesn't do anything.

Log of "git reset" switching through the states:

doener@atjola:git (master) $ git status # On branch master nothing to commit (working directory clean)

doener@atjola:git (master) $ git reset -- builtin-reset.c builtin-reset.c: needs merge

doener@atjola:git (master) $ git status # On branch master # Changed but not updated: # (use "git add <file>..." to update what will be committed) # # unmerged: builtin-reset.c # no changes added to commit (use "git add" and/or "git commit -a")

doener@atjola:git (master) $ git reset -- builtin-reset.c

doener@atjola:git (master) $ git status # On branch master # Changes to be committed: # (use "git reset HEAD <file>..." to unstage) # # deleted: builtin-reset.c # # Untracked files: # (use "git add <file>..." to include in what will be committed) # # builtin-reset.c

doener@atjola:git (master) $ git reset -- builtin-reset.c

doener@atjola:git (master) $ git status # On branch master nothing to commit (working directory clean)

Thanks, Björn

Johannes Schindelin· Nov 3, 2007, 12:03 UTC · re: Björn Steinbrink · lore

Re: git reset -- path weirdness

Hi,
On Sat, 3 Nov 2007, Bj?rn Steinbrink wrote:
> I noticed some weirdness with git reset when a path is given. Basically 
> it seems to cycle the file through 3 states: unstaged, unmerged, 
> deleted(!) which is IMHO weird at best. A bisection showed that the 
> behaviour was introduced with the shell -> conversion of git-reset.
Indeed.  I'm on it.

Ciao, Dscho

Johannes Schindelin· Nov 3, 2007, 13:12 UTC · re: Björn Steinbrink · lore

[PATCH] git-reset: do not be confused if there is nothing to reset

The purpose of the function update_index_from_diff() (which is the callback function we give do_diff_cache()) is to update those index entries which differ from the given commit.

Since do_diff_cache() plays games with the in-memory index, this function discarded the cache and reread it.

Then, back in the function read_from_tree() we wrote the index.

Of course, this broke down when there were no changes and update_index_from_diff() was not called, and therefore the mangled index was not discarded.

The solution is to move the index writing into the function update_index_from_diff().

Noticed by Björn Steinbrink.
Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 builtin-reset.c  |   24 ++++++++++++++++++------
 t/t7102-reset.sh |    7 +++++++
 2 files changed, 25 insertions(+), 6 deletions(-)
diff --git a/builtin-reset.c b/builtin-reset.c
index e1dc31e..5467e36 100644
--- a/builtin-reset.c
+++ b/builtin-reset.c
@@ -113,10 +113,17 @@ static int update_index_refresh(void)
 	return run_command_v_opt(argv_update_index, RUN_GIT_CMD);
 }
 
+struct update_cb_data {
+	int index_fd;
+	struct lock_file *lock;
+	int exit_code;
+};
+
 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;
 
 	/* do_diff_cache() mangled the index */
 	discard_cache();
@@ -133,29 +140,34 @@ 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;
 	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;
 
-	index_fd = hold_locked_index(lock, 1);
+	cb.lock = xcalloc(1, sizeof(struct lock_file));
+	cb.index_fd = hold_locked_index(cb.lock, 1);
+	cb.exit_code = 0;
 	read_cache();
 	if (do_diff_cache(tree_sha1, &opt))
 		return 1;
 	diffcore_std(&opt);
 	diff_flush(&opt);
-	return write_cache(index_fd, active_cache, active_nr) ||
-		close(index_fd) ||
-		commit_locked_index(lock);
+
+	return cb.exit_code;
 }
 
 static void prepend_reflog_action(const char *action, char *buf, size_t size)
diff --git a/t/t7102-reset.sh b/t/t7102-reset.sh
index f64b1cb..cea9afb 100755
--- a/t/t7102-reset.sh
+++ b/t/t7102-reset.sh
@@ -402,4 +402,11 @@ test_expect_success 'test resetting the index at give paths' '
 
 '
 
+test_expect_success 'resetting an unmodified path is a no-op' '
+	git reset --hard &&
+	git reset -- file1 &&
+	git diff-files --exit-code &&
+	git diff-index --cached --exit-code HEAD
+'
+
 test_done
-- 
1.5.3.5.1505.gd778c

← back to recent threads