Nguyễn Thái Ngọc Duy [off-list ref] writes:
entry_count is used in update_one() for two purposes:
1. to skip through the number of processed entries in in-memory index
2. to record the number of entries this cache-tree covers on disk
Unfortunately when CE_REMOVE is present these numbers are not the same
because CE_REMOVE entries are automatically removed before writing to
disk but entry_count is not adjusted and still counts CE_REMOVE
entries.
Nicely explained. I wonder if we can also add a piece of test to
the patch 4/4 to demonstrate the issue with CE_REMOVE entries,
though.
Thanks.
On Sun, Dec 16, 2012 at 2:20 PM, Junio C Hamano [off-list ref] wrote:
Nguyễn Thái Ngọc Duy [off-list ref] writes:
quoted
entry_count is used in update_one() for two purposes:
1. to skip through the number of processed entries in in-memory index
2. to record the number of entries this cache-tree covers on disk
Unfortunately when CE_REMOVE is present these numbers are not the same
because CE_REMOVE entries are automatically removed before writing to
disk but entry_count is not adjusted and still counts CE_REMOVE
entries.
Nicely explained. I wonder if we can also add a piece of test to
the patch 4/4 to demonstrate the issue with CE_REMOVE entries,
though.
A hand crafted one, maybe. I did not attempt to recreate it with git
commands (and I don't think we update cache-tree after unpack_trees).
So I wrote something like this instead:
int main(int ac, char **av)
{
unsigned char sha1[20];
setup_git_directory();
read_cache();
active_cache[1]->ce_flags |= CE_REMOVE;
write_cache_as_tree(sha1, 0, NULL);
return 0;
}
I can polish it a bit and write new tests based on it and
test-dump-cache-tree if you want.
--
Duy
Nguyen Thai Ngoc Duy [off-list ref] wrote:
On Sun, Dec 16, 2012 at 2:20 PM, Junio C Hamano [off-list ref]
wrote:
quoted
Nicely explained. I wonder if we can also add a piece of test to
the patch 4/4 to demonstrate the issue with CE_REMOVE entries,
though.
A hand crafted one, maybe. I did not attempt to recreate it with git
commands (and I don't think we update cache-tree after unpack_trees).
Yeah, that's what I thought. No need to bother creating a bug that won't appear in the real life :-)
Tusks for sanity checking.
--
Pardon terseness, typo and HTML from a tablet.