Re: [PATCH v2 0/4] add ref content check for files backend
- From
Jeff King <peff@peff.net>
- Date
- Aug 29, 2024, 19:48 UTC
- Message-ID
- <20240829194855.GE423429@coredump.intra.peff.net>
- In-Reply-To
- <ZtAcowXWinP2Iguj@tanuki>
On Thu, Aug 29, 2024 at 09:00:58AM +0200, Patrick Steinhardt wrote:
Show 25 quoted lines
> > diff --git c/Documentation/BreakingChanges.txt w/Documentation/BreakingChanges.txt > > index 0532bfcf7f..2a85740f3c 100644 > > --- c/Documentation/BreakingChanges.txt > > +++ w/Documentation/BreakingChanges.txt > > @@ -115,6 +115,12 @@ info/grafts as outdated, 2014-03-05) and will be removed. > > + > > Cf. <20140304174806.GA11561@sigill.intra.peff.net>. > > > > +* Support for core.prefersymlinkrefs will be dropped. Support for > > + existing repositories that use symbolic links to represent a > > + symbolic ref may or may not be dropped. > > ++ > > +Cf. <20240829040215.GA4054823@coredump.intra.peff.net> > > + > > == Superseded features that will not be deprecated > > Yes, I'm very much in favor of that. As Peff said, I don't see a single > reason why it would make sense to use symlinks nowadays. We have also > supported the "new" syntax for ages now, and I'd be surprised if there > were repos out there using it on purpose. > > We should probably do the above together with a new check that starts to > warn about symbolic links in "refs/" such that users become aware of > this deprecation. We'd have to grow the infrastructure to also scan root > refs though, which to the best of my knowledge we don't currently scan.
I think the first step of the proposal (and what I had written in the patches that I linked) was just that we would stop _writing_ symlinks. And there we'd only need to warn people who have that config option set.
Whether to drop the reading side is less clear to me. I think in the long run it is good as a cleanup (and one less source of weird behavior that malicious local repos can trigger). But that decision can be made separately. I think it would be OK to just issue a deprecation warning whenever we actually follow a symlink (because I think we do so manually, since we need to know the target name as part of the resolution process).
-Peff