Re: [PATCH] Really fix git-merge-one-file-script this time.

Following up to my own message...

Linus, have you decided to like or dislike the behaviour of git-merge-one-file-script touching the work tree in some cases but not in other cases? A straightforward merge implementation that does a "git-read-tree -m" followed by a "git-merge-cache git-merge-one-file-script" does the following to your work tree and the cache:

 - Paths merged unsuccessfully makes git-merge-cache phase fail
   and the work tree is not affected for such paths.

 - Paths merged "git-read-tree -m" trivially does not change the
   work tree and "git-read-tree -m" result is kept in the cache.

 - Paths merged by "merge" successfully, and paths chosen from a
   single side by "git-merge-one-file-script" change the work
   tree, possibly checking out the file if you started out from
   an empty work tree.
 
I am not worried about the first case where you will have to
manually examine and resolve anyway. I am wondering if the rest
is the desired behavior for _your_ way of using the GIT merge.
After a successful merge, what kind of verification would you
typically do?

First of all, would you usually do the merge in an empty work tree, or in a populated work tree? Secondly, would you care about the distinction between "git-read-tree -m" trivial merges and "merge" trivial merges when reviewing the result?

If you work in an empty work tree, and never review the merge result while in that tree, then not touching the work tree in git-merge-one-file-script at all may be desirable, especially if you really want to keep things only in the cache. On the other hand, if you do review there, leaving the merge result in the work tree is desirable. Especially, if you want to verify the resulting files that are "merge" trivial but not are "git-read-tree -m" trivial, the files you see in the work tree are the only ones you need to check.

If you do your merge in a populated work tree, and assuming your starting work tree matches one of the commits being merged [*1*], it becomes harder to review the changes to the "merge" trivial but not "git-read-tree -m" trivial files. The cache does not tell you which ones are which with the current implementation of "git-merge-one-file-script". "git-diff-cache" against the tree before the merge would report all merges, including "git-read-tree -m" trivial ones, so you end up needing to save the output from git-merge-one-file-script and decide which paths to check.

I am wondering if the following changes would make sense and make things easier for you:

 * git-merge-one-file-script is changed to register the path
   with --cacheinfo using magic SHA1 0{40} instead of using the
   resulting file on the filesystem.  Do keep the current
   behaviour of leaving the merge results of trivial merges
   (both kind) in the work tree.

 * git-write-tree is changed to refuse to write from a cache
   that records the magic SHA1.

 * git-ls-files acquires a new option --merged to notice the
   magic SHA1 and shows the paths that have such SHA1.

 * git-update-cache acquires a new option --resolve to notice
   the magic SHA1 and:

   - if the named path is not in the work tree anymore, delete
     the entry.

   - if the named path exists in the work tree, compute the
     latest SHA1 for that file and update the entry.

Changes other than the first two listed above are purely optional, since the Porcelain layer can implement them without the Plumbing support. Not doing them would keep the Plumbing somewhat cleaner by not having to know about this magic SHA1 convention. On the other hand, we already use that convention in git-diff-cache, so it may even be a consistent change to make these optional changes. Essentially, the magic SHA1 in the cache means "I know the user wants me to keep an eye on this path when it matters" [*2*].

Please veto if these changes would not help _your_ use pattern.

[Footnotes]

*1* That is, you do "read-tree -m O A B" and your work tree before the merge matches A (e.g. linux-2.6.git or your yet-to-be-published descendant of it), B is a subsystem tree (e.g. rmk/linux-2.6-serial.git) and O is the common ancestor.

*2* This convention would also make an implementation of "SCM add" in the Porcelain layer a bit more efficient. A typical workflow without such a convention would consist of:

 * Create a file and start editing.
 * "SCM add" file, causing "git-update-cache --add -- file".
 * Do more changes, and review.
 * "SCM commit" which does"git-update-cache" changed files,
   "git-write-tree" and "git-commit-tree" to commit.

which wastes one extra blob object per "SCM add". My gut feeling is that more than 80% of the time "SCM add" is followed by some edit to the added file before "SCM commit", unless it is the initial import. If we adopt that convention, "SCM add" would register with --cacheinfo with the magic SHA1 without creating the useless blob, and "SCM commit" will be written to lazily pick things up from the work tree.