On Wed, Sep 13, 2017 at 10:03:58AM +0200, Michael Haggerty wrote:
> On 09/12/2017 07:31 PM, Jonathan Nieder wrote:
> > From: Stefan Beller <sbeller@xxxxxxxxxx>
> >
> > Pass DO_FOR_EACH_INCLUDE_BROKEN when iterating over replacement refs
> > so that the iteration does not require opening the named objects from
> > the object store. This avoids a dependency cycle between object access
> > and replace ref iteration.
> >
> > Moreover the ref subsystem has not been migrated yet to access the
> > object store via passed in repository objects. As a result, without
> > this patch, iterating over replace refs in a repository other than
> > the_repository it produces errors:
> >
> > error: refs/replace/3afabef75c627b894cccc3bcae86837abc7c32fe does not point to a valid object!
> >
> > Noticed while adapting the object store (and in particular its
> > evaluation of replace refs) to handle arbitrary repositories.
>
> Have you checked that consumers of this API can handle broken
> references? Aside from missing values, broken references can have
> illegal names (though hopefully not unsafe in the sense of causing
> filesystem traversal outside of `$GITDIR`).
It looks like there are only two callers. One complains about
names that aren't 40-hex. The other ("git replace -l") parses the 40-hex
in "long" mode, but will print anything in short mode (not that printing
is very dangerous).
I do have to wonder if for_each_replace_ref() should just do the 40-hex
syntactic check itself (and issue a warning for other refs). It seems
like that would be the point of calling for_each_replace_ref() instead
of just for_each_ref_in("refs/replace") yourself, and both of the
callers would benefit.
-Peff
