"Derrick Stolee via GitGitGadget" <gitgitgadget@xxxxxxxxx> writes: > Update in v2 > ============ > > * Instead of creating a new public reprepare_alt_odb() method, inline > its implementation inside reprepare_packed_git(). [snip] > diff --git a/packfile.c b/packfile.c > index 79e21ab18e7..06419c8e8ca 100644 > --- a/packfile.c > +++ b/packfile.c > @@ -1008,6 +1008,16 @@ void reprepare_packed_git(struct repository *r) > struct object_directory *odb; > > obj_read_lock(); > + > + /* > + * Reprepare alt odbs, in case the alternates file was modified > + * during the course of this process. This only _adds_ odbs to > + * the linked list, so existing odbs will continue to exist for > + * the lifetime of the process. > + */ > + r->objects->loaded_alternates = 0; > + prepare_alt_odb(r); I understand that the change to inline what was originally reprepare_alt_odb() was in response to a reviewer comment, but I would prefer the original version, since this assumes that prepare_alt_odb() only adds to what's there instead of first clearing the odb linked list and odb_by_path hashmap. But I guess clearing a linked list and hashmap can be a bit cumbersome in C, so maybe it would be reasonable to assume that this behavior would not change. In any case, maybe a comment should be added to prepare_alt_odb() saying that if an update of the alternates is needed, one can do so by clearing loaded_alternates and re- invoking prepare_alt_odb() (at least so that a developer changing prepare_alt_odb() can see the comment and understand what behaviors this function needs to preserve). Everything else (the new functionality, the commit message etc.) looks good, of course.
