Re: [PATCH 0/3] log: create tighter default decoration filter

From: Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date: Jul 26, 2022, 14:44 UTC
Message-ID: <220726.86tu73ncf8.gmgdl@evledraar.gmail.com>
In-Reply-To: <pull.1301.git.1658844250.gitgitgadget@gmail.com>

On Tue, Jul 26 2022, Derrick Stolee via GitGitGadget wrote:

Show 13 quoted lines

> This was previously reduced by adding the log.excludeDecoration config
> option and modifying that config in git maintenance's prefetch task (to hide
> refs/prefetch/*). I then followed that pattern again for the bundle URI
> feature [1], but this caught some reviewers by surprise as an unfortunate
> side-effect. This series is a way to roll back the previous decision to use
> log.excludeDecoration and instead use tighter filters by default.
>
> As noted in the last patch, the current design ignores the new filters if
> there are any previously-specified filters. This includes the
> log.excludeDecorations=refs/prefetch/ set by the git maintenance command.
> This means that users who ran that command in their repo will not get the
> benefits of the more strict filters. While we stop writing
> log.excludeDecorations, we don't remove existing instances of it.

Leaving aside the question of these magic refs, and if we need new ones (e.g. refs/bundle/*) I have sometimes made use of out-of-standard refspace refs.

E.g. when I build git I create refs/built-tags/* tag object refs (i.e. not in refs/tags/*), which is a neat way to get "git tag -l" and the like to ignore it.

But to still have it show decorated in logs (e.g. I'll see what my "private" branch is at), and "for-each-ref --contains" still knows about it.

Now, that's a rather obscure use-case, and I suspect other "special refs" are similarly obscure (e.g. GitLab's refs/keep-around/* comes to mind).

But I think this change is going about it the wrong way, let's have a list of refs that Git knows about as magical, instead of assuming that we can ignore everything that's not on a small list of things we're including.

Wouldn't that give you what you want, and not exclude these sorts of custom refs unexpectedly for users?

> I'm interested if anyone has another way around this issue, or if we
> consider adding the default filter as long as no --decorate=refs options are
> specified.

I think the resulting UX here is bad, in that we ship hardcoded list of these if you don't specify the config in 2/3. So I can do:

      -c log.excludeDecoration=this-will-never-match

To "clear" the list, but not this:

      -c log.excludeDecoration=

Which we usually have as a synonym for "false", is it just adding a prefix of "" that matches everything to my exclusion list (but HEAD still shows up...).

Speaking of funny refnames isn't your:

	git -c log.excludeDecoration=HEAD log

Going to exclude refs like "HEAD/foo/bar" too? I.e. it's always a prefix.

I suspect a much better way out of this, which I think fixes the problem you're raising here is:

	log.useBuiltinDecorationExclusions=[bool]

Or whatever, i.e. we ship a hardcoded list of exclusions.

Leaving aside whether that's a blacklist or whitelist, or whether we turn that to "true" or "false" by default it wouldn't have the behavior of "flushing out" the built-in config.

Another way of doing that if you really want to use one variable would be to have:

      -c log.excludeDecoration=foo

Add to the built-in list, but:

      -c log.excludeDecoration=
      -c log.excludeDecoration=foo

clear it, we have a couple of other list config variables that work that way.