Re: [PATCH 1/3] test-tool: add pack-deltas helper

From: Junio C Hamano <gitster@pobox.com>
Date: Apr 28, 2025, 16:37 UTC
Message-ID: <xmqqmsc0dyyf.fsf@gitster.g>
In-Reply-To: <275808ae-7126-4a24-b5f3-283ea8023f5f@gmail.com>

Derrick Stolee <stolee@gmail.com> writes:

Show 8 quoted lines

>> Yeah, I think we clearly showed our "it's just test helper, whose
>> callers are supposed to know what they are doing" attitude, but with
>> proper helpers, it is not too much additional effort to do the right
>> thing.
>
> But with this philosophy in mind I can change the CLI to be of the form
> "--num-objects <n>" to use the parse-options feature. This should make
> things more extensible in the future.

True. If we are aiming to deliver this to end-user's hands in some future, I agree that we want to make it extensible, make it dtrt without being told, and make it harder to give wrong input. If we are going in that direction [*], I suspect this should not be a separate and independent input---rather, shouldn't the tool already _know_ what objects it placed in the resulting output stream, and should be able to _count_ that number by itself? One thing it lets us do to have this as a separate number is to create an invalid pack stream where the header gives a wrong number, and as a test tool, that may trump the convenience of not having to give the number explicitly.

Another thing we may want to add to the tool is to give it a mode that either (1) refuses to place the same object in a single pack stream more than once, or (2) warn when it happens. The latter would be useful to create an invalid pack stream for testing.

Thanks.

[Footnote]

* ... which I would welcome as the project manager, even though
  personally I find that 'it is just test helper' attitude
  attractive.  Not everybody in this project is experienced enough
  to understand the 'it is just test helper, let's not spend too
  much effort on it' attitude and elements in the code there should
  not be blindly copied and pasted to production part of the system.