git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Patrick Steinhardt <ps@pks.im>
To: shejialuo <shejialuo@gmail.com>
Cc: git@vger.kernel.org, Karthik Nayak <karthik.188@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Michael Haggerty <mhagger@alum.mit.edu>
Subject: Re: [PATCH v6 4/9] packed-backend: check if header starts with "# pack-refs with: "
Date: Wed, 26 Feb 2025 09:08:32 +0100	[thread overview]
Message-ID: <Z77MAMqVGrT2jgcf@pks.im> (raw)
In-Reply-To: <Z73D5XLzzg-OPmdT@ArchLinux>

On Tue, Feb 25, 2025 at 09:21:41PM +0800, shejialuo wrote:
> We always write a space after "# pack-refs with:". However, when
> creating the packed-ref snapshot, we only check whether the header
> starts with "# pack-refs with:". However, we need to make sure that we
> would not break compatibility by tightening the rule. The following is
> how some third-party libraries handle the header of "packed-ref" file.
> 
> 1. libgit2 is fine and always writes the space. It also expects the
>    whitespace to exist.
> 2. JGit does not expect th header to have a trailing space, but expects
>    the "peeled" capability to have a leading space, which is mostly
>    equivalent because that capability is typically the first one we
>    write. It always writes the space.
> 3. gitoxide expects the space t exist and writes it.
> 4. go-git doesn't create the header by default.
> 
> So, we are safe to tighten the rule by checking whether the header
> starts with "# pack-refs with: ".

The commit message nicely describes why it's safe to do the change, but
it doesn't describe why it's something we _want_ to do.

Ideally, we'd be able to argue with a technical spec of the format, but
unless I'm mistaken such a document does not exist. The next-best thing
is to do what everyone can agree on, and that seems to be to both write
and expect a space after the colon. By not following consensus that
exists in other libraries we're being more loose.

So if we for example started to stop writing the space due to a bug,
we'd still continue to parse the header alright and thus not notice the
problem, but now we have broken other implementations. That may be a
good enough justification for the change itself.

Patrick

  reply	other threads:[~2025-02-26  8:08 UTC|newest]

