| Message ID | 3dc6ae936c88d3105bc82daab3edd805c9b5c63b.1726476401.git.ps@pks.im (mailing list archive) |
|---|---|
| State | Accepted |
| Commit | d8faf50c36e94bdf47f57e2f1e7ea1b767e0ea00 |
| Headers | show |
| Series | refs/reftable: wire up exclude patterns | expand |
On Mon, Sep 16, 2024 at 10:50:05AM +0200, Patrick Steinhardt wrote: > diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c > index 3f35140e489..478c62ca836 100644 > --- a/builtin/receive-pack.c > +++ b/builtin/receive-pack.c > @@ -339,12 +339,26 @@ static void show_one_alternate_ref(const struct object_id *oid, > static void write_head_info(void) > { > static struct oidset seen = OIDSET_INIT; > + struct strvec excludes_vector = STRVEC_INIT; > + const char **exclude_patterns; > + > + /* > + * We need access to the reference names both with and without their > + * namespace and thus cannot use `refs_for_each_namespaced_ref()`. We > + * thus have to adapt exclude patterns to carry the namespace prefix > + * ourselves. > + */ > + exclude_patterns = get_namespaced_exclude_patterns( > + hidden_refs_to_excludes(&hidden_refs), > + get_git_namespace(), &excludes_vector); OK, so here we use the result of calling hidden_refs_to_excludes() as the first argument to your new get_namespaced_exclude_patterns(). But I think that in this case when the caller specifies a pattern with '^', we still do not exclude any references in the backend, since hidden_refs_to_excludes() will return NULL when there is >1 pattern with '^' as the first character. I don't think that this results in broken behavior, since the callback to the refs API will still be expected to filter out references that it doesn't want. > refs_for_each_fullref_in(get_main_ref_store(the_repository), "", > - hidden_refs_to_excludes(&hidden_refs), > - show_ref_cb, &seen); > + exclude_patterns, show_ref_cb, &seen); > for_each_alternate_ref(show_one_alternate_ref, &seen); Thanks, Taylor
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c index 3f35140e489..478c62ca836 100644 --- a/builtin/receive-pack.c +++ b/builtin/receive-pack.c @@ -339,12 +339,26 @@ static void show_one_alternate_ref(const struct object_id *oid, static void write_head_info(void) { static struct oidset seen = OIDSET_INIT; + struct strvec excludes_vector = STRVEC_INIT; + const char **exclude_patterns; + + /* + * We need access to the reference names both with and without their + * namespace and thus cannot use `refs_for_each_namespaced_ref()`. We + * thus have to adapt exclude patterns to carry the namespace prefix + * ourselves. + */ + exclude_patterns = get_namespaced_exclude_patterns( + hidden_refs_to_excludes(&hidden_refs), + get_git_namespace(), &excludes_vector); refs_for_each_fullref_in(get_main_ref_store(the_repository), "", - hidden_refs_to_excludes(&hidden_refs), - show_ref_cb, &seen); + exclude_patterns, show_ref_cb, &seen); for_each_alternate_ref(show_one_alternate_ref, &seen); + oidset_clear(&seen); + strvec_clear(&excludes_vector); + if (!sent_capabilities) show_ref("capabilities^{}", null_oid()); diff --git a/t/t5509-fetch-push-namespaces.sh b/t/t5509-fetch-push-namespaces.sh index 98e8352b6cc..f029ae0d286 100755 --- a/t/t5509-fetch-push-namespaces.sh +++ b/t/t5509-fetch-push-namespaces.sh @@ -124,6 +124,14 @@ test_expect_success 'try to update a ref that is not hidden' ' git -C original push pushee-namespaced main ' +test_expect_success 'git-receive-pack(1) with transfer.hideRefs does not match unstripped refs during advertisement' ' + git -C pushee update-ref refs/namespaces/namespace/refs/heads/foo/1 refs/namespaces/namespace/refs/heads/main && + git -C pushee pack-refs --all && + test_config -C pushee transfer.hideRefs refs/namespaces/namespace/refs/heads/foo && + GIT_TRACE_PACKET="$(pwd)/trace" git -C original push pushee-namespaced main && + test_grep refs/heads/foo/1 trace +' + test_expect_success 'try to update a hidden full ref' ' test_config -C pushee transfer.hideRefs "^refs/namespaces/namespace/refs/heads/main" && test_must_fail git -C original push pushee-namespaced main
In `write_head_info()` we announce references to the remote client. We need to honor "transfer.hideRefs" here so that we do not announce any references that the client shouldn't be able to learn about. This is done via two separate mechanisms: - We hand over exclude patterns to the reference backend. We can only honor "plain" exclude patterns here that do not have prefixes with special meaning such as "^" or "!". Filtering down the references is handled by `hidden_refs_to_excludes()`. - In `show_ref_cb()` we perform a second check against hidden refs. For one this is done such that we can handle those special prefixes. And second, handling exclude patterns in ref backends is optional, so we also have to handle "normal" patterns. The special-meaning "^" prefix alters whether a hidden ref applies to the namespace-stripped reference name or the full name. So while we would usually call `refs_for_each_namespaced_ref()` to only get those references in the current namespace, we can't because we'd get the already-rewritten reference names. Instead, we are forced to use `refs_for_each_fullref_in()` and then manually strip away the namespace prefix such that we have access to both names. But this also means that we do not get namespace handling for exclude patterns, which `refs_for_each_namespaced_ref()` brings for free. This results in a bug because we potentially end up hiding away references based on their namespaced name and not on the stripped name as we really should be doing. Fix this by manually rewriting the exclude patterns to their namespaced variants. Signed-off-by: Patrick Steinhardt <ps@pks.im> --- builtin/receive-pack.c | 18 ++++++++++++++++-- t/t5509-fetch-push-namespaces.sh | 8 ++++++++ 2 files changed, 24 insertions(+), 2 deletions(-)