git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Junio C Hamano <gitster@pobox.com>
To: git@vger.kernel.org
Cc: Michael Haggerty <mhagger@alum.mit.edu>
Subject: [BUG] "fetch $there +refs/*:refs/*" fails if there is a stash
Date: Tue, 01 May 2012 15:19:07 -0700	[thread overview]
Message-ID: <7vsjfj7des.fsf@alter.siamese.dyndns.org> (raw)

This ought to work:

    $ git checkout HEAD^0 ;# make sure we are on detached HEAD
    $ git fetch $somewhere +refs/*:refs/*

if you want to pull down all the branches from somewhere, potentially
nuking the refs we currently have.

However, if $somewhere has a stash, i.e. refs/stash, even though our
"fetch" sees it in the "ls-remote $somewhere" output and tries to make
sure that the object comes over the wire, we never show "want" line for
refs/stash, because we silently drop it with check_ref_format().

I have run out concentration before digging this through, but the attached
single liner patch fixes it.  The thing is, this function is given a list
of refs and drops refs/stash (which is not what I want in the context of
the above +refs/*:refs/*), and modifies the caller's list of refs, and
then the caller (i.e. do_fetch_pack() that called everything_local()) then
uses updated list (i.e. without refs/stash) to run find_common() and
get_pack(), but the layer higher above (namely, do_fetch() that calls
fetch_refs() that in turn calls store_updated_refs(), all in
builtin/fetch.c) is _not_ aware of this filtering and expects that the
code at this layer *must* have asked for and obtained all the objects
reachable from what was listed in ls-remote output, leading to an
inconsistent state.

[Michael CC'ed as he seems to be dead set tightening check_ref_format()]

The specific failure of "refs/stash" is fixed with this patch, but I think
this call to check_ref_format() in the filter_refs() should be removed.
The function is trying to see what we _asked_ against what the remote side
said they have, and if we tried to recover objects from a broken remote by
doing:

	git fetch $somewhere refs/heads/a..b:refs/heads/a_dot_dot_b

and the remote side advertised that it has such a ref (note that a..b is
an illegal path component), we should be able to do so without a misguided
call to check_refname_format() getting in the way.

It is the same story if the name advertised by a broken remote were
"refs/head/../heads/master".  As long as the RHS of the refspec
(i.e. refs/heads/a_dot_dot_b) is kosher, we must allow such a request to
succeed, so that people can interoperate in less than perfect world.

 builtin/fetch-pack.c | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/builtin/fetch-pack.c b/builtin/fetch-pack.c
index 7124c4b..34cd8a5 100644
--- a/builtin/fetch-pack.c
+++ b/builtin/fetch-pack.c
@@ -541,7 +541,7 @@ static void filter_refs(struct ref **refs, int nr_match, char **match)
 	for (ref = *refs; ref; ref = next) {
 		next = ref->next;
 		if (!memcmp(ref->name, "refs/", 5) &&
-		    check_refname_format(ref->name + 5, 0))
+		    check_refname_format(ref->name, 0))
 			; /* trash */
 		else if (args.fetch_all &&
 			 (!args.depth || prefixcmp(ref->name, "refs/tags/") )) {

             reply	other threads:[~2012-05-01 22:19 UTC|newest]

Thread overview: 10+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-05-01 22:19 Junio C Hamano [this message]
2012-05-02  9:15 ` [BUG] "fetch $there +refs/*:refs/*" fails if there is a stash Michael Haggerty
2012-05-02 16:26   ` Junio C Hamano
2012-05-04 22:30 ` [PATCH] fetch/push: allow refs/*:refs/* Junio C Hamano
2012-05-04 22:35   ` [PATCH] get_fetch_map(): tighten checks on dest refs Junio C Hamano
2012-05-05  6:03   ` [PATCH] fetch/push: allow refs/*:refs/* Michael Haggerty
2012-05-05 18:22     ` Michael Haggerty
2012-05-07 16:24     ` Junio C Hamano
2012-05-15  8:49       ` Michael Haggerty
2012-05-15 15:19         ` 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=7vsjfj7des.fsf@alter.siamese.dyndns.org \
    --to=gitster@pobox.com \
    --cc=git@vger.kernel.org \
    --cc=mhagger@alum.mit.edu \
    /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).