If a user fetches refs/heads/master from a repo with namespace "ns", the remote is expected to (1) not send the real refs/heads/master, and (2) send refs/namespaces/ns/refs/heads/master with the name refs/heads/master. (1) indeed happens now, but not (2) - Git only sends refs that have the user-given prefix, but it checks them against the full name of the ref (the one starting with refs/namespaces), and not the namespace-stripped one. This is demonstrated by the patch in the test. Currently, it results in "fatal: couldn't find remote ref refs/heads/master" despite both unnamespaced and namespaced master being present. With the code change, it produces the expected result. Check the ref prefixes against the namespace-stripped name. This bug was discovered through applying patches [1] that override protocol.version to 2 in repositories when running tests, allowing us to notice differences in behavior across different protocol versions. [1] https://public-inbox.org/git/cover.1547677183.git.jonathantanmy@xxxxxxxxxx/ Signed-off-by: Jonathan Tan <jonathantanmy@xxxxxxxxxx> --- Changes from v1: - updated commit message to explain from the end user viewpoint - wrote the test using end-user-facing Git commands Thanks, Junio, for your review. > OK, so "ls-refs: filter based on non-namespace name" (in the title) is a > means to the objective 'ls-refs: make sure it honors namespaces" > which is a bugfix? Yes and no. I've updated the commit message to explain that it honors namespaces in that it doesn't send the wrong refs, but it doesn't honor them in that it doesn't send the right refs. > The new test peeks at the protocol level, but wouldn't we be able to > see the breakage by running ls-remote or something and observing its > result as well, or is the bug only observable with test-tool and not > triggerable by end-user facing git commands? ls-remote wouldn't work as its filtering is incompatible with ref-prefix (see 631f0f8c4b ("ls-remote: do not send ref prefixes for patterns", 2018-10-31)), but it can be observed with fetch. I've replaced the test with one that uses fetch instead. --- ls-refs.c | 2 +- t/t5702-protocol-v2.sh | 21 +++++++++++++++++++++ 2 files changed, 22 insertions(+), 1 deletion(-) diff --git a/ls-refs.c b/ls-refs.c index a06f12eca8..7782bb054b 100644 --- a/ls-refs.c +++ b/ls-refs.c @@ -40,7 +40,7 @@ static int send_ref(const char *refname, const struct object_id *oid, const char *refname_nons = strip_namespace(refname); struct strbuf refline = STRBUF_INIT; - if (!ref_match(&data->prefixes, refname)) + if (!ref_match(&data->prefixes, refname_nons)) return 0; strbuf_addf(&refline, "%s %s", oid_to_hex(oid), refname_nons); diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh index 0f2b09ebb8..3d802aa587 100755 --- a/t/t5702-protocol-v2.sh +++ b/t/t5702-protocol-v2.sh @@ -514,6 +514,27 @@ test_expect_success 'fetch with http:// using protocol v2' ' grep "git< version 2" log ' +test_expect_success 'fetch from namespaced repo respects namespaces' ' + test_when_finished "rm -f log" && + + git init "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" && + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" one && + test_commit -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" two && + git -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" \ + update-ref refs/namespaces/ns/refs/heads/master one && + + GIT_TRACE_PACKET="$(pwd)/log" git -C http_child -c protocol.version=2 \ + fetch "$HTTPD_URL/smart_namespace/nsrepo" \ + refs/heads/master:refs/heads/theirs && + + # Server responded using protocol v2 + grep "fetch< version 2" log && + + git -C "$HTTPD_DOCUMENT_ROOT_PATH/nsrepo" rev-parse one >expect && + git -C http_child rev-parse theirs >actual && + test_cmp expect actual +' + test_expect_success 'push with http:// and a config of v2 does not request v2' ' test_when_finished "rm -f log" && # Till v2 for push is designed, make sure that if a client has -- 2.19.0.271.gfe8321ec05.dirty
