On Wed, Nov 4, 2020 at 9:33 PM Matheus Tavares <matheus.bernardino@xxxxxx> wrote: > > From: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx> > > Separate the attribute gathering from the actual conversion by adding > _ca() variants of the conversion functions. These variants receive a > precomputed 'struct conv_attrs', not relying, thus, on a index state. s/a/an/ > They will be used in a future patch adding parallel checkout support, > for two reasons: > > - We will already load the conversion attributes in checkout_entry(), > before conversion, to decide whether a path is eligible for parallel > checkout. Therefore, it would be wasteful to load them again later, > for the actual conversion. > > - The parallel workers will be responsible for reading, converting and > writing blobs to the working tree. They won't have access to the main > process' index state, so they cannot load the attributes. Instead, > they will receive the preloaded ones and call the _ca() variant of > the conversion functions. Furthermore, the attributes machinery is > optimized to handle paths in sequential order, so it's better to leave > it for the main process, anyway. Well explained. > Signed-off-by: Jeff Hostetler <jeffhost@xxxxxxxxxxxxx> > [matheus.bernardino: squash, remove one function definition and reword] <rant++>I'd rather have "Reworked-by: Matheus Tavares <matheus.bernardino@xxxxxx>" or "Improved-by: Matheus Tavares <matheus.bernardino@xxxxxx>" than lines such as the above one.</rant++> > Signed-off-by: Matheus Tavares <matheus.bernardino@xxxxxx> > @@ -1447,7 +1447,7 @@ void convert_to_git_filter_fd(const struct index_state *istate, > ident_to_git(dst->buf, dst->len, dst, ca.ident); > } > > -static int convert_to_working_tree_internal(const struct index_state *istate, > +static int convert_to_working_tree_internal(const struct conv_attrs *ca, It is still internal, but for consistency it might be better to add "_ca" to the name of this function too, as we now pass it a "ca" instead of an "istate".