On Thu, Dec 07, 2023 at 02:34:51AM -0500, Jeff King wrote:
> On Thu, Dec 07, 2023 at 08:01:41AM +0100, Patrick Steinhardt wrote:
>
> > > So forgetting at all about how we structure the code, it seems to me
> > > that the problem is not new code, but all of the existing code which
> > > looks for access("refs", X_OK).
> >
> > True. The question is of course how much value there is in an old tool
> > to be able to discover a new repository that it wouldn't be able to read
> > in the first place due to it not understanding the reference format. So
> > I'd very much like to see that eventually, we're able to get rid of
> > "legacy" cruft that doesn't serve any purpose anymore.
>
> Right, nobody is going to be mad about not being able to use the
> repository with old code. My concern is that by skipping it in the
> discovery phase, though, the user may see unexpected behavior (like
> continuing and finding some _other_ repo). I admit it's a pretty narrow
> case, though.
Agreed, that's also an angle I brought up in a separate thread [1]. The
second benefit is that the user would get a proper error message stating
that the "extensions.refFormat" is not understood compared to Git just
skipping over the repository completely.
> > The question is whether we can do a better job of this going forward so
> > that at least we don't have to pose the same question in the future.
> > Right now, we'll face the same problem whenever any part of the current
> > on-disk repository data structures changes.
> >
> > I wonder whether it would make sense to introduce something like a
> > filesystem-level hint, e.g. in the form of a new ".is-git-repository"
> > file. If Git discovers that file then it assumes the directory to be a
> > Git repository -- and everything else is set up by parsing the config
> > and thus the repository's configured format.
>
> I kind of think the presence of a well-formed HEAD is a good indicator
> that it is a Git directory. IOW, I don't have any real problem with
> simply loosening is_git_directory() to remove the "refs/" check
> entirely. But again, that can only help us going forward; older versions
> will still get confused if we truly ditch "refs/" for other formats.
>
> Of course some ref formats may want to avoid the "HEAD" file itself, so
> your .is-git-repository would be cleaner. I'm just not sure if it's
> worth the headache to try to switch things now.
I think that both "HEAD" and "refs/" are in the same spirit and consider
both to be legacy cruft that ideally wouldn't exist with the reftable
backend. I think dropping just one of these requirements ("refs/") is
the least favorable option though:
- We'd still have unneeded files that only exist to aid old clients.
- At the same time, the old clients wouldn't be able to detect the
repository anymore and need an update. So we could just as well drop
both files and would have the same outcome.
- This is not a long-term solution in case anything else in the
on-disk format will ever change.
Whether it's worth getting rid of them now... probably not, at least not
for the time being. But if we want to address this issue I'd rather want
to aim for a proper solution that also works for future changes.
Patrick
[1]: <ZXFy0_T1AZLh058g@tanuki>
Attachment:
signature.asc
Description: PGP signature
