Re: reset reports file as modified when it's in fact deleted
flat view
From: Carlos Martín Nieto <hidden>
Date: 2016-06-15 22:52:28
On Mon, Nov 07, 2011 at 11:26:42AM -0500, Jeff King wrote:
On Mon, Nov 07, 2011 at 10:43:30AM +0100, Carlos Martín Nieto wrote:quoted
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.txtYou 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
Ah, I see. I got the previous sequence because that's what we have in an instruction manual and where we saw it.
quoted
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.
That's probably overkill. The issue with reporting 'M' for a deleted file is that it conflicts with what the status output would say, even though it's in the same format.
quoted hunk ↗ jump to hunk
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");
While the name fits in with the rest of the variables, it's kind of the wrong way around, isn't it? It doesn't need an 'rm', it /was/ rm'd. Other than that stupid thing, the patch works great, thanks. Can we get it into git?
quoted hunk ↗ jump to hunk
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@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Attachments
- signature.asc [application/pgp-signature] 490 bytes