Re: [PATCH v2 15/20] merge-ort: step 3 of tree writing -- handling subdirectories as we go

From: Jonathan Tan <jonathantanmy@google.com>
To: newren@gmail.com
Cc: jonathantanmy@google.com, git@vger.kernel.org
Subject: Re: [PATCH v2 15/20] merge-ort: step 3 of tree writing -- handling subdirectories as we go
Date: Mon, 30 Nov 2020 10:41:47 -0800	[thread overview]
Message-ID: <20201130184147.3086836-1-jonathantanmy@google.com> (raw)
In-Reply-To: <CABPp-BEfqrT7B=Mu=LWpi3GYkP0SRLFFjPVkca8krairry3iew@mail.gmail.com>

> On Tue, Nov 24, 2020 at 6:07 PM Jonathan Tan <jonathantanmy@google.com> wrote:
> >
> > > For now, I'll keep the code as-is, but add more comments to both the
> > > data structure and the code.  If I've missed something about how I
> > > could make use of your BEGIN_TREE idea, let me know and I'll look at
> > > it again.
> >
> > In collect_merge_info_callback(), you call setup_path_info() to add to
> > opt->priv->paths, then call traverse_trees() (which recursively calls
> > collect_merge_info_callback()). I was thinking that in addition to doing
> > that, you could call setup_path_info() a second time, but teach it to
> > add a synthetic path (maybe have a special bit in struct conflict_info
> > or something like that) that indicates "this is the end of the tree".
> > Subsequent code can notice that bit and not do the normal processing,
> > but instead do end-of-tree processing.
> 
> So, I realized that I already had end-of-tree markers -- the
> directories themselves.  But due to some other weirdness in how I had
> built up the processing, the existence of those markers was both
> obscured, and deliberately side-stepped.  So, I did a little
> restructuring so we can use these as actual end-of-tree markers more
> directly.

Ah sorry...what I meant was to have both begin-of-tree and end-of-tree
elements in the path list, so one of them is real and the other
synthetic. Right now you have an end-of-tree real path in the list of
paths, yes.

> > Having said that, maybe it will turn out that your additional comments
> > in v3 will be clearer, and we wouldn't need the synthetic entry.
> 
> Hopefully it's clearer now, but the entries aren't synthetic.  My big
> opt->priv->paths strmap with all full relative paths contained all
> files _and_ directories already, and now I just use the directory
> markers more directly.  Hopefully the extra comments help too.

OK - I see that you have a new version [1] and hopefully I'll be able to
take a look soon.

[1] https://lore.kernel.org/git/pull.923.git.git.1606635803.gitgitgadget@gmail.com/