Christian Couder <chriscool@xxxxxxxxxxxxx> writes: > From: Junio C Hamano <gitster@xxxxxxxxx> > > Commit 9e8ecea (Add 'merge' mode to 'git reset', 2008-12-01) disallowed > "git reset --merge" when there was unmerged entries. It acknowledged > that this is not the best possible behavior, and that it would be better > if unmerged entries were reset as if --hard (instead of --merge) has > been used. > > Recently another commit (reset: use "unpack_trees()" directly instead of > "git read-tree", 2009-12-30) changed the behavior of --merge to accept > resetting unmerged entries if they are reset to a different state than > HEAD, but it did not reset the changes in the work tree. So the behavior > was kind of improved, but it was not yet as if --hard has been used. It would be more honest if we said something like: It changed a safer "I can't do as asked, please do it by hand" into a more dangerous "I pretend that I did so but I didn't do the full job; you need to fix up the result but I am not telling you that you have to", which is a lot worse. here instead (that is one reason why I said my fix-up was "squashable"). But that is a minor issue. I have been thinking about two issues on this --merge change. > - Updates merged_entry() and deleted_entry() so that they pay attention > to cache entries with null sha1 (note that we _might_ want to use a > new in-core ce->ce_flags instead of using the null-sha1 hack). They > are previously unmerged entries, and the files in the work tree that > correspond to them are resetted away by oneway_merge() to the version > from the tree we are resetting to. One is the use of ce_flags instead of relying on the 20-byte comparison I said above, for both performance (minor) and future maintainability (much bigger) concern. I have a feeling that we will regret later that we used the null_sha1 trick here, when we want to express another "special" kind of cache entry in unrelated situations. The use of null_sha1 hack was expedient but I fell victim of the same mentality of declaring that this is the _last_ such kind of special index entry and closing the door to others who want to extend the system later with different kind of special cache entry, which I often complain about myself to patches from other people. Another is that it _might_ make sense to use two-tree form of read-tree machinery (but using a different version of unpack-trees.c::twoway_merge() function), instead of the one-tree form of "we don't bother checking if the index is consistent with HEAD and assume it is, and jump to the target." "git reset --merge $there" is about the situation where you started a "mergy" operation (e.g. "git merge", "git am -3", "git rebase", ...) while you had unrelated local changes in the work tree, and you want to go back to the state before that operation $there (which is HEAD if the mergy operation is "merge", but is different from HEAD if it was "am -3" and you have successfully applied a patch or more already). Linus may know and won't use "reset --merge" in a situation where it is not suitable, but not everybody is Linus. Even though "reset --merge" may "correctly" work with respect to the table you added to "git reset" documentation, it would do something that may not "make sense" from the end-user's point of view when used in a situation that it wasn't designed for. Using the two-tree form allows the machinery to inspect the difference between HEAD and the index, and detect cases where "reset --merge" was attempted when it shouldn't. For example, if stage #2 of an unmerged path does not match HEAD, we know there is something wrong. This latter issue is much bigger and needs a lot more thought, and I don't think it should block the series from going forward at all. But I think it is worth keeping in the back of our heads. -- 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