threads / discuss / 56160

ref-in-want does not consider namespace

Subject: ref-in-want does not consider namespace

## tl;dr

4 messages between Jul 26, 2021 and Jul 27, 2021.

replies: 3people: 2as markdown or json

Kim Altintop· Jul 26, 2021, 08:36 UTC · lore

Hello,

I am experimenting with the ref-in-want feature using a custom client and plain `git-upload-pack` on the server end. To my suprise, it turned out that fetching from a namespaced repository requires the `want-ref` lines to specify the refs in namespaced form. That is:

Say I have the ref `refs/namespaces/foo/refs/heads/main`. I launch upload-pack with `--namespace=foo`. Running `ls-refs` with a namespace-relative

  `ref-prefix refs/heads/*`

will work as expected, but sendingr

  `want-ref refs/heads/main`

will result in

  `ERR unknown ref refs/heads/main`

Sending

  `want-ref refs/namespaces/foo/refs/heads/main`

works, and the corresponding `wanted-refs` section will contain the ref as given (ie. including the namespace path).

Since my understanding of git namespaces is that they should be transparent to the client, I find this surprising.

I dug up an early version of a patch introducing the feature, which did handle namespaces [0], but what eventually landed (authored by Brandon Williams) didn't.

Was this an oversight, or is there a specific reason for the behaviour?

Thanks, Kim

[0]: https://lore.kernel.org/git/d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com/

Kim Altintop· Jul 26, 2021, 11:55 UTC · re: Kim Altintop · lore

Re: ref-in-want does not consider namespace

Show 5 quoted lines

> Running `ls-refs` with a namespace-relative
>
> `ref-prefix refs/heads/*`
>
> will work as expected

That turned out to not be entirely true: it does work as expected with a 2.31.1 install, but fails to select any refs with 2.20.1.

Jonathan Tan· Jul 26, 2021, 16:44 UTC · re: Kim Altintop · lore

Re: ref-in-want does not consider namespace

Show 8 quoted lines

> > Running `ls-refs` with a namespace-relative
> >
> > `ref-prefix refs/heads/*`
> >
> > will work as expected
> 
> That turned out to not be entirely true: it does work as expected with a
> 2.31.1 install, but fails to select any refs with 2.20.1.

Your expectation that "want-ref refs/heads/main" should match "refs/namespaces/foo/refs/heads/main" when you run upload-pack with "--namespace=foo" makes sense to me. Just to clarify - does this mean tha 2.31.1 works like you expect?

In any case, t5703 should contain a namespace test, but it doesn't seem to. We could just use the one in my original patch [1] (thanks for looking it up, by the way). Would you be able to contribute a patch? I see that in [1], I had my own helpers to construct the input, but "test-tool pkt-line" now exists (as you can see in t5703) so you can do that instead.

[1] https://lore.kernel.org/git/d0d42b3bb4cf755f122591e191354c53848f197d.1485381677.git.jonathantanmy@google.com/

Kim Altintop· Jul 27, 2021, 09:11 UTC · re: Jonathan Tan · lore

Re: ref-in-want does not consider namespace

Thanks Jonathan

> Your expectation that "want-ref refs/heads/main" should match
> "refs/namespaces/foo/refs/heads/main" when you run upload-pack with
> "--namespace=foo" makes sense to me. Just to clarify - does this mean
> tha 2.31.1 works like you expect?

"ref-prefix" works as expected in the newer version, but not "want-ref". I haven't properly bisected, but the fix seems to coincide with b3970c7, although I'm not sure if that was intentional.

> In any case, t5703 should contain a namespace test, but it doesn't seem
> to.

Yes I noticed that your original patch did contain a test for this.

> Would you be able to contribute a patch?

With some handholding, I'd be at least up for giving it a spin :)

One thing that is unclear to me is how "hideRefs" is supposed to interact with "want-ref". At the moment, it seems like a client can "want-ref" _any_ ref, even if it is hidden by "hideRefs". It might seem sensible to ignore hidden refs, but then the situation could occur that the effective list of "want-ref"s is empty.

If that happens, and the client did not send any "want"s either, "upload-pack" ignores the fetch command altogether and just waits for the next command. In this case, the client can only recover by timing out the connection.

So should this be special-cased somehow and result in an error response?