Re: [PATCH v3 3/3] correct ce_compare_data() in a middle of a merge

On 07/07/16 20:43, Junio C Hamano wrote:
 > Torsten Bögershausen <tboegi@web.de> writes:
 >
 >> This is the call stack:
 >>
 >> merge-recursive.c/add_cacheinfo(unsigned int mode, const unsigned 
char *sha1,
 >> 		const char *path, int stage, int refresh, int options)
 >> {
 >> 	struct cache_entry *ce;
 >> 	ce = make_cache_entry
 >> 	if (!ce)
 >> 		return error(_("addinfo_cache failed for path '%s'"), path);
 >> 	return add_cache_entry(ce, options);
 >> }
 >> #Calls
 >> read-cache.c/make_cache_entry(path=file sha1=0x99b633 stage=0)
 >>
 >>
 >> struct cache_entry *make_cache_entry(unsigned int mode,
 >> 		const unsigned char *sha1, const char *path, int stage,
 >> 		unsigned int refresh_options)
 >> {
 >>         [snip]
 >> 	ret = refresh_cache_entry(ce, refresh_options);
 >> 	if (ret != ce)
 >> 		free(ce);
 >> 	return ret;
 >> }
 >>
 >> #Calls
 >> refresh_cache_ent(&the_index, ce, options, NULL, NULL);
 >>
 >> #Calls
 >> ie_modified()
 >>
 >> #Calls
 >> read-cache.c/ie_match_stat()
 >>
 >> #Calls
 >> changed |= ce_modified_check_fs(ce, st);
 >>
 >> #Calls
 >> ce_compare_data(path=file sha1=0x99b633)
 >>
 >> #Calls
 >> index_fd(..., ..., ce->name, ...)
 >> #Note the sha is lost here, the parameter sha is the output.
 >>
 >> Deep down, has_cr_in_index(path) will look at ad55e2 instead,
 >> which is wrong.
 > The call to add_cacheinfo() is made with 99b633 to record the 3-way
 > merge result to the index in this callchain:
 >
 >  merge_trees()
 >  -> git_merge_trees()
 >  -> process_renames() # does nothing for this case
 >  -> process_entry()
 >     -> merge_content()
 >        -> merge_file_special_markers()
 >           -> merge_file_1()
 >              -> merge_3way()
 >                 -> ll_merge() # correctly handles the renormalization
 >              -> write_sha1_file() # this is what gives us 99b633
 >     -> update_file() # this is fed the 99b633 write_sha1_file() computed
 >        -> update_file_flags()
 >           -> read_sha1_file() # reads 99b633
 >           -> convert_to_working_tree()
 >           -> write_in_full() # updates the working tree
 >           -> add_cacheinfo() # to record 99b633 at "file" in the index
 >
 > So refresh() may incorrectly find that 99b633 does not match what is
 > in the filesystem if it looks at _any_ entry in the index.  We know
 > we have written the file ourselves, so by definition it should match
 > ;-)

Does it, always ? There was a lengthy discussion around January, if git checkout -f should always give a clean workspace. (My suggestion was yes, please), but the conclusion was to always check the other way around: What would "git add" say ? If the smudge/clean don't provide a round trip, then the worktree should not be clean.

My understanding is, that after merge, the filters must be round trip (and all other conversions), otherwise the merge willl fail.

In this case the merge fails because of a bug in Git.