Re: [RFC/PATCH 1/1] format-patch: add an option to record base tree info

Xiaolong Ye <xiaolong.ye@intel.com> writes:

I have a mixed feeling about this one, primarily because this was already tried quite early in the life of "format-patch" command.

    http://thread.gmane.org/gmane.comp.version-control.git/9694/focus=9757

Only the name is different (it was called "applies-to" and named a tree object).

The approach the patch I am responding to takes, which is to touch diff.[ch], is probably flawed. Even if we assume, for the sake of reviewing, that "which commit (or tree) this patch applies to" is a good thing to add, this is useful only to the very first patch in the series. Hence its output should come from some function in builtin/log.c that iterates over commits and done for only the first one in a series, not by the diff machinery that is called by that loop iterating over commits.

It is unclear what your goal is, and "would be helpful" is just as convincing as saying it would be helpful to record the phase of the moon when the commit was made. A typical patch rarely touches all the files in the project, so there will be multiple commits in the existing history of the project to which the patch would apply cleanly. Is it your goal to insist on one exact commit the patch is applied to? Or are you OK as long as the patch is applied to the same set of blobs the diff was taken from? A developer may have a few unrelated and private commits on top of Linus's released version, on top of which she builds a series. As long as the paths touched by the patches in this series do not overlap with the paths touched by the initial few unrelated and private commits, such a series should be testable directly on top of Linus's released version, without forcing her to first rebase the series to remove these initial few unrelated and private commits from her history before running "format-patch --base-commit" (your patch is recording the commit object name, and it shouldn't call it tree-info), but you are forcing her to do so because these private commits will not be available to those who apply her patches.

If you are OK with accepting a patch application to a tree with the same blobs the diff was taken from, then the format-patch output already has all the necessary information. For each "diff --git" part, there is the "index $old..$new" line that records preimage and postimage blob object ID, so you should be able to find a tree that has the matching blobs in the paths mentioned in the patch, and it is guaranteed that the patch would apply cleanly to such a tree.

Of course, that requires the recipient of the patch to have the all the blobs mentioned as the preimage in the patch, but it would be a reasonable assumption, as your patch assumes that the recipient has the commit, and if he has a commit by definition he would have all the blobs recorded by that commit.

Incidentally, the "index $old..$new" lines are what make "am -3" possible.