Web lists-archives.com

Re: [PATCH 2/3] fast-export: do not refer to non-existing marks




Hi Sverre,

On Sun, 6 Nov 2011, Sverre Rabbelier wrote:

> On Sun, Nov 6, 2011 at 05:45, Jonathan Nieder <jrnieder@xxxxxxxxx> wrote:
> >> Extract a handle_reset function that deals with this, which can then
> >> be re-used in a later commit.
> >
> > So, does this patch drop the confusing behavior and add one that is
> > more intuitive for remote helpers?  It's not clear from this
> > description what sort of deal the patch makes and whether it is a good
> > or bad one.
> 
> Ah, yes. Perhaps something like:
> 
> "Extract a reset_ref function that deals with this situation by
> printing the commit sha1 when no mark has been written yet."
> 
> > Ah --- the functional change is to use a sha1 when there is no mark
> > corresponding to the object.
> >
> > Why is this codepath being run at all when b is excluded by the
> > revision range (a..a a = ^a a a)?  Is this the same bug tested
> > for in patch 1/3 or something separate?
> 
> I must admit that I don't recall how exactly we stumbled on this case.
> It might even make sense to instead die when we run into this corner
> case, but I'm not convinced that there's no valid use case for this
> (which we would block by die-ing).

I know, it has been a while since we hacked on this in your tiny room in
the Netherlands, and it has been almost as long since this here mail
thread stalled, when the consensus back then seemed that this patch is not
even necessary.

You might find it satisfying that this change, in a slightly different
form, made it to `master` recently, more precisely in
https://github.com/git/git/commit/530ca19c02b1fa1d13195d24fc76c2926ceecdc2

So: closure, at long last.

Ciao,
Dscho