Re: reset reports file as modified when it's in fact deleted

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:

> When I delete a file (git rm) and then reset so it exists in the index
> again, the message tells me 'M file.txt' even though the file doesn't
> exist in the worktree anymore. Running git status afterwards does give
> the correct output. So, here's the minimal steps to reproduce:
> 
>     $ git init
>     Initialized empty Git repository in /home/carlos/test/reset-err/.git/
>     $ touch file.txt
>     $ git add file.txt
>     $ git ci file.txt -m initial
>     [master (root-commit) a536393] initial
>      0 files changed, 0 insertions(+), 0 deletions(-)
>      create mode 100644 file.txt
>     $ git rm file.txt
>     rm 'file.txt'
>     $ git reset -- file.txt
>     Unstaged changes after reset:
>     M		 file.txt
>     $ git status -b -s
>     ## master
>      D file.txt

You can simplify this even further by not touching the index at all:

  git init -q &&
  >file && git add file && git commit -q -m initial &&
  rm file &&
  git reset

produces:

  Unstaged changes after reset:
  M       file

> I'd expect the output after "Unstaged changes after reset" to tell me
> file.txt has been deleted instead of modified. This happens with
> 1.7.8-rc0, 1.7.7 and 1.7.4.1 and I expect with many more that I don't
> have here.
> 
> I thought the index diff code might have been checking the index at the
> wrong time, but I can run 'git reset HEAD -- file.txt' as many times
> as I want, and it will still say 'M', so now I'm not sure.

The index diff code isn't running at all. Those messages are produced by
refresh_index, which outputs only two flags: modified or unmerged. I
think the reason for this is somewhat historical. You would run
"update-index --refresh", and it would helpfully say "by the way, when
refreshing this entry, I noticed that it is in need of being updated in
the index". The text was "file.txt: needs update".

Later, many porcelain commands started to refresh the index themselves,
and the "needs update" message was very confusing. So it was switched to
the more familiar "M file.txt" (though you can still see the original
plumbing message if you run update-index yourself).

I think it is a little more friendly to distinguish deletion from just
modification. And there's shouldn't be any compatibility questions, as
these are explicitly porcelain output (plumbing will still say "needs
update").

There are a few other cases users might expect to see. We'll never show
copies or renames, of course, because we aren't actually doing a diff
with copy detection. We won't see an "A"dded file, because such a file
would be in the working tree but not the index, meaning it is not
tracked.

We could see a "T"ypechange, but the distinction between that and a
modified file is lost by the time we get to refresh_index. We could pass
it up, but I wonder if it's really worth it.

The patch to do "D"eleted is pretty simple:

diff --git a/read-cache.c b/read-cache.c
index dea7cd8..cc1ebdf 100644
--- a/read-cache.c
+++ b/read-cache.c
@@ -1103,9 +1103,11 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 	int in_porcelain = (flags & REFRESH_IN_PORCELAIN);
 	unsigned int options = really ? CE_MATCH_IGNORE_VALID : 0;
 	const char *needs_update_fmt;
+	const char *needs_rm_fmt;
 	const char *needs_merge_fmt;
 
 	needs_update_fmt = (in_porcelain ? "M\t%s\n" : "%s: needs update\n");
+	needs_rm_fmt = (in_porcelain ? "D\t%s\n" : "%s: needs update\n");
 	needs_merge_fmt = (in_porcelain ? "U\t%s\n" : "%s: needs merge\n");
 	for (i = 0; i < istate->cache_nr; i++) {
 		struct cache_entry *ce, *new;
@@ -1145,7 +1147,10 @@ int refresh_index(struct index_state *istate, unsigned int flags, const char **p
 			}
 			if (quiet)
 				continue;
-			show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
+			if (cache_errno == ENOENT)
+				show_file(needs_rm_fmt, ce->name, in_porcelain, &first, header_msg);
+			else
+				show_file(needs_update_fmt, ce->name, in_porcelain, &first, header_msg);
 			has_errors = 1;
 			continue;
 		}
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]