git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[BUG] "fetch $there +refs/*:refs/*" fails if there is a stash

From
Junio C Hamano <gitster@pobox.com>
Date
May 1, 2012, 22:19 UTC
Message-ID
<7vsjfj7des.fsf@alter.siamese.dyndns.org>
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/") )) {
Next: Michael Haggerty
Message 1 of 10 in “[BUG] "fetch $there +refs/*:refs/*" fails if there is a stash”
  1. Junio C HamanoMay 1, 2012
  2. Michael HaggertyMay 2, 2012
  3. Junio C HamanoMay 2, 2012
  4. fetch/push: allow refs/*:refs/*Junio C Hamano, May 4, 2012
  5. get_fetch_map(): tighten checks on dest refsJunio C Hamano, May 4, 2012
  6. Michael HaggertyMay 5, 2012
  7. Michael HaggertyMay 5, 2012
  8. Junio C HamanoMay 7, 2012
  9. Michael HaggertyMay 15, 2012
  10. Junio C HamanoMay 15, 2012

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.