Re: [PATCHv2] git-mv: Keep moved index entries inact
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jul 29, 2008, 05:23 UTC
- Message-ID
- <7vfxptmfg0.fsf@gitster.siamese.dyndns.org>
- In-Reply-To
- <7vod4ho6uq.fsf@gitster.siamese.dyndns.org>
Junio C Hamano <gitster@pobox.com> writes:
Show 15 quoted lines
> Petr Baudis <pasky@suse.cz> writes: > >> On Mon, Jul 28, 2008 at 12:19:19PM -0700, Junio C Hamano wrote: >>> We need to refresh the entry to pick up potential ctime changes. >>> >>> read-cache.c | 7 ++++++- >>> builtin-ls-files.c | 21 +++++++++++++++------ >>> 2 files changed, 21 insertions(+), 7 deletions(-) >>> >>> diff --git a/read-cache.c b/read-cache.c >>> index 1cae361..834096f 100644 >> >> Oops, silly me. Sorry for being slower than you at fixing this. ;-) > > I did think about ctime issues when I applied the patch.
Actually I changed my mind.
I think it is wrong for something as low-level as rename_index_entry_at() to unconditionally look at the working tree and try refreshing the entry. The next caller of this function may not even require a working tree.
I think Dscho is correct; expecting the saved cacheinfo to stay fresh across rename does not make much sense. What we care about with "git mv" is that we keep what the user staged, i.e. kind of blob and the contents. It cannot be guaranteed if the index entry is stat clean or not, as st_ctime may or may not be updated across rename(2) according to POSIX.
So let's do this instead.
-- >8 -- t7001: fix "git mv" test
The test assumed that we can keep the cached stat information fresh across rename(2); many filesystems however update st_ctime (and POSIX allows them to do so), so that assumption does not hold. We can explicitly refresh the index for the purpose of the test, as the only thing we are interested in is the staged contents and the mode bits are preserved across "mv".
Signed-off-by: Junio C Hamano <gitster@pobox.com> --- t/t7001-mv.sh | 2 ++ 1 files changed, 2 insertions(+), 0 deletions(-)
diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh index b0fa407..910a28c 100755 --- a/t/t7001-mv.sh +++ b/t/t7001-mv.sh @@ -185,6 +185,7 @@ test_expect_success 'git mv should overwrite symlink to a file' ' ! test -e moved && test -f symlink && test "$(cat symlink)" = 1 && + git update-index --refresh && git diff-files --quiet ' @@ -202,6 +203,7 @@ test_expect_success 'git mv should overwrite file with a symlink' ' git mv -f symlink moved && ! test -e symlink && test -h moved && + git update-index --refresh && git diff-files --quiet '