On Wed, Mar 2, 2011 at 1:19 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Elijah Newren <newren@xxxxxxxxx> writes:
>
>> @@ -1274,9 +1275,13 @@ static int merge_content(struct merge_options *o,
>> Â Â Â }
>>
>> Â Â Â if (mfi.clean && !df_conflict_remains &&
>> - Â Â Â Â sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode)
>> + Â Â Â Â sha_eq(mfi.sha, a_sha) && mfi.mode == a.mode &&
>> + Â Â Â Â lstat(path, &st) == 0) {
>> Â Â Â Â Â Â Â output(o, 3, "Skipped %s (merged same as existing)", path);
>> - Â Â else
>> + Â Â Â Â Â Â add_cacheinfo(mfi.mode, mfi.sha, path,
>> + Â Â Â Â Â Â Â Â Â Â Â Â Â 0 /*stage*/, 1 /*refresh*/, 0 /*options*/);
>> + Â Â Â Â Â Â return mfi.clean;
>> + Â Â } else
>
> Hmmmm. ÂDuring a merge, we allow files missing from the working tree as if
> it is not modified from the index. ÂChanging the behaviour based on the
> existence of the path on the filesystem does not feel quite right.
Really? Ouch. Then things have been broken ever since
make_room_for_directories_of_df_conflicts() was introduced, as that
function deletes files intentionally and expects them to be reinstated
later when possible. If a user has a file deleted that happens to be
involved in a D/F conflict before a merge, and the file is unchanged,
the merge will reinstate it.
> Even if we decide that regressing in that use case were acceptable, what
> guarantees that the path that happens to be in the work tree has the same
> contents as what the merge result should be? ÂIOW, shouldn't we be looking
> at the original index, making sure that our side (stage #2) at the path
> had a file there that matches the merge result mfi.{sha,mode}, instead of
> looking at the working tree?
We DID look at the original index, make sure that our side (stage #2)
at the path had a file there that matches the merge result
mfi.{sha,mode}.
But, that's not enough to know that we can skip the update of the
file. The merged results being different are only one reason to
update the file; the other is that we may have deleted the file
ourselves in make_room_for_directories_of_df_conflicts() and need to
replace it. This is the location in the code where such deleted files
are reinstated.
Suggestions?
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html