Lukas Fleischer [off-list ref] writes:
2. transfer.hideRefs and receive.hideRefs do not seem to work with Git
namespaces in general. show_ref_cb() replaces each ref outside the
current namespace with ".have" before passing it to show_ref() which
in turn performs the ref_is_hidden() check. This has the nice side
effect that receive.hideRefs=.have does exactly what I want, however
it also means that hideRefs feature does not allow for excluding only
specific tags outside the current namespace. Is that intended? Can we
rely on Git always looking for ".have" in the hideRefs list in this
case?
When I asked 'Is transfer.hiderefs insufficient?', I wasn't
expecting it to be usable out of box. It was a suggestion to build
on top of it, instead of adding a parallel support for something
specific to namespaces.
For example, if the problem is that you cannot tell ref_is_hidden()
what namespace the ref is from because it is called after running
strip_namespace(), perhaps you can find a way to have the original
"namespaced ref" specified on transfer.hiderefs and match them?
Then in repository for project A, namespaced refs for project B can
be excluded by specifying refs/namespaces/B/* on transfer.hiderefs.
Perhaps along the lines of this?
builtin/receive-pack.c | 9 +++++++++
1 file changed, 9 insertions(+)
diff --git a/builtin/receive-pack.c b/builtin/receive-pack.c
index bcb624b..db0a99d 100644
--- a/builtin/receive-pack.c
+++ b/builtin/receive-pack.c
@@ -221,6 +221,15 @@ static void show_ref(const char *path, const unsigned char *sha1)
static int show_ref_cb(const char *path, const struct object_id *oid, int flag, void *unused)
{
+ const char *ns = get_git_namespace();
+
+ /*
+ * Give the "hiderefs" mechanism a chance to inspect and
+ * reject the namespaced ref itself.
+ */
+ if (ns[0] && ref_is_hidden(path))
+ return 0;
+
path = strip_namespace(path);
/*
* Advertise refs outside our current namespace as ".have"On Tue, 27 Oct 2015 at 19:18:26, Junio C Hamano wrote:
[...]
When I asked 'Is transfer.hiderefs insufficient?', I wasn't
expecting it to be usable out of box. It was a suggestion to build
on top of it, instead of adding a parallel support for something
specific to namespaces.
Agreed, and I do have a couple of patches to improve hideRefs. I still
have some questions before submitting them, though. See below.
For example, if the problem is that you cannot tell ref_is_hidden()
what namespace the ref is from because it is called after running
strip_namespace(), perhaps you can find a way to have the original
"namespaced ref" specified on transfer.hiderefs and match them?
Then in repository for project A, namespaced refs for project B can
be excluded by specifying refs/namespaces/B/* on transfer.hiderefs.
Perhaps along the lines of this?
[...]
My original question remains: Do we want to continue supporting things
like transfer.hideRefs=.have (which currently magically hides all refs
outside the current namespace)? For 100% backwards compatibility, we
would have to. On the other hand, one could consider the current
behavior a bug and one could argue that it is weird enough that probably
nobody (apart from me) relies on it right now. If we decide to keep it
anyway, I think it should be documented.
Another patch I have in my patch queue adds support for a whitelist mode
to hideRefs. There are several ways to implement that:
1. Make transfer.hideRefs='' hide all refs (it currently does not). The
user can then whitelist refs explicitly using negative patterns
below that rule. This is how my current implementation works. Using
the empty string seemed most natural since hideRefs matches prefixes
and every string has the empty string as a prefix. If that seems too
weird, we could probably special case something like
transfer.hideRefs='*' instead.
2. Detect whether hideRefs only contains negative patterns. Switch to
whitelist mode ("hide by default") in that case.
3. Add another option to switch between "hide by default" and "show by
default".
I personally prefer the first option. Any other opinions?
On Wed, Oct 28, 2015 at 08:00:45AM +0100, Lukas Fleischer wrote:
My original question remains: Do we want to continue supporting things
like transfer.hideRefs=.have (which currently magically hides all refs
outside the current namespace)? For 100% backwards compatibility, we
would have to. On the other hand, one could consider the current
behavior a bug and one could argue that it is weird enough that probably
nobody (apart from me) relies on it right now. If we decide to keep it
anyway, I think it should be documented.
I don't think that hiding ".have" refs at that level is especially
useful. I do not use namespaces, but I do use alternates extensively,
and that is the original source of these ".have" refs. But filtering
them at the advertisement layer is very inefficient, as it is expensive
to get the list in the first place (we spawn ls-remote, which spawns
upload-pack in the alternate!). So we'd want to prevent that process
much earlier.
I have an unpublished patch to specially disable alternates
advertisement entirely (i.e., adding a new boolean config,
receive.advertiseAlternates). In my case, it is because the alternates
repositories have huge numbers of refs (sometimes ranging into the
gigabytes) and the performance hit on even loading that packed-refs file
is too large.
I suppose that behavior _could_ be triggered by ".have" appearing in the
hiderefs config, though (i.e., before accessing the alternate, check
ref_is_hidden(".have")). That seems a bit too subtle to me, though.
Another patch I have in my patch queue adds support for a whitelist mode
to hideRefs. There are several ways to implement that:
1. Make transfer.hideRefs='' hide all refs (it currently does not). The
user can then whitelist refs explicitly using negative patterns
below that rule. This is how my current implementation works. Using
the empty string seemed most natural since hideRefs matches prefixes
and every string has the empty string as a prefix. If that seems too
weird, we could probably special case something like
transfer.hideRefs='*' instead.
2. Detect whether hideRefs only contains negative patterns. Switch to
whitelist mode ("hide by default") in that case.
3. Add another option to switch between "hide by default" and "show by
default".
I personally prefer the first option. Any other opinions?
I am just a bystander and would not use this myself, but I think the 1st
is the least ugly. I am not sure why ignoring "refs/" does not work,
though (it does not catch ".have", of course, but I think that is a
feature; there are a finite set of pseudo-refs, so you can ignore those,
too, if you want).
-Peff