Re: [PATCH 4/5] gitweb: Add a feature for adding more branch refs

From: Jakub Narębski <jnareb@gmail.com>
Date: Dec 5, 2013, 11:40 UTC
Message-ID: <CANQwDwdfbAyJyCWwEXBDp5BGz1JvVj_eUn5NOCWEthUtKfF5Lg@mail.gmail.com>
In-Reply-To: <1386237635.2186.22.camel@localhost.localdomain>

On Thu, Dec 5, 2013 at 11:00 AM, Krzesimir Nowak <krzesimir@endocode.com> wrote:

> On Wed, 2013-12-04 at 19:06 +0100, Jakub Narębski wrote:
>> On Wed, Dec 4, 2013 at 2:43 PM, Krzesimir Nowak <krzesimir@endocode.com> wrote:

Show 27 quoted lines

>>> +The gitweb.extrabranchrefs is actually a multi-valued configuration
>>> +variable, so following example is also correct and the result is the
>>> +same as of the snippet above:
>>> ++
>>> +--------------------------------------------------------------------------------
>>> +[gitweb]
>>> +       extrabranchrefs = sandbox
>>> +       extrabranchrefs = wip other
>>> +--------------------------------------------------------------------------------
>>
>> I think this part should be better left for a separate patch. There is
>> important difference between single-valued and multi-valued configuration
>> variable: with single-valued later occurrences override earlier ones,
>> which includes settings in more specific config file (e.g. per-repository)
>> overriding setting in more general one (e.g. per-user or system-wide).
>>
>> With multi-valued we won't be able to override earlier / more generic
>> settings... well, unless we add support for no-value, or empty-value
>> as clearer, i.e.
>>
>>   [gitweb]
>>            extrabranchrefs = sandbox
>>            extrabranchrefs
>>  # or    extrabranchrefs =
>>            extrabranchrefs = wip other
>>
>> resulting in ('wip', 'other').

What I didn't notice is that gitweb already supports and *uses* multi-value configuration variables: gitweb.url and gitweb.ctag (note: the feature is called 'ctags' and is not overridable), though not for %feature-s.

So the point is moot, sorry for distraction.

BTW. there is config_to_multi() subroutine that you can use.

> That point in this example is a bit moot IMO - if you don't want sandbox
> ref to appear in list of branches view then just delete the offending
> line.

I was thinking about more complicated situation: if you have gitweb configured in "scan for repositories" mode (like mod_homedir), then repository owner may change gitweb configuration for own repository via repo config file. But gitweb reads also web server user config file and system wide git config file (/etc/gitconfig). If gitweb.extraBranchRefs is set in one of those files, repo owner cannot override, but only add to it.

But I think this is worrying about nothing. I'm sorry for distraction.

Show 11 quoted lines

> I also made a small experiment. In per-instance config I have
> $feature{'extra-branch-refs'}{'default'} = ['wip'];
> $feature{'extra-branch-refs'}{'override'} = 1;
>
> In per-repository config I have:
> [gitweb]
>         extrabranchrefs = sandbox
>         extrabranchrefs = other
>
> List of branches view shows only branches from sandbox and other. So
> clearly per-repository config overrides per-instance config.

That's why is called 'override'... but it is not what I was worrying about, see above.

-- 
Jakub Narebski