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

From: Junio C Hamano <[email-masked]>
Date: Mon, 28 Apr 2025 09:37:28 -0700

Derrick Stolee <[email-masked]> writes:

>> 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.