Re: [PATCH v2 13/20] merge-ort: step 1 of tree writing -- record basenames, modes, and oids

[Elijah Newren <newren@gmail.com>]

On Wed, Nov 11, 2020 at 12:01 PM Jonathan Tan <jonathantanmy@google.com> wrote:
>
> > +struct directory_versions {
> > +     struct string_list versions;
>
> Maybe comment that this is an unordered list of basenames to <whatever
> the type of ci->merged.result is>.

There actually is an order, and it's important.  It's reverse
lexicographic order of full pathnames (the ordering comes from the
fact that process_entries() iterates paths in that order).  The
reasons for that ordering are (1) all the basenames within a directory
are adjacent so that I can write out a tree for a directory as soon as
it is done, and (2) paths within a directory are listed before the
directory itself so that I get the necessary info for subtrees before
trying to write out their parent trees.

It's not until later patches that I take advantage of this ordering
(and when I do I have a very long commit message to describe it all),
but I can add a comment that this is a list of basenames to
merge_info.

>
> > @@ -442,6 +464,7 @@ static void process_entries(struct merge_options *opt,
> >       struct strmap_entry *e;
> >       struct string_list plist = STRING_LIST_INIT_NODUP;
> >       struct string_list_item *entry;
> > +     struct directory_versions dir_metadata;
> >
> >       if (strmap_empty(&opt->priv->paths)) {
> >               oidcpy(result_oid, opt->repo->hash_algo->empty_tree);
> > @@ -458,6 +481,9 @@ static void process_entries(struct merge_options *opt,
> >       plist.cmp = string_list_df_name_compare;
> >       string_list_sort(&plist);
> >
> > +     /* other setup */
> > +     string_list_init(&dir_metadata.versions, 0);
> > +
>
> Might be clearer to just initialize dir_metadata as {
> STRING_LIST_INIT_NODUP }.

It'll eventually grow to { STRING_LIST_INIT_NODUP,
STRING_LIST_INIT_NODUP, NULL, 0 }, which is a tad long, but if the
initializer is clearer I'm happy to switch over to it.

> The rest makes sense.