On Fri, Jul 12, 2019 at 02:02:52PM -0700, Junio C Hamano wrote: > * md/list-objects-filter-combo (2019-06-28) 10 commits > - list-objects-filter-options: make parser void > - list-objects-filter-options: clean up use of ALLOC_GROW > - list-objects-filter-options: allow mult. --filter > - strbuf: give URL-encoding API a char predicate fn > - list-objects-filter-options: make filter_spec a string_list > - list-objects-filter-options: move error check up > - list-objects-filter: implement composite filters > - list-objects-filter-options: always supply *errbuf > - list-objects-filter: put omits set in filter struct > - list-objects-filter: encapsulate filter components > > The list-objects-filter API (used to create a sparse/lazy clone) > learned to take a combined filter specification. > > There is a bit of interaction with cc/multi-promisor topic, whose > conflict resolution I have no confidence in X-<. Extra sets of > eyes are appreciated. > Sorry for the delay. I was on vacation and then catching up for a week after I got back. I uploaded a merged commit here: https://github.com/matvore/git/tree/filts And the merged file itself (only this one had conflicts) is here: https://github.com/matvore/git/blob/filts/list-objects-filter.c I'll comment on the conflicts: > diff --cc list-objects-filter-options.c > index ba1425cb4a,28c571f922..0000000000 > --- a/list-objects-filter-options.c > +++ b/list-objects-filter-options.c > @@@ -1,25 -1,19 +1,30 @@@ > #include "cache.h" > #include "commit.h" > #include "config.h" > #include "revision.h" > #include "argv-array.h" > #include "list-objects.h" > #include "list-objects-filter.h" > #include "list-objects-filter-options.h" > ++<<<<<<< md/list-objects-filter-combo > +#include "trace.h" > +#include "url.h" > + > +static int parse_combine_filter( > + struct list_objects_filter_options *filter_options, > + const char *arg, > + struct strbuf *errbuf); > ++||||||| merged common ancestors > ++======= > + #include "promisor-remote.h" > ++>>>>>>> cc/multi-promisor > This portion is trivial to merge - just move the "promisor-remote.h" inclusion to the correct spot. > /* > * Parse value of the argument to the "filter" keyword. > * On the command line this looks like: > * --filter=<arg> > * and in the pack protocol as: > * "filter" SP <arg> > * > * The filter keyword will be used by many commands. > * See Documentation/rev-list-options.txt for allowed values for <arg>. > @@@ -29,22 -23,33 +34,49 @@@ > * expand_list_objects_filter_spec() first). We also "intern" the arg for the > * convenience of the current command. > */ > static int gently_parse_list_objects_filter( > struct list_objects_filter_options *filter_options, > const char *arg, > struct strbuf *errbuf) > { > const char *v0; > > ++<<<<<<< md/list-objects-filter-combo > + if (filter_options->choice) > + BUG("filter_options already populated"); > ++||||||| merged common ancestors > ++ if (filter_options->choice) { > ++ if (errbuf) { > ++ strbuf_addstr( > ++ errbuf, > ++ _("multiple filter-specs cannot be combined")); > ++ } > ++ return 1; > ++ } > ++ > ++ filter_options->filter_spec = strdup(arg); > ++======= > + if (!arg) > + return 0; > + > + if (filter_options->choice) { > + if (errbuf) { > + strbuf_addstr( > + errbuf, > + _("multiple filter-specs cannot be combined")); > + } > + return 1; > + } > + > + filter_options->filter_spec = strdup(arg); > ++>>>>>>> cc/multi-promisor The cc/multi-promisor branch allowed gently_parse_list_objects_filter to accept a NULL filter spec, in which case it does nothing. So the merged branch keeps this behavior. md/list-objects-filter-combo changed the contract of this function such that an attempt to combine filter specs will terminate with BUG rather than return an error. All the callers already check filter_options.choice, so this is still fine (it particular, I double-checked partial_clone_get_default_filter_spec and its call site at builtin/fetch.c:1524) > /* > * Record the initial filter-spec in the config as > * the default for subsequent fetches from this remote. > */ > ++<<<<<<< md/list-objects-filter-combo > + core_partial_clone_filter_default = > + xstrdup(expand_list_objects_filter_spec(filter_options)); > + git_config_set("core.partialclonefilter", > + core_partial_clone_filter_default); > ++||||||| merged common ancestors > ++ core_partial_clone_filter_default = > ++ xstrdup(filter_options->filter_spec); > ++ git_config_set("core.partialclonefilter", > ++ core_partial_clone_filter_default); > ++======= > + filter_name = xstrfmt("remote.%s.partialclonefilter", remote); > + git_config_set(filter_name, filter_options->filter_spec); > + free(filter_name); > + > + /* Make sure the config info are reset */ > + promisor_remote_reinit(); > ++>>>>>>> cc/multi-promisor > } > > void partial_clone_get_default_filter_spec( md/list-objects-filter-combo used the expand_list_objects_filter_spec function to expand the filter spec string rather than get it directly. So the merged result simply applies that alteration to cc/multi-promisor. I checked whether callers to this function (partial_clone_register) would ever give a null filter_options (or a non-null with a NULL filter_spec) and both calls are guarded by "if (filter_options.choice)" so filter_options.filter_spec should also be set. > - struct list_objects_filter_options *filter_options) > + struct list_objects_filter_options *filter_options, > + const char *remote) > { > ++<<<<<<< md/list-objects-filter-combo > + struct strbuf errbuf = STRBUF_INIT; > + > ++||||||| merged common ancestors > ++======= > + struct promisor_remote *promisor = promisor_remote_find(remote); > + > ++>>>>>>> cc/multi-promisor This part of the conflict is trivial to merge - just use both the variable declarations. /* * Parse default value, but silently ignore it if it is invalid. */ > ++<<<<<<< md/list-objects-filter-combo > + if (!core_partial_clone_filter_default) > + return; > + > + string_list_append(&filter_options->filter_spec, > + core_partial_clone_filter_default); > + gently_parse_list_objects_filter(filter_options, > + core_partial_clone_filter_default, > + &errbuf); > + strbuf_release(&errbuf); > ++||||||| merged common ancestors > ++ if (!core_partial_clone_filter_default) > ++ return; > ++ gently_parse_list_objects_filter(filter_options, > ++ core_partial_clone_filter_default, > ++ NULL); > ++======= > + if (promisor) > + gently_parse_list_objects_filter(filter_options, > + promisor->partial_clone_filter, > + NULL); > ++>>>>>>> cc/multi-promisor } This is a confusing conflict because cc/multi-promisor (understandably) turned the if-guard inside-out, removing "return", since it's only guarding against one line anyway. When I merged it, I swtiched it back (if (!promisor) return) since creating and freeing the strbuf requires multiple lines. I would be fine with rebasing my or Christian's change on top of the other, or doing it as a merge.
