Re: [PATCH] git-mv: Keep moved index entries inact

[Junio C Hamano <gitster@pobox.com>]

Petr Baudis <pasky@suse.cz> writes:

> A new test has been added to the testsuite to reflect this change.
> Also, based on suggestion by Junio about desired symlink behaviour
> of git mv, I have added two tests for that; however, I do not have
> need or desire to spend time fixing this, so they are expected
> to fail for now until someone gets around to fixing that.

Well, somebody would eventually come to help, then ;-).

>  builtin-mv.c  |   62 ++++++++-------------------------------------------------
>  cache.h       |    2 ++
>  read-cache.c  |   15 ++++++++++++++
>  t/t7001-mv.sh |   57 ++++++++++++++++++++++++++++++++++++++++++++++++++++
>  4 files changed, 83 insertions(+), 53 deletions(-)

Very nice code reduction, isn't it?

> diff --git a/read-cache.c b/read-cache.c
> index 1648428..70e5f57 100644
> --- a/read-cache.c
> +++ b/read-cache.c
> @@ -38,6 +38,21 @@ static void replace_index_entry(struct index_state *istate, int nr, struct cache
>  	istate->cache_changed = 1;
>  }
>  
> +void rename_index_entry_at(struct index_state *istate, int nr, const char *new_name)
> +{
> +	struct cache_entry *old = istate->cache[nr], *new;
> +	int namelen = strlen(new_name);
> +
> +	new = xmalloc(cache_entry_size(namelen));
> +	copy_cache_entry(new, old);
> +	new->ce_flags = (new->ce_flags & ~CE_NAMEMASK) | namelen;
> +	memcpy(new->name, new_name, namelen);
> +
> +	cache_tree_invalidate_path(istate->cache_tree, old->name);
> +	remove_index_entry_at(istate, nr);
> +	add_index_entry(istate, new, ADD_CACHE_OK_TO_ADD|ADD_CACHE_OK_TO_REPLACE);
> +}

Hmm, would this use of copy_cache_entry() kosher, I have to wonder.  This
new copy of cache entry begins its life unhashed, doesn't it?  Shouldn't
we be not copying its hashed/unhashed bits from the old one?

Also setting of that ce_flags looks wrong when namelen does not fit within
the width of CE_NAMEMASK.  Shouldn't it be doing the same thing as
create_ce_flags()?

> diff --git a/t/t7001-mv.sh b/t/t7001-mv.sh
> index 336cfaa..6b615f8 100755
> --- a/t/t7001-mv.sh
> +++ b/t/t7001-mv.sh
> @@ -156,4 +156,61 @@ test_expect_success 'absolute pathname outside should fail' '(
>  
>  )'
>  
> +# git mv meets angry Git maintainer

What's this comment about?

> +test_expect_success 'git mv should not change sha1 of moved cache entry' '
> +
> +	rm -fr .git &&
> +	git init &&
> +	echo 1 >dirty &&
> +	git add dirty &&
> +	entry="$(git ls-files --stage dirty | cut -f 1)"

"rev-parse :dirty"?