Re: [PATCH] remote: make prune work for mixed mirror/non-mirror repos

[Dennis Kaarsemaker <dennis@kaarsemaker.net>]

(Sorry, I sent v2 before seeing this mail)

On do, 2013-06-20 at 15:46 -0700, Junio C Hamano wrote:
> Dennis Kaarsemaker <dennis@kaarsemaker.net> writes:
> 
> > When cloning a repo with --mirror, and adding more remotes later,
> > get_stale_heads for origin would mark all refs from other repos as stale. In
> > this situation, with refs-src and refs->dst both equal to refs/*, we should
> > ignore refs/remotes/* when looking for stale refs to prevent this from
> > happening.
> 
> I do not think it is a right solution to single out refs/remotes/*.
> 
> Going back to your original example:
> 
>     [remote "origin"]
>             url = git://github.com/git/git.git
>             fetch = +refs/*:refs/*
>             mirror = true
>     [remote "peff"]
>             url = git://github.com/peff/git.git
>             fetch = +refs/heads/*:refs/remotes/peff/*
> 
> Wouldn't you obtain "refs/remotes/github/html" from your "origin"
> via "git pull origin"?  What happens to your local copy of that ref,
> when it goes away from the origin and then you try to "fetch --prune
> origin" the next time with this patch (and without this patch)?

git pull origin gives me refs/html in this case. I did not try fetch
--prune, but prune origin DTRT: if the html branch goes away at the
origin, it goes away locally. Both with and without this patch.

It's refs/remotes/peff/somebranch that in this case *also* goes away
without this patch, but is untouched with this patch

> What should happen?

Exactly that.

> What if you had this instead of the above version of remote.peff.*?
> 
>     [remote "peff"]
>             url = git://github.com/peff/git.git
>             fetch = +refs/heads/*:refs/remotes/github/*

That doesn't change anything.

> I think this is an unsolvable problem, and I _think_ the root cause
> of the issue is the configuration above that allows the RHS of
> different fetch refspecs to overlap.  refs/* is more generic and
> covers refs/remotes/peff/* and refs/remotes/github/*.  You cannot
> even know, just by looking at "origin" and your local repository,
> if refs/remotes/github/html you have should go away or it might have
> come from somewhere else.
> 
> The best we _could_ do, without contacting all the defined remotes,
> is probably to check each ref that we did not see from "origin" (for
> example, you find "refs/remotes/peff/frotz" that your origin does
> not have) and see if it could match RHS of fetch refspec of somebody
> else (e.g. RHS of "refs/heads/*:refs/remotes/peff/*" matches that
> ref).  Then we can conclude that refs/remotes/peff/frotz _might_
> have come from Peff's repository and not from "origin", and then we
> can optionally issue a warning and refrain from removing it.

I like that idea, though I also like the simplicity of simply singling
out "remotes" as that's where normal remotes usually sit. And don't
forget about tags (see patch v2).

> This inevitably will have false positives and leave something that
> did originally came from "origin", because peff may no longer have
> 'frotz' branch in his repository.  I do not think we can do better
> than that, because we are trying to see if we can improve things
> without having to contact all the remotes.

But then the ref would have to be called "refs/remotes/peff/frotz"
upstream. Hmm, that is of course completely possible: cloning something
that's already a clone.

> But if you go that route, the logic needs to go the same way when
> you are pruning against 'peff', and anything that you do not see in
> his repository right now but you have in refs/remotes/peff/ cannot
> be pruned, because it might have come from your origin via more
> generic refs/*:refs/* mapping.  It follows that you could never
> prune anything under refs/remotes/peff/* hierarchy.
> 
> You could introduce a "assume that more specific mapping never
> overlaps with a more generic mapping" rule (i.e. refs/* from RHS of
> remote.origin.fetch is more generic than refs/remotes/peff/* from
> RHS of remote.peff.fetch, and assume everything that you see in your
> local refs/remotes/peff/* came from peff and not from origin, I
> think, but at that point, is it worth the possible complexity to
> code that rule in the prune codepath and brittleness of that
> assumption that your origin will never add a new ref under that
> hierarchy, e.g. refs/remotes/peff/xyzzy?
> 
> So, I dunno.

Yeah, I'm starting to think this is not such a good idea. How about plan
B: issuing a warning when adding a remote with a refspec that also
matches another remote's refspec?

Or plan C: add a per-remote pruneIgnore setting that in this case I
could set to refs/tags/* refs/remotes/* as I know it's correct? Could
even be combined with plan B.

-- 
Dennis Kaarsemaker
www.kaarsemaker.net