From: Phillip Wood <phillip.wood123@gmail.com>
To: Toon Claes <toon@to1.studio>
Cc: Toon Claes <toon@iotcl.com>,
git@vger.kernel.org, Junio C Hamano <gitster@pobox.com>,
Taylor Blau <me@ttaylorr.com>
Subject: Re: [PATCH 1/1] cat-file: quote-format name in error when using -z
Date: Tue, 20 Dec 2022 10:18:51 +0000 [thread overview]
Message-ID: <aab41ec5-7854-b62c-f11c-ba850cbac95b@dunelm.org.uk> (raw)
In-Reply-To: <87a63i7h8h.fsf@to1.studio>
Hi Toon
On 20/12/2022 05:31, Toon Claes wrote:
>
> 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.
>> 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
next prev parent reply other threads:[~2022-12-20 10:18 UTC|newest]
Thread overview: 32+ messages / expand[flat|nested] mbox.gz Atom feed top
2022-12-09 15:00 [PATCH 0/1] cat-file: quote-format name in error when using -z Toon Claes
2022-12-09 15:00 ` [PATCH 1/1] " Toon Claes
2022-12-09 19:33 ` Phillip Wood
2022-12-09 23:58 ` Junio C Hamano
2022-12-11 16:30 ` Phillip Wood
2022-12-12 0:11 ` Junio C Hamano
2022-12-12 11:34 ` Toon Claes
2022-12-12 22:09 ` Junio C Hamano
2022-12-13 15:06 ` Phillip Wood
2022-12-14 8:29 ` Junio C Hamano
2022-12-20 5:31 ` Toon Claes
2022-12-20 10:18 ` Phillip Wood [this message]
2022-12-21 12:42 ` Toon Claes
2023-01-05 6:24 ` [PATCH v2 0/1] " Toon Claes
2023-01-05 6:24 ` [PATCH v2 1/1] " Toon Claes
2023-01-16 19:07 ` [PATCH v3 0/1] " Toon Claes
2023-01-16 19:07 ` [PATCH v3 1/1] " Toon Claes
2023-01-17 15:24 ` Phillip Wood
2023-03-03 19:17 ` [PATCH v4 0/2] " Toon Claes
2023-03-03 19:17 ` [PATCH v4 1/2] cat-file: extract printing batch error message into function Toon Claes
2023-03-03 20:26 ` Junio C Hamano
2023-03-03 23:14 ` Junio C Hamano
2023-05-10 19:01 ` [PATCH v5 0/1] cat-file: quote-format name in error when using -z Toon Claes
2023-05-10 19:01 ` [PATCH v5 1/1] " Toon Claes
2023-05-10 20:13 ` Junio C Hamano
2023-05-12 8:54 ` Toon Claes
2023-05-12 16:57 ` Junio C Hamano
2023-05-15 8:47 ` Phillip Wood
2023-05-15 17:20 ` Junio C Hamano
2023-06-02 13:29 ` Phillip Wood
2023-03-03 19:17 ` [PATCH v4 2/2] " Toon Claes
2023-03-03 20:14 ` [PATCH v4 0/2] " Junio C Hamano
Reply instructions:
You may reply publicly to this message via plain-text email
using any one of the following methods:
* Save the following mbox file, import it into your mail client,
and reply-to-all from there: mbox
Avoid top-posting and favor interleaved quoting:
https://en.wikipedia.org/wiki/Posting_style#Interleaved_style
* Reply using the --to, --cc, and --in-reply-to
switches of git-send-email(1):
git send-email \
--in-reply-to=aab41ec5-7854-b62c-f11c-ba850cbac95b@dunelm.org.uk \
--to=phillip.wood123@gmail.com \
--cc=git@vger.kernel.org \
--cc=gitster@pobox.com \
--cc=me@ttaylorr.com \
--cc=phillip.wood@dunelm.org.uk \
--cc=toon@iotcl.com \
--cc=toon@to1.studio \
/path/to/YOUR_REPLY
https://kernel.org/pub/software/scm/git/docs/git-send-email.html
* If your mail client supports setting the In-Reply-To header
via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line
before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).