* ref-in-want does not consider namespace
@ 2021-07-26 8:36 Kim Altintop
2021-07-26 11:55 ` Kim Altintop
0 siblings, 1 reply; 4+ messages in thread
From: Kim Altintop @ 2021-07-26 8:36 UTC (permalink / raw)
To: git; +Cc: jonathantanmy, bmwill
[-- Attachment #1: Type: text/plain, Size: 1278 bytes --]
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/
[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 249 bytes --]
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ref-in-want does not consider namespace
2021-07-26 8:36 ref-in-want does not consider namespace Kim Altintop
@ 2021-07-26 11:55 ` Kim Altintop
2021-07-26 16:44 ` Jonathan Tan
0 siblings, 1 reply; 4+ messages in thread
From: Kim Altintop @ 2021-07-26 11:55 UTC (permalink / raw)
To: git; +Cc: jonathantanmy, bwilliamseng
> 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.
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ref-in-want does not consider namespace
2021-07-26 11:55 ` Kim Altintop
@ 2021-07-26 16:44 ` Jonathan Tan
2021-07-27 9:11 ` Kim Altintop
0 siblings, 1 reply; 4+ messages in thread
From: Jonathan Tan @ 2021-07-26 16:44 UTC (permalink / raw)
To: kim; +Cc: git, jonathantanmy, bwilliamseng
> > 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/
^ permalink raw reply [flat|nested] 4+ messages in thread
* Re: ref-in-want does not consider namespace
2021-07-26 16:44 ` Jonathan Tan
@ 2021-07-27 9:11 ` Kim Altintop
0 siblings, 0 replies; 4+ messages in thread
From: Kim Altintop @ 2021-07-27 9:11 UTC (permalink / raw)
To: Jonathan Tan; +Cc: git, bwilliamseng
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?
^ permalink raw reply [flat|nested] 4+ messages in thread
end of thread, other threads:[~2021-07-27 9:12 UTC | newest]
Thread overview: 4+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-07-26 8:36 ref-in-want does not consider namespace Kim Altintop
2021-07-26 11:55 ` Kim Altintop
2021-07-26 16:44 ` Jonathan Tan
2021-07-27 9:11 ` Kim Altintop
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.