Re: [PATCH 0/3] More add_submodule_odb() cleanup in merge code

> On Wed, Sep 8, 2021 at 11:18 AM Jonathan Tan <jonathantanmy@google.com> wrote:
> >
> > (CC-ing Elijah in case he has insight into the merge-ort part.)
> 
> All the submodule merging related functions were lifted from
> merge-recursive.c and minimally modified to fit the new structure.
> The only substantive change I made was to fix the merge result for
> virtual merge bases, but that's like one or two lines of code.  In
> particular, everything relating to submodule objects was totally
> untouched...and I think that's reflected in the fact that your PATCH 2
> basically is the same patch twice, once for merge-recursive and once
> for merge-ort.
> 
> I read over PATCH 2 and I didn't find anything that looked
> problematic, but I'm not up-to-speed on the add_submodule_odb and repo
> handling bits of the codebase so I'm not sure I would catch anything.
> But I am encouraged by the fact that it looks like you did the same
> stuff to merge-recursive and merge-ort; I'd be worried you missed
> something if that weren't the case.

> As a sidenote, though...
> 
> This does remind me that I noticed that the following functions from
> object-store.h do not take an explicit repository:
> 
> write_object_file()
> hash_object_file()
> hash_object_file_literally()
> force_object_loose()
> 
> I have a patch sitting around somewhere (possibly only still
> referenced in my 'temp' branch) to make repo_*() variants of the above
> functions, and turn the above into simple wrappers of the repo_*()
> variants which just pass the_repository (much like someone else did
> with read_object_file() and repo_read_object_file()).  It also updates
> merge-ort to use the new repo_*() functions.  However, I ended up
> excluding it from my merge-ort submissions since it wasn't necessary.
> Would this be of interest in your submodule work, though?  I guess
> it'd only matter if we started doing real merges of submodules as part
> of a parent repo merge.  (As opposed to the fast-forward-only merges
> that merge-recursive/merge-ort do right now for submodules.)