Re: [PATCH v3 4/4] fast-export: make sure refs are updated properly

[Johannes Schindelin <Johannes.Schindelin@gmx.de>]

Hi Peff,

On Fri, 2 Nov 2012, Jeff King wrote:

> On Tue, Oct 30, 2012 at 05:37:21PM -0700, Jonathan Nieder wrote:
> 
> > If the commit does not have the SHOWN or UNINTERESTING flag set but it
> > is going to get the UNINTERESTING flag set during the walk because of
> > a negative commit listed on the command line, this patch won't help.
> 
> Right, so my understanding of the situation is that doing this:
> 
>   $ git branch foo master~1
>   $ git fast-export foo master~1..master
> 
> won't show "foo", which seems wrong to me. _But_ we currently get that
> wrong already, so Felipe's patches are not making anything worse, but
> are fixing some situations (namely when master~1 is not mentioned on the
> command-line, but rather in a marks file).
> 
> Is that correct?
> 
> If so, then this series isn't regressing behavior; the only downside is
> that it's an incomplete fix. In theory this could get in the way of the
> full fix later on, but given the commit messages and the archive of this
> discussion, it would be simple enough to revert it later in favor of a
> more full fix. Is that accurate?

>From my understanding, yes.

> Sorry if I am belaboring the discussion. I just want to make sure I
> understand the situation before deciding what to do with the topic. It
> sounds like the consensus at this point is "not perfect, but good enough
> to make forward progress".

I appreciate that stance very much. The patch Sverre and I proposed was
also an incomplete fix (although I suspect it would fix the issue you
pointed out above), so I agree with the "perfect is the enemy of the good"
approach, obviously.

May I just ask to include a summary of that rationale into the commit
message rather than relying on people having internet access and knowing
where to look? Adding the following to the commit message would be good
enough for me:

	Note that

		$ git branch foo master~1
		$ git fast-export foo master~1..master

	still does not update the "foo" ref, but a partial fix is better
	than no fix.

Thanks,
Dscho