On Sat, Dec 5, 2020 at 8:10 AM Christian Couder <christian.couder@xxxxxxxxx> wrote: > > 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/ Good catch, thanks. > > 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". Right, will do.
