Re: [PATCH v4 3/7] scalar: validate the optional enlistment argument
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 24, 2022, 19:29 UTC
- Message-ID
- <xmqqsfoyhgqe.fsf@gitster.g>
- In-Reply-To
- <220524.86h75ex01s.gmgdl@evledraar.gmail.com>
Ævar Arnfjörð Bjarmason <avarab@gmail.com> writes:
Show 16 quoted lines
> Both are built by our main build process, and thus should have > corresponding adjustments in our main test code, just as is already the > case for both "git" and "test-tool". > > But even if that wasn't the case I'd still be of the view that we should > add "scalar" to that list. > > It's just a matter of potential time sinks in the future. If we > introduce a hidden segfault in the scalar code and don't notice for some > time because we're using that test pattern that's going to suck, and > likely to waste a lot of time. We might even ship a broken command to > users. > > Whereas having "scalar" on that list is going to be a relatively easy > matter of grepping and doing some boilerplate changes if and when we > ever "git rm" it entirely, or "promote it" from contrib or whatever.
In addition, it already is an actual time sink that causes us send a lot more bytes back and forth than the number of bytes necessary to send a reroll that adds one liner to the same step.
> I also think that just getting rid of that whitelist entirely is an > acceptable solution. Perhaps it's just being overzealous in forbidding > everything except "git", we should still not use it for the likes of > "grep", but we could just leave that to the documentation.
It indeed is tempting entry into a slippery slope, and I'd see it as a change bigger than we could comfortably make as a "while at it" change.
We can stop arguing and instead send in a reroll that squashes in something like this, which shouldn't be controversial, I would say.
t/test-lib-functions.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-)
diff --git i/t/test-lib-functions.sh w/t/test-lib-functions.sh index 93c03380d4..8899eaabed 100644 --- i/t/test-lib-functions.sh +++ w/t/test-lib-functions.sh @@ -1106,7 +1106,7 @@ test_must_fail_acceptable () { fi case "$1" in - git|__git*|test-tool|test_terminal) + git|__git*|scalar|test-tool|test_terminal) return 0 ;; *)