On Mon, Oct 25, 2021 at 02:38:04AM -0700, Junio C Hamano wrote: > SZEDER Gábor <szeder.dev@xxxxxxxxx> writes: > > > I still very much dislike the idea of a BUG() in the progress code > > that can trigger outside of the test suite, because the progress line > > is only a UI gimmick and not a crucial part of any Git operation, and > > even though a progress line might be buggy, the underlying Git > > operation is not affected by it and would still finish successfully, > > as was the case with the dozen of so progress line bugs in the past. > > I too recall that we have fixed numerous bugs in the past year in > the area, but weren't they kind of obvious ones _once_ they are > pointed out at you (e.g. progress never reaching to 100%)? Yet the > developers have failed to catch them because their eyes would coast > over without paying attention to them, exactly because the progress > bar is merely a UI gimmick. > > I haven't formed a firm opinion on this yet, but I think the idea > behind these BUG() is to help such problems be caught while they are > still in the lab. You may not notice when your live progress bar > behaved a bit funny, but if you hit a BUG(), that would be squarely > in your face and you cannot ignore it. The "outside of the test suite" part is important. Running the test suite with a GIT_TEST_CHECK_PROGRESS knob is sufficient to catch these issues as my inital two patch series have shown at the very beginning of this thread before Ævar hijacked them. Interested Git developers can even enable it in their .profile if they wish; I did so with GIT_TEST_SPLIT_INDEX for a while to expose some of my patches to more real-world use. However, I think it's conceptually a bad idea to abort with a BUG() an otherwise successful Git operation by default, when that bug happens to be in such an ancillary component as the progress display, and find the justifications given in the commit message unconvincing.
