On Thu, Mar 24, 2016 at 10:01:58AM -0700, Junio C Hamano wrote: >Ye Xiaolong <xiaolong.ye@xxxxxxxxx> writes: > >> On Wed, Mar 23, 2016 at 11:25:41AM -0700, Junio C Hamano wrote: >>>I also do not see the point of showing "parent id" which as far as I >>>can see is just a random commit object name and show different >>>output that is not even described what it is. It would be better to >> >> Here is our consideration: >> There is high chance that branch_get_upstream will retrun NULL(thus we >> are not able to get exact base commit), since developers may checkout >> branch from a local branch or a commit and haven't set "--set-upstream-to" >> to track a remote branch, in this case, we want to provide likely useful >> info(here is parent commit id and patch id) > >I do not agree with that reasoning at all, primarily because your >"likely useful" is not justfied. > >Could you explain what makes you think that it is "likely" that the >commit that matches "parent commit id" is available to the recipient >of the patch? > Hi, Junio Still want to discuss with you about the proposal that showing the "parent commit-id as well as patch-id" when exact base commit is failed to get through cmdline or upstream", from 0day robot's view, there would be 2 kinds of base tree info appended at the bottom of patch message. For the info starts with "base-commit: <xxx> ...", robot would know it is reliable base commit, it would just checkout it and apply the prerequisite patches and patchset for the work. For the info starts with "parent-commit: <xxx>; parent-patch-id: <xxx>", there are 3 situations 0day robot would need to handle: 1) parent commit is well-known public commit(e.g. one in Linus's tree), in this case, robot will just checkout the parent commit and apply the patchset accordingly. 2) parent commit is well-known in-flight patch, since 0day maintains the database of in-flight patches indexed by their patch-ids and commit-ids(of the git tree mentioned below), it also exports a git tree which holds all in-flight patches, where each patchset map to a new branch: https://github.com/0day-ci/linux/branches 0day will find that patch in database by parent patch id, then do the checkout and apply work. 3) parent commit/patch-id is unknown, maybe because it's a - private patch; - public patch that's slightly modified locally; - public patch that's not covered by the 0day database all should be rare cases. In practice, most developers may not feed exact commit id by --base=<xxx> regularly when using git format-patch, this "showing parent commit" solution could save developers' energy and reach a high coverage in the meantime. Thanks, Xiaolong. >Whatever the reason is, if it _is_ likely, then I do not see a point >in spending cycle to do get-upstream and merge-base, or giving an >option to the user to explicitly specify the base. Given that this >series does these things, I'd have to conclude your "likely useful" >is "might possibly turn out to be useful in some cases if you are >lucky but is no way reliable" at best. > >Rather than throwing an unreliable information in the output and >blindly proceeding, I'd think you would want to error out and tell >the user to explicitly give the base without producing the patch >output. That way, you will not get patches with unreliable >information. > >Suggesting to use set-upstream-to when you give that error message >may also be helpful. -- 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