Re: [PATCH] Allow hideRefs to match refs outside the namespace

[Lukas Fleischer <lfleischer@lfos.de>]

I wrote this email on Thursday but it seems like it did not make it
through the mailing list. Resubmitting...

On Wed, 28 Oct 2015 at 17:21:59, Junio C Hamano wrote:
> Lukas Fleischer <lfleischer@lfos.de> writes:
> 
> > Right now, refs with a path outside the current namespace are replaced
> > by ".have" before passing them to show_ref() which in turn checks
> > whether the ref matches the hideRefs pattern. Move the check before the
> > path substitution in show_ref_cb() such that the hideRefs feature can be
> > used to hide specific refs outside the current namespace.
> >
> > Signed-off-by: Lukas Fleischer <lfleischer@lfos.de>
> > ---
> > The other show_ref() call sites are in show_one_alternate_sha1() and in
> > write_head_info(). The call site in show_one_alternate_sha1() is for
> > alternates and passes ".have". The other one is
> >
> >     show_ref("capabilities^{}", null_sha1);
> >
> > and is not relevant to the hideRefs feature. Note that this kind of
> > breaks backwards compatibility since the "magic" hideRefs patterns
> > ".have" and "capabilities^{}" no longer work, as explained in the
> > discussion.
> 
> If somebody is using namespaces and has "refs/frotz/" in the
> hiderefs configuration, we hide refs/frotz/ no matter which
> namespace is being accessed.  With this change, with the removal the
> check from show_ref(), wouldn't such a repository suddenly see a
> behaviour change?
> [...]

It would indeed. However, we cannot stay 100% backwards compatible when
adding support for matching refs outside the current namespace without
introducing new syntax. For example, if Git namespaces are in use (i.e.
GIT_NAMESPACE is set), "refs/namespaces/foo/refs/bar" in hideRefs would
not have hidden refs/namespaces/foo/refs/bar before the change but it
does afterwards. You might argue that nobody would have added
"refs/namespaces/foo/refs/bar" to hideRefs in the first place but
namespaces can be nested and it might be that the user meant to hide
refs/namespaces/bar/refs/namespaces/foo/refs/bar instead. Yes, those are
weird corner cases. But then again, I think that using hideRefs with
namespaces already is a corner case as well. I also think that using the
same syntax to match both original and stripped refs is bad design. It
makes things complicated and the resulting feature doesn't have the full
expressive power of the simpler version only matching original refs.

So, we can either intentionally break backwards compatibility for some
rare corner cases, or keep the current behavior and introduce some new
syntax for matching the original (unstripped) refs. For the latter, we
could either introduce a new option ("hideUnstrippedRefs"?) or special
syntax inside hideRefs ("/refs/foo" instead of "refs/foo" for matching
the unstripped version only?) What do you think?