Re: [PATCH/RFC] receive-pack: allow for hiding refs outside the namespace

[Jeff King <peff@peff.net>]

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