Re: [PATCH 1/1] cat-file: quote-format name in error when using -z

From: Phillip Wood <phillip.wood123@gmail.com>
Date: Dec 20, 2022, 10:18 UTC
Message-ID: <aab41ec5-7854-b62c-f11c-ba850cbac95b@dunelm.org.uk>
In-Reply-To: <87a63i7h8h.fsf@to1.studio>

Hi Toon

On 20/12/2022 05:31, Toon Claes wrote:

Show 12 quoted lines

> 
> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
>> Thanks for working on this. I'd previously suggested NUL terminating the output
>> of "git cat-file -z" to avoid this problem [1]
>>
>> [1]
>> https://lore.kernel.org/git/66b71194-ad0e-18d0-e43b-71e5c47ba111@gmail.com/
> 
> What happened to this proposal? I don't see any replies to that. That's
> a bit sad, because it would have been nice to have it this behavior from
> the start.

Yes it would have been nice, unfortunately it feel through the cracks due to a combination of unfortunate timing and lack of time. I think the patch was probably in next by the time I realized the problem and wrote that email. Taylor was on holiday at the time and then I went away around the time he got back. I know it was on his todo list but I think he was busy catching up from being away getting ready for git merge. I had other things I was working on and unfortunately didn't get round to following it up.

Show 18 quoted lines

>> but quoting the object name is a better solution.
> 
> I would not say it's a better solution, but it's a less invasive
> solution that /minimizes/ breaking changes. Ideally I'd like to have NUL
> terminated output for "git cat-file -z". In a success situation I
> assume this would return:
> 
>      <oid> SP <type> SP <size> NUL
>      <contents> NUL
> 
> In a failure situation something like:
> 
>      <object> SP missing NUL
> 
> So when you pass -z you can keep reading until the first NUL and then
> you'll know if you should read for contents as well.
> 
> Would you consider change behavior to this now?

Hmm I'm not sure (and luckily it isn't up to me to take the final decision!). I really don't know how many people are using "-z" I suspect it is not many and so the fallout wouldn't be too bad but we'd still be inconveniencing some users. I theory we could so "sorry we made a mistake when implementing this feature and have decided to change it" but we have a solution in your patch which hopefully does not break any users (I say hopefully because think if one is careful and keep track of which responses you've read it is possible to detect missing objects now even if their name contains a new line but it will be messy and probably slightly inefficient but anyone doing that will notice the change in output).

Overall I'd say it is tempting but risky and inconvenient to any existing users of "-z" (assuming someone else is actually using it). Quoting the object name is definitively the safer option at this point.

Best Wishes

Phillip