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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Xiaolong Ye <xiaolong.ye@xxxxxxxxx> writes:

> It would be helpful for maintainers or reviewers to know the base tree
> info of the patches created by git format-patch. Teach git format-patch
> a --base-tree-info option to record these info.
>
> Signed-off-by: Xiaolong Ye <xiaolong.ye@xxxxxxxxx>
> ---

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.
--
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



[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]