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

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

 



Dennis Kaarsemaker <dennis@xxxxxxxxxxxxxxx> 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)?

What should happen?

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/*

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.

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

> Signed-off-by: Dennis Kaarsemaker <dennis@xxxxxxxxxxxxxxx>
> ---
>  remote.c | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
>
> diff --git a/remote.c b/remote.c
> index e71f66d..863c183 100644
> --- a/remote.c
> +++ b/remote.c
> @@ -1884,6 +1884,7 @@ struct stale_heads_info {
>  	struct ref **stale_refs_tail;
>  	struct refspec *refs;
>  	int ref_count;
> +	int ignore_remotes;
>  };
>  
>  static int get_stale_heads_cb(const char *refname,
> @@ -1903,7 +1904,8 @@ static int get_stale_heads_cb(const char *refname,
>  	 * remote we consider it to be stale.
>  	 */
>  	if (!((flags & REF_ISSYMREF) ||
> -	      string_list_has_string(info->ref_names, query.src))) {
> +	      string_list_has_string(info->ref_names, query.src) ||
> +	      (info->ignore_remotes && !prefixcmp(refname, "refs/remotes/")))) {
>  		struct ref *ref = make_linked_ref(refname, &info->stale_refs_tail);
>  		hashcpy(ref->new_sha1, sha1);
>  	}
> @@ -1917,6 +1919,8 @@ struct ref *get_stale_heads(struct refspec *refs, int ref_count, struct ref *fet
>  	struct ref *ref, *stale_refs = NULL;
>  	struct string_list ref_names = STRING_LIST_INIT_NODUP;
>  	struct stale_heads_info info;
> +	if(!strcmp(refs->src, "refs/*") && !strcmp(refs->dst, "refs/*"))
> +		info.ignore_remotes = 1;
>  	info.ref_names = &ref_names;
>  	info.stale_refs_tail = &stale_refs;
>  	info.refs = refs;
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html




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