Subject: Re: What's cooking in git.git (Feb 2025, #09; Fri, 28)

From: shejialuo <[email-masked]>
Date: Tue, 4 Mar 2025 20:25:44 +0800

On Tue, Mar 04, 2025 at 07:27:37AM +0100, Patrick Steinhardt wrote:
> On Mon, Mar 03, 2025 at 09:03:53AM -0800, Junio C Hamano wrote:
> > shejialuo <[email-masked]> writes:
> > 
> > > On Fri, Feb 28, 2025 at 04:45:31PM -0800, Junio C Hamano wrote:
> > >
> > >> * sj/ref-consistency-checks-more (2025-02-27) 9 commits
> > >>  - builtin/fsck: add `git refs verify` child process
> > >>  - packed-backend: check whether the "packed-refs" is sorted
> > >>  - packed-backend: add "packed-refs" entry consistency check
> > >>  - packed-backend: check whether the refname contains NUL characters
> > >>  - packed-backend: add "packed-refs" header consistency check
> > >>  - packed-backend: check if header starts with "# pack-refs with: "
> > >>  - packed-backend: check whether the "packed-refs" is regular file
> > >>  - builtin/refs: get worktrees without reading head information
> > >>  - t0602: use subshell to ensure working directory unchanged
> > >> 
> > >>  "git fsck" becomes more careful when checking the refs.
> > >> 
> > >>  Comments?
> > >>  source: <Z8CMx7O19PMs9sVY@ArchLinux>
> > >
> > > I think I have addressed the comments from you, Patrick and Karthik.
> > > Could we make the patch into "next"?
> > 
> > Mine was merely a small kibitzing on the logic flow structure, and I
> > didn't really looked at the larger picture beyond that part of the
> > code I looked at.  Let's hear from Patrick and Karthik (cc'ed) if
> > they find the result of the updates satisfactory.
> 
> Yes, I'm happy with the current state of this patch series. I'm a tiny
> bit worried about the new call to `git refs verify` in git-fsck(1) being
> added this late into the release cycle as we're now exercising a bunch
> of new code with only a few weeks of testing. My basic assumption is
> that mostly noone uses `git refs verify` explicitly right now, so all of
> the code we have introduced there over the last couple of releases did
> not yet receive much testing at all.
> 

Yes, I also think that there are few people who know the `git refs
verify` and execute this command. That's the reason why I want to
integrate `git refs verify` in git-fsck(1) thus we could get feedback
and improve the code.

However, as you have said, we are also taking the risk.

> So while I think that executing the command in git-fsck(1) is a good
> thing overall, I would feel a bit more comfortable if that last commit
> of the series landed in the next release cycle. But maybe I'm just being
> overly cautious?
> 

Yes, by dropping the last commit, the risk would be reduced. Actually, I
don't mind which way we choose. But I somehow think that we should
execute the command in git-fsck(1), we need to get the feedback from the
users. From my point, I want to know how do the users react to the new
aded checks. Because we have tightened more rules, some may be good and
some may not be reasonable. And we could improve this in the next
release.

However, as I have said, both way works fine for me. So, I am open.

Thanks,
Jialuo

> Patrick