Re: [PATCH 06/10] fast-export: when using paths, avoid corrupt stream with non-existent mark

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

 



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?

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

-Peff



[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]

  Powered by Linux