On Mon, Nov 12, 2018 at 4:45 AM Jeff King <peff@xxxxxxxx> wrote: > On Sun, Nov 11, 2018 at 12:01:43AM -0800, Elijah Newren wrote: > > > > It does seem funny that the behavior for the earlier case (bounded > > > commits) and this case (skipping some commits) are different. Would you > > > ever want to keep walking backwards to find an ancestor in the earlier > > > case? Or vice versa, would you ever want to simply delete a tag in a > > > case like this one? > > > > > > I'm not sure sure, but I suspect you may have thought about it a lot > > > harder than I have. :) > > > > I'm not sure why you thought the behavior for the two cases was > > different? For both patches, my testcases used path limiting; it was > > you who suggested employing a negative revision to bound the commits. > > Sorry, I think I just got confused. I was thinking about the > documentation fixup you started with, which did regard bounded commits. > But that's not relevant here. > > > Anyway, for both patches assuming you haven't bounded the commits, you > > can attempt to keep walking backwards to find an earlier ancestor, but > > the fundamental fact is you aren't guaranteed that you can find one > > (i.e. some tag or branch points to a commit that didn't modify any of > > the specified paths, and nor did any of its ancestors back to any root > > commits). I hit that case lots of times. If the user explicitly > > requested a tag or branch for export (and requested tag rewriting), > > and limited to certain paths that had never existed in the repository > > as of the time of the tag or branch, then you hit the cases these > > patches worry about. Patch 4 was about (annotated and signed) tags, > > this patch is about unannotated tags and branches and other refs. > > OK, that makes more sense. > > So I guess my question is: in patch 4, why do we not walk back to find > an appropriate ancestor pointed to by the signed tag object, as we do > here for the unannotated case? > > And I think the answer is: we already do that. It's just that the > unannotated case never learned the same trick. So basically it's: > > 1. rewriting annotated tags to ancestors is already known on "master" > > 2. patch 4 further teaches it to drop a tag when that fails > > 3. patch 6 teaches both (1) and (2) to the unannotated code path, > which knew neither > > Is that right? Ah, now I see where the slight disconnect was. And yes, you are correct. > > > This hunk makes sense. > > > > Cool, this was the entirety of the code...so does this mean that the > > code makes more sense than my commit message summary did? ...and > > perhaps that my attempts to answer your questions in this email > > weren't necessary anymore? > > No, it only made sense that the hunk implemented what you claimed in the > commit message. ;) > > I think your responses did help me understand that what the commit > message is claiming is a good thing.