Thread overview: 168+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-01-05 13:46 [PATCH 00/10] add more ref consistency checks shejialuo
2025-01-05 13:49 ` [PATCH 01/10] files-backend: add object check for regular ref shejialuo
2025-01-07 14:17   ` Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 13:40     ` shejialuo
2025-01-24  7:54       ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 02/10] builtin/refs.h: get worktrees without reading head info shejialuo
2025-01-07 14:57   ` Karthik Nayak
2025-01-07 16:34     ` shejialuo
2025-01-08  8:40       ` Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 03/10] packed-backend: check whether the "packed-refs" is regular shejialuo
2025-01-07 16:33   ` Karthik Nayak
2025-01-17 14:00     ` shejialuo
2025-01-17 22:01       ` Eric Sunshine
2025-01-18  3:05         ` shejialuo
2025-01-19  8:03         ` Karthik Nayak
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:49 ` [PATCH 04/10] packed-backend: add "packed-refs" header consistency check shejialuo
2025-01-08  0:54   ` shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:23     ` shejialuo
2025-01-24  7:51       ` Patrick Steinhardt
2025-02-17 13:16     ` shejialuo
2025-01-05 13:49 ` [PATCH 05/10] packed-backend: check whether the refname contains NULL binaries shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:33     ` shejialuo
2025-01-05 13:49 ` [PATCH 06/10] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-17 14:35     ` shejialuo
2025-01-05 13:50 ` [PATCH 07/10] packed-backend: create "fsck_packed_ref_entry" to store parsing info shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 08/10] packed-backend: add check for object consistency shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 09/10] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-01-16 13:57   ` Patrick Steinhardt
2025-01-05 13:50 ` [PATCH 10/10] builtin/fsck: add `git refs verify` child process shejialuo
2025-01-06 22:16   ` Junio C Hamano
2025-01-07 12:00     ` shejialuo
2025-01-07 15:52       ` Junio C Hamano
2025-01-30  4:04 ` [PATCH v2 0/8] add more ref consistency checks shejialuo
2025-01-30  4:06   ` [PATCH v2 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-01-30 17:53     ` Junio C Hamano
2025-01-30  4:07   ` [PATCH v2 2/8] builtin/refs: get worktrees without reading head info shejialuo
2025-01-30 18:04     ` Junio C Hamano
2025-01-31 13:29       ` shejialuo
2025-01-31 16:16         ` Junio C Hamano
2025-01-30  4:07   ` [PATCH v2 3/8] packed-backend: check whether the "packed-refs" is regular shejialuo
2025-01-30 18:23     ` Junio C Hamano
2025-01-31 13:54       ` shejialuo
2025-01-31 16:20         ` Junio C Hamano
2025-02-01  9:47           ` shejialuo
2025-02-03 20:15             ` Junio C Hamano
2025-02-04  3:58               ` shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-01-30  4:07   ` [PATCH v2 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-01-30 18:58     ` Junio C Hamano
2025-01-31 14:23       ` shejialuo
2025-01-30  4:07   ` [PATCH v2 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-05 10:09       ` shejialuo
2025-01-30  4:07   ` [PATCH v2 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-04  4:28       ` shejialuo
2025-01-30  4:08   ` [PATCH v2 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-01-30 19:02     ` Junio C Hamano
2025-01-31 14:35       ` shejialuo
2025-01-31 16:23         ` Junio C Hamano
2025-02-01  9:50           ` shejialuo
2025-02-03  8:40         ` Patrick Steinhardt
2025-02-03  8:40     ` Patrick Steinhardt
2025-01-30  4:08   ` [PATCH v2 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-01-30 19:03     ` Junio C Hamano
2025-01-31 14:37       ` shejialuo
2025-02-03  8:40     ` Patrick Steinhardt
2025-02-04  5:32       ` shejialuo
2025-02-06  5:56   ` [PATCH v3 0/8] add more ref consistency checks shejialuo
2025-02-06  5:58     ` [PATCH v3 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-06  5:58     ` [PATCH v3 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-06  5:58     ` [PATCH v3 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-06  5:59     ` [PATCH v3 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:12         ` shejialuo
2025-02-12 17:48         ` Junio C Hamano
2025-02-14  3:53           ` shejialuo
2025-02-06  5:59     ` [PATCH v3 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-06  5:59     ` [PATCH v3 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:18         ` shejialuo
2025-02-06  5:59     ` [PATCH v3 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:20         ` shejialuo
2025-02-12 10:42           ` Patrick Steinhardt
2025-02-12 10:56         ` shejialuo
2025-02-06  6:00     ` [PATCH v3 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-12  9:56       ` Patrick Steinhardt
2025-02-12 10:21         ` shejialuo
2025-02-14  4:50     ` [PATCH v4 0/8] add more ref consistency checks shejialuo
2025-02-14  4:51       ` [PATCH v4 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-14  4:52       ` [PATCH v4 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-14  9:19         ` Karthik Nayak
2025-02-14 12:20           ` shejialuo
2025-02-14  4:52       ` [PATCH v4 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-14  9:50         ` Karthik Nayak
2025-02-14 12:37           ` shejialuo
2025-02-14  4:52       ` [PATCH v4 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-14 10:30         ` Karthik Nayak
2025-02-14 12:43           ` shejialuo
2025-02-14 14:01         ` Junio C Hamano
2025-02-14  4:52       ` [PATCH v4 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-14  4:53       ` [PATCH v4 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-14  4:59       ` [PATCH v4 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-14  4:59       ` [PATCH v4 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-14  9:04       ` [PATCH v4 0/8] add more ref consistency checks Karthik Nayak
2025-02-14 12:16         ` shejialuo
2025-02-17 15:25       ` [PATCH v5 " shejialuo
2025-02-17 15:27         ` [PATCH v5 1/8] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-17 15:27         ` [PATCH v5 2/8] builtin/refs: get worktrees without reading head information shejialuo
2025-02-25  8:26           ` Patrick Steinhardt
2025-02-17 15:27         ` [PATCH v5 3/8] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-25  8:27           ` Patrick Steinhardt
2025-02-17 15:27         ` [PATCH v5 4/8] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-25  8:27           ` Patrick Steinhardt
2025-02-25 12:34             ` shejialuo
2025-02-17 15:27         ` [PATCH v5 5/8] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-17 15:28         ` [PATCH v5 6/8] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-17 15:28         ` [PATCH v5 7/8] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-17 15:28         ` [PATCH v5 8/8] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-25  8:27         ` [PATCH v5 0/8] add more ref consistency checks Patrick Steinhardt
2025-02-25 13:19         ` [PATCH v6 0/9] " shejialuo
2025-02-25 13:21           ` [PATCH v6 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-25 13:21           ` [PATCH v6 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-25 13:21           ` [PATCH v6 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-25 17:44             ` Junio C Hamano
2025-02-26 12:05               ` shejialuo
2025-02-25 13:21           ` [PATCH v6 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-26  8:08             ` Patrick Steinhardt [this message]
2025-02-26 12:28               ` shejialuo
2025-02-25 13:21           ` [PATCH v6 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-25 13:21           ` [PATCH v6 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-25 13:22           ` [PATCH v6 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-25 13:22           ` [PATCH v6 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-25 13:22           ` [PATCH v6 9/9] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-26 13:48           ` [PATCH v7 0/9] add more ref consistency checks shejialuo
2025-02-26 13:49             ` [PATCH v7 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-26 13:49             ` [PATCH v7 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-26 13:49             ` [PATCH v7 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-26 18:36               ` Junio C Hamano
2025-02-27  0:57                 ` shejialuo
2025-02-27 14:10                   ` Patrick Steinhardt
2025-02-27 16:57                   ` Junio C Hamano
2025-02-28  5:02                     ` shejialuo
2025-02-26 13:50             ` [PATCH v7 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-26 13:50             ` [PATCH v7 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-26 13:50             ` [PATCH v7 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-26 13:50             ` [PATCH v7 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-26 13:50             ` [PATCH v7 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-26 13:51             ` [PATCH v7 9/9] builtin/fsck: add `git refs verify` child process shejialuo
2025-02-27 16:03             ` [PATCH v8 0/9] add more ref consistency checks shejialuo
2025-02-27 16:05               ` [PATCH v8 1/9] t0602: use subshell to ensure working directory unchanged shejialuo
2025-02-27 16:06               ` [PATCH v8 2/9] builtin/refs: get worktrees without reading head information shejialuo
2025-02-27 16:06               ` [PATCH v8 3/9] packed-backend: check whether the "packed-refs" is regular file shejialuo
2025-02-27 16:06               ` [PATCH v8 4/9] packed-backend: check if header starts with "# pack-refs with: " shejialuo
2025-02-27 16:06               ` [PATCH v8 5/9] packed-backend: add "packed-refs" header consistency check shejialuo
2025-02-27 16:07               ` [PATCH v8 6/9] packed-backend: check whether the refname contains NUL characters shejialuo
2025-02-27 16:07               ` [PATCH v8 7/9] packed-backend: add "packed-refs" entry consistency check shejialuo
2025-02-27 16:07               ` [PATCH v8 8/9] packed-backend: check whether the "packed-refs" is sorted shejialuo
2025-02-27 16:07               ` [PATCH v8 9/9] builtin/fsck: add `git refs verify` child process shejialuo

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=Z77MAMqVGrT2jgcf@pks.im \
    --to=ps@pks.im \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=karthik.188@gmail.com \
    --cc=mhagger@alum.mit.edu \
    --cc=shejialuo@gmail.com \
    /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 NNTP newsgroup(s).