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:

quoted

quoted

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.

quoted

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>