ab/only-single-progress-at-once (was: What's cooking in git.git (Dec 2021, #03; Fri, 10))

From: "Ævar Arnfjörð Bjarmason" <avarab@gmail.com>
To: Junio C Hamano <gitster@pobox.com>
Cc: git@vger.kernel.org, "SZEDER Gábor" <szeder.dev@gmail.com>
Subject: ab/only-single-progress-at-once (was: What's cooking in git.git (Dec 2021, #03; Fri, 10))
Date: Sun, 12 Dec 2021 19:41:38 +0100	[thread overview]
Message-ID: <211212.86sfuxac8c.gmgdl@evledraar.gmail.com> (raw)
In-Reply-To: <xmqqilvvluoa.fsf@gitster.g>

On Fri, Dec 10 2021, Junio C Hamano wrote:

> * ab/only-single-progress-at-once (2021-11-03) 8 commits
>  - progress.c: add & assert a "global_progress" variable
>  - various *.c: use isatty(0|2), not isatty(STDIN_FILENO|STDERR_FILENO)
>  - pack-bitmap-write.c: don't return without stop_progress()
>  - progress.c: add temporary variable from progress struct
>  - progress.c tests: test some invalid usage
>  - progress.c tests: make start/stop commands on stdin
>  - progress.c test helper: add missing braces
>  - leak tests: fix a memory leaks in "test-progress" helper
>
>  Further tweaks on progress API.
>
>  Needs review.
>  The last three rounds has seen little reaction, even though earlier
>  round saw a lot of responses. The latest round needs a serious
>  review or at least Acks from past commentors.
>  source: <cover-v6-0.8-00000000000-20211102T122507Z-avarab@gmail.com>

I think less in "needs review" and more in needing a tiebreaker and/or
reply to my [1]. I.e. the ongoing discussion SZEDER and I have had about
the relative danger of adding this BUG() to this part of the code.

I think the best thing to do is just to merge it to "next".

Maybe I'm wrong about the BUG() not triggering in a way that someone
will spot if we expose this more widely.

But we've been having some version of that discussion for the past few
months without any any new specifics about *how* it might be raised,
just (I think it's fair to say) general paranoia that it might happen
somehow/somewhere.

Which is fair enough, but I'd think a good way forward with it would be
to give it wider exposure during this cycle. If it's triggered somehow
it's trivial to amend/revert the tip commit to remove the assertion.

If we merge it down I promise I'll (and try to remember to, putting it
in my calendar if so...) start some discussion before the next release
about whether we'll want to cut the release with that BUG(), which if we
don't trigger it by then we'll probably feel confident about keeping.

Does that sound like a good way forward? Otherwise the "one alternative
way forward[...]" mentioned in [1] is something we could do.

1. https://lore.kernel.org/git/211203.868rx2t0hv.gmgdl@evledraar.gmail.com/