From: Junio C Hamano <gitster@pobox.com>
To: Dennis Kaarsemaker <dennis@kaarsemaker.net>
Cc: git@vger.kernel.org
Subject: Re: [PATCH] remote: make prune work for mixed mirror/non-mirror repos
Date: Thu, 20 Jun 2013 15:46:20 -0700 [thread overview]
Message-ID: <7vppvgpfib.fsf@alter.siamese.dyndns.org> (raw)
In-Reply-To: <1371766304-4601-1-git-send-email-dennis@kaarsemaker.net> (Dennis Kaarsemaker's message of "Fri, 21 Jun 2013 00:11:44 +0200")
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)?
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@kaarsemaker.net>
> ---
> 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;
next prev parent reply other threads:[~2013-06-20 22:46 UTC|newest]
Thread overview: 23+ messages / expand[flat|nested] mbox.gz Atom feed top
2013-06-20 21:23 [BUG?] remote prune origin interacts badly with clone --mirror and multiple remotes Dennis Kaarsemaker
2013-06-20 22:11 ` [PATCH] remote: make prune work for mixed mirror/non-mirror repos Dennis Kaarsemaker
2013-06-20 22:46 ` Junio C Hamano [this message]
2013-06-20 23:07 ` Dennis Kaarsemaker
2013-06-20 23:30 ` Junio C Hamano
2013-06-20 23:38 ` Dennis Kaarsemaker
2013-06-20 23:44 ` Junio C Hamano
2013-06-20 23:08 ` Jeff King
2013-06-20 23:29 ` Dennis Kaarsemaker
2013-06-20 23:36 ` Junio C Hamano
2013-06-20 22:53 ` [PATCH v2] " Dennis Kaarsemaker
2013-06-21 10:04 ` [PATCH 0/3] Handling overlapping refspecs slightly smarter Dennis Kaarsemaker
2013-06-21 10:04 ` [PATCH 1/3] remote: Add warnings about mixin --mirror and other remotes Dennis Kaarsemaker
2013-06-21 18:42 ` Junio C Hamano
2013-06-23 13:35 ` Dennis Kaarsemaker
2013-06-23 21:22 ` Junio C Hamano
2013-06-23 21:43 ` Dennis Kaarsemaker
2013-06-23 22:33 ` Junio C Hamano
2013-06-26 21:10 ` Dennis Kaarsemaker
2013-06-26 23:42 ` Junio C Hamano
2013-06-21 10:04 ` [PATCH 2/3] remote: Add test for prune and mixed --mirror and normal remotes Dennis Kaarsemaker
2013-06-21 10:04 ` [PATCH 3/3] remote: don't prune when detecting overlapping refspecs Dennis Kaarsemaker
2013-06-21 18:53 ` Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=7vppvgpfib.fsf@alter.siamese.dyndns.org \
--to=gitster@pobox.com \
--cc=dennis@kaarsemaker.net \
--cc=git@vger.kernel.org \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).