On Fri, Jan 17, 2025 at 09:40:18PM +0800, shejialuo wrote: > On Thu, Jan 16, 2025 at 02:57:25PM +0100, Patrick Steinhardt wrote: > > On Sun, Jan 05, 2025 at 09:49:09PM +0800, shejialuo wrote: > > > Although we use "parse_loose_ref_content" to check whether the object id > > > is correct, we never parse it into the "struct object" structure thus we > > > ignore checking whether there is a real object existing in the repo and > > > whether the object type is correct. > > > > > > Use "parse_object" to parse the oid for the regular ref content. If the > > > object does not exist, report the error to the user by reusing the fsck > > > message "BAD_REF_CONTENT". > > > > > > Then, we need to check the type of the object. Just like "git-fsck(1)", > > > we only report "not a commit" error when the ref is a branch. Last, > > > update the test to exercise the code. > > > > I wonder whether it wouldn't make more sense to put this into a generic > > part of `git refs verify`. This isn't a check for whether the format of > > the files backend is correct, but rather a check whether the refdb is > > sane. As such, it also applies do the reftable backend. > > > > So should we maybe extend `git refs verify` so that it also knows to > > perform generic checks that apply independent of the backend in use? > > > > I somehow understand your meaning here and I think what your meaning > here is that we could use internal ref method to parse the oid after we > check the format of the ref files. Thus, we could totally make these two > different kinds of checks separately. > > However, if we have already parsed the raw ref files, we could reuse the > parsed hex and then use "parse_object" to get the object id to check. > This is the main reason why I add this check now. > > And I agree with your thinking here. Actually, we may put this into > object check part. Because in "git-fsck(1)", we parse the refdb to know > whether an object is dangling or not. > > I will postpone these checks in the later patches. Really thanks here > for this wonderful suggestion. Yeah. I'm thinking ahead a bit in this context and want to avoid that we eventually have to reimplement the same set of checks for every single ref backend that we have. So separating the backend-generic bits from the non-generic ones is what I'm after. Patrick
