Hi Junio & Peff, On Thu, 14 Mar 2019, Junio C Hamano wrote:
> "Johannes Schindelin via GitGitGadget" <[email protected]> > writes: > > > @@ -442,6 +442,18 @@ static enum get_oid_result get_short_oid(const char > > *name, int len, > > find_short_packed_object(&ds); > > status = finish_object_disambiguation(&ds, oid); > > > > + /* > > + * If we didn't find it, do the usual reprepare() slow-path, > > + * since the object may have recently been added to the repository > > + * or migrated from loose to packed. > > + */ > > + if (status == MISSING_OBJECT) { > > + reprepare_packed_git(the_repository); > > + find_short_object_filename(&ds); > > + find_short_packed_object(&ds); > > + status = finish_object_disambiguation(&ds, oid); > > + } > > + > > This looks obviously correct, but two things that made me wonder > briefly were: > > 1. is reprepare_packed_git() a bit too heavy-weight, if the only > thing we are addressing is the loose-object cache going stale? > > 2. is there a way to cleanly avoid the three-line duplicate? > > My tentative answers are (1) even if it is, but get_short_oid() is > already heavy-weight enough; it won't be worth restructuring the > code to make it possible to clear only the loose-object cache, and > (2) a loop that runs twice when the first result is MISSING_OBJECT > and otherwise leaves after once would need an extra variable, its > iniialization, check and increment, which is more than what we might > save with such a restructuring, so it won't be worth pursuing. > > But others may have better ideas, as always ;-) Peff tried with a function, but I think that this would actually be a really appropriate occasion for a well-placed `goto`: -- snip -- diff --git a/sha1-name.c b/sha1-name.c index 6dda2c16df10..36a66026964a 100644 --- a/sha1-name.c +++ b/sha1-name.c @@ -415,7 +415,7 @@ static enum get_oid_result get_short_oid(const char *name, int len, struct object_id *oid, unsigned flags) { - int status; + int status, attempts = 0; struct disambiguate_state ds; int quietly = !!(flags & GET_OID_QUIETLY); @@ -438,10 +438,21 @@ static enum get_oid_result get_short_oid(const char *name, int len, else ds.fn = default_disambiguate_hint; +try_again: find_short_object_filename(&ds); find_short_packed_object(&ds); status = finish_object_disambiguation(&ds, oid); + /* + * If we did not find it, do the usual reprepare() slow-path, since the + * object may have recently been added to the repository or migrated + * from loose to packed. + */ + if (status == MISSING_OBJECT && !attempts++) { + reprepare_packed_git(the_repository); + goto try_again; + } + if (!quietly && (status == SHORT_NAME_AMBIGUOUS)) { struct oid_array collect = OID_ARRAY_INIT; -- snap -- Granted, it's 11 lines inserted and one changed as opposed to 12 lines inserted, but it does make the code DRYer (and therefore slightly safer to modify in the future). I pushed this to the GitGitGadget PR. Thanks, Dscho
