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

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