On Sun, Nov 18, 2007 at 10:47:24AM +0100, Steffen Prohaska wrote: > > On Nov 18, 2007, at 4:59 AM, J. Bruce Fields wrote: > >> On Sat, Nov 10, 2007 at 02:49:54PM +0100, Steffen Prohaska wrote: >>> +[[bisect-merges]] >>> +Why bisecting merge commits can be harder than bisecting linear history >>> +----------------------------------------------------------------------- >>> + >>> +This section discusses how gitlink:git-bisect[1] plays >>> +with differently shaped histories. The text is based upon >>> +an email by Junio C. Hamano to the git mailing list >>> +(link:http://marc.info/?l=git&m=119403257315527&w=2[link:http://marc.info/?l=git&m=119403257315527&w=2]). >>> +It was adapted for the user manual. >> >> This is not the only text that's been taken from someplace else, but if >> we attributed them all in the text it would get a little cumbersome.... >> I think the place for that kind of thing is in the commit message, but >> if we really think we need to include it in the main text, we could add >> a separate "'acknowledgements" section. > > ack for this change. > > And a general comment: I had two purposes in mind when I > added this paragraph > 1) Attribution; > 2) Giving a reference to the original discussion. If someone disagrees, > or has further questions, the original discussion on the mailing > list could be useful. > > I believe the commit message is sufficient for attribution. > I don't think we need more. Maybe if the manual goes to print > we need to reconsider. But who know if this ever will happen. > > However, adding a References section with links to other > resources could be a useful thing. Such resources could give > additional information, without sacrificing the conciseness of > the manual. An example are links to the original discussion on > the list. Often they contain more details, which may sometimes > be useful. > > Does asciidoc provide a mechanism for this? Something like > \cite{} in LaTeX. We could add a "for more details, see link:..." at the end, though I don't think it's necessary in this case. >>> +Using gitlink:git-bisect[1] on a history with merges can be >>> +challenging. Bisecting through merges is not a technical >>> +problem. The real problem is what to do when the culprit turns >>> +out to be a merge commit. How to spot what really is wrong, and >>> +figure out how to fix it. The problem is not for the tool but >>> +for the human, and it is real. >> >> I think we can pare that down a little. > > From your 7df716bec6bf6e3dafe4c36a6313a4346de2585a > > +The gitlink:git-bisect[1] commands deals fine with history that includes > +merge commits. However, when the final commit that ends on is a merge > +commit, the user may need to work harder than usual to figure out > +exactly what the problem was. > > If you don't take the text below, first line: s/commands/command/ Whoops, thanks! > > I'd reverse the order of the sentences. The section is about > the difficulty, not about praising git-bisect. How about > > When the final commit that a bisect ends on is a merge commit, > the user may need to work harder than usual to figure out > exactly what the problem was. This is not a technical > problem. In principle, the gitlink:git-bisect[1] command > deals fine with history that includes merge commits. > > It's your call. I'm also fine with your version. OK; I see your point but I think I'll stick with my version (with another minor fix or two)--the order seems more natural: first dismiss the obvious objection to the title, then introduce the problem that leads to the following discussion. >>> + >>> +Imagine this history: >>> + >>> +................................................ >>> + ---Z---o---X---...---o---A---C---D >>> + \ / >>> + o---o---Y---...---o---B >>> +................................................ >>> + >>> +Suppose that on the upper development line, the meaning of one >>> +of the functions that existed at Z was changed at commit X. The >>> +commits from Z leading to A change both the function's >>> +implementation and all calling sites that existed at Z, as well >>> +as new calling sites they add, to be consistent. There is no >>> +bug at A. >>> + >>> +Suppose that in the meantime the lower development line somebody >>> +added a new calling site for that function at commit Y. The >>> +commits from Z leading to B all assume the old semantics of that >>> +function and the callers and the callee are consistent with each >>> +other. There is no bug at B, either. >>> + >>> +Suppose further that the two development lines were merged at C >>> +and there was no textual conflict with this three way merge. >>> +The result merged cleanly. >>> + >>> +Now, during bisect you find that the merge C is broken. You >>> +started to bisect, because you found D is bad and you know Z was >>> +good. The breakage is understandable, as at C, the new calling >>> +site of the function added by the lower branch is not converted >>> +to the new semantics, while all the other calling sites that >>> +already existed at Z would have been converted by the merge. The >>> +new calling site has semantic adjustment needed, but you do not >>> +know that yet. >>> + > > From your 7df716bec6bf6e3dafe4c36a6313a4346de2585a > > +Nevertheless, the code at C is broken, because the callers added > +on the lower line of development have not been converted to the new > +semantics introduced on the upper line of development. So if all > +you know is that D was bad, Z was good, and that a > > s/a//? I'm not sure; you are the native speaker ;) I agree, it's better without the "a", thanks. > > +gitlink:git-bisect[1] identified C as the culprit, how will you > +figure out that the problem was due to this change in semantics? > > > >>> +You need to find out what is the cause of the breakage by looking >>> +at the merge commit C and the history leading to it. How would >>> +you do that? >>> + >>> +Both "git diff A C" and "git diff B C" would be an enormous patch. >>> +Each of them essentially shows the whole change on each branch >>> +since they diverged. The developers may have well behaved to >>> +create good commits that follow the "commit small, commit often, >>> +commit well contained units" mantra, and each individual commit >>> +leading from Z to A and from Z to B may be easy to review and >>> +understand, but looking at these small and easily reviewable >>> +steps alone would not let you spot the breakage. You need to >>> +have a global picture of what the upper branch did (and >>> +among many, one of them is to change the semantics of that >>> +particular function) and look first at the huge "diff A C" >>> +(which shows the change the lower branch introduces), and see if >>> +that huge change is consistent with what have been done between >>> +Z and A. >>> + > > From your 7df716bec6bf6e3dafe4c36a6313a4346de2585a > > +When the result of a git-bisect is a non-merge commit, you should > +normally be able to discover the problem be examining just that commit. > > s/be examining/by examining/ Oops, thanks. >> I'd say "partly for this reason", as I don't think this is the only >> reason people do that. >> >> I've done the above revisions and a few others and pushed them to >> >> git://linux-nfs.org/~bfields/git.git maint >> >> I'll take another look in the morning. > > Besides the minor fixes above, ack from me. We already spend > a lot of time on this section. It improved compared to the > first version and I think it's now ready for the manual. OK, thanks for your patience! This plus an unrelated trivial fix are available for Junio to pull from git://linux-nfs.org/~bfields/git.git maint (Full history also available from the maint-history branch). --b. - 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
