Re: [PATCH v3 6/6] commit-graph: show usage on "commit-graph [write|verify] garbage"

On Tue, Jul 20, 2021 at 11:17:13PM +0200, Ævar Arnfjörð Bjarmason wrote:

quoted

So we should definitely fix this instance via a reroll of this series,
but that still leaves the others up for grabs. Ævar, are those (the ones
in the 'multi-pack-index' and 'env--helper' builtins) something that you
want to clean up while you're working in this area, or would you rather
that I take care of it?

I don't mind either way, just want to make sure that we don't duplicate
effort.

I'm all for you picking it up :)

If you wanted to pick up these patches (or some of them) and
partially/entirely replace this series I'd be happy with that too,
i.e. if it makes conflicts etc. easier.

I think either is fine; there shouldn't be any conflicts in the
multi-pack-index code just eyeballing based on what you wrote.

I started working on it (which I suppose can count for me volunteering
to patch it up myself ;-)), but I wondered why we have env--helper at
all. When you wrote it back in b4f207f339 (env--helper: new undocumented
builtin wrapping git_env_*(), 2019-06-21), you said that it wasn't added
as a test-tool because it had some uses outside of tests.

But I can't find any locations. We do use env--helper (via
test_bool_env, which you also introduced) in a couple of t/lib-*.sh
files, but this would be far from the first test-tool that has been used
in a test library.

So ISTM that this could be converted to a test-tool and removed from the
list of builtins. *And* we could do that without a deprecation warning,
because it was never documented in the first place.

Can you double check my thinking and/or let me know if there is a
compelling reason to keep it as a builtin?

I just re-submitted this now because it's been staring at me in my
"should re-roll at somep point" list for a while...

FWIW if you're poking at this area more generally we really could do
with some standardization around these built-in sub-commands:

    git built-in --here subcommand
    git built-in subcommand --or-here

That's probably a step too far for this loose end for me, so if you want
to work on that please be my guest, but I probably don't have time for
it in the near future.

Thanks,
Taylor