Jonathan Nieder wrote: > Brandon Williams wrote: >> When using protocol v2 a client constructs a list of ref-prefixes which >> are sent across the wire so that the server can do server-side filtering >> of the ref-advertisement. The logic that does this exists for both >> fetch and push (even though no push support for v2 currently exists yet) >> and is roughly the same so lets consolidate this logic and make it >> general enough that it can be used for both the push and fetch cases. >> >> Signed-off-by: Brandon Williams <bmwill@xxxxxxxxxx> >> --- >> builtin/fetch.c | 13 +------------ >> refspec.c | 29 +++++++++++++++++++++++++++++ >> refspec.h | 4 ++++ >> transport.c | 21 +-------------------- >> 4 files changed, 35 insertions(+), 32 deletions(-) > > I assume this is meant to be a refactoring with no functional change. > Alas, it's causing fetch-by-SHA-1 to fail for me: [...] > $ bin-wrappers/git -C /tmp/r -c protocol.version=2 fetch \ > https://kernel.googlesource.com/pub/scm/git/git \ > 6373cb598e1a4e0340583ad75d5abba01ff79774 > fatal: no matching remote head Backtrace: #0 0x0000555555671e09 in fetch_pack (args=0x7fffffffda80, fd=0x555555a6a9b0, conn=0x555555a65780, ref=0x0, dest=0x555555a64900 "https://kernel.googlesource.com/pub/scm/git/git";, sought=0x555555a69a80, nr_sought=1, shallow=0x555555a6a9d8, pack_lockfile=0x555555a631c8, version=protocol_v2) at fetch-pack.c:1562 #1 0x00005555557290b9 in fetch_refs_via_pack (transport=0x555555a63190, nr_heads=1, to_fetch=0x555555a69a80) at transport.c:326 #2 0x000055555572b712 in transport_fetch_refs (transport=0x555555a63190, refs=0x555555a65520) at transport.c:1247 #3 0x000055555559a2a6 in fetch_refs (transport=0x555555a63190, ref_map=0x555555a65520) at builtin/fetch.c:942 #4 0x000055555559ac3e in do_fetch (transport=0x555555a63190, rs=0x7fffffffdc30) at builtin/fetch.c:1157 #5 0x000055555559b535 in fetch_one (remote=0x555555a63870, argc=1, argv=0x7fffffffdff8, prune_tags_ok=0) at builtin/fetch.c:1398 #6 0x000055555559b9a3 in cmd_fetch (argc=1, argv=0x7fffffffdff8, prefix=0x0) at builtin/fetch.c:1487 #7 0x000055555556764b in run_builtin (p=0x555555a0fa30 <commands+816>, argc=3, argv=0x7fffffffdff0) at git.c:350 #8 0x000055555556795f in handle_builtin (argc=3, argv=0x7fffffffdff0) at git.c:565 #9 0x0000555555567b07 in run_argv (argcp=0x7fffffffde8c, argv=0x7fffffffde80) at git.c:617 #10 0x0000555555567cbc in cmd_main (argc=3, argv=0x7fffffffdff0) at git.c:694 #11 0x000055555560774c in main (argc=8, argv=0x7fffffffdfc8) at common-main.c:45 Both refs_tmp and transport->remote_refs are NULL. When refspec_ref_prefixes gets called, it produces the usual argv_array with 6373cb598e1a4e0340583ad75d5abba01ff79774, refs/6373cb598e1a4e0340583ad75d5abba01ff79774, etc. In the old code, this doesn't happen because we trigger the item->exact_sha1 test. Would something like the following make sense? It passes tests and appears to avoid trouble but I haven't looked any closer than that. Signed-off-by: Jonathan Nieder <jrnieder@xxxxxxxxx> --- i/refspec.c +++ w/refspec.c @@ -202,6 +202,8 @@ void refspec_ref_prefixes(const struct refspec *rs, const struct refspec_item *item = &rs->items[i]; const char *prefix = NULL; + if (item->exact_sha1) + continue; if (rs->fetch == REFSPEC_FETCH) prefix = item->src; else if (item->dst)
