Re: vendor/illumos merges

On Tue, May 18, 2021 at 7:49 PM Mark Johnston <markj@freebsd.org> wrote:

> On Sun, Apr 25, 2021 at 04:02:22PM +0200, Ulrich Spörlein wrote:
> > On Sat, 2021-04-24 at 11:08:58 -0400, Mark Johnston wrote:
> > >On Sat, Apr 24, 2021 at 12:44:40PM +0200, Ulrich Spörlein wrote:
> > >> On Fri, 2021-04-23 at 17:26:33 -0400, Mark Johnston wrote:
> > >> >Hi,
> > >> >
> > >> >Now that FreeBSD uses OpenZFS as the upstream for ZFS,
> vendor/illumos is
> > >> >mostly unused.  However, we still use illumos as an upstream for CTF
> > >> >tools and DTrace, though there haven't been any imports in a while.
> > >> >
> > >> >illumos has put a lot of work into their CTF toolchain, and I'd like
> to
> > >> >import that.  There are a couple of snags that I'd appreciate some
> > >> >guidance on.
> > >> >
> > >> >First, I believe I should delete now-unused ZFS code from the vendor
> > >> >branch and merge the result to main.  I did this locally and got an
> > >> >empty merge, which is what I'd expect.  Is there any problem with
> this?
> > >>
> > >> Why would you record this empty merge? If you clean up vendor/foo,
> just
> > >> do that but don't merge a no-op back into main (nothing changed, after
> > >> all).
> > >
> > >Ok, I guess there is no reason to merge that change separately.  It
> > >will end up being merged with subsequent imports though.
> > >
> > >> >Second, with Subversion we had both vendor/illumos and
> > >> >vendor-sys/illumos, and now we just have the former, seemingly with
> > >> >sys/* bits imported from vendor-sys.  Some of the upstream commits
> touch
> > >> >both userspace and kernel bits, but the merge targets for these in
> > >> >FreeBSD are different: cddl/contrib/opensolaris vs.
> > >> >sys/cddl/contrib/opensolaris.  How should I merge into main in this
> > >> >case?  I don't really see any options other than to split each
> offending
> > >> >upstream commit into two parts, one for userspace and one for the
> > >> >kernel, and merge them separately.
> > >> >
> > >> >If it helps to look at the branch where I staged the upstream
> commits,
> > >> >I've pushed it to vendor/illumos2 in
> https://github.com/markjdb/freebsd
> > >> >.
> > >>
> > >> Can you clarify why the merging of the two might be an issue? Note
> that
> > >> unlike subversion, in git there's no "merge a certain subtree"
> handling,
> > >> all that is recorded is a tree of some form and then a set of parents
> or
> > >> ancestor commits. (git is a content tracker, not really a VCS :)
> > >>
> > >> I was under the impression that userland and kernel imports/merges
> need
> > >> to happen at the same time anyway, so I assume you would import all
> the
> > >> bits under vendor/foo in 1 commit and then merge them in 1 commit into
> > >> main. Is that not how it goes?
> > >
> > >How can I do that with git subtree merge?  Suppose an illumos commit
> > >modifies cmd/dtrace/foo.c (userspace) and uts/common/dtrace/foo.c
> > >(kernel).  That maps to cddl/contrib/opensolaris/cmd/dtrace/foo.c and
> > >sys/cddl/contrib/opensolaris/uts/common/dtrace/foo.c in FreeBSD,
> > >respectively.  So to do a subtree merge, I need to use distinct prefixes
> > >depending on whether I'm importing userspace or kernel changes.  When
> > >they are mixed together, it's not clear to me how I can merge at all.
> > >
> > >I see that for OpenZFS we keep all code, including userspace code, under
> > >sys/contrib/openzfs, so it doesn't have this problem.
> >
> > I don't think you want a subtree merge, especially as things are
> > scattered all over the place. Also note that none of this subtree magic
> > is in any way recorded in the git data, all it does is help you with the
> > 3-way merges (or whatever).
> >
> > So I would do:
> > - import whatever you need into contrib/foo, commit normally.
> > - munge /usr/src to have every kernel and userland stuff (not sure what
> > other merge tools exist, just make sure to copy over file deletions as
> > well :). You could rsync --del two times with the right source/dest
> > pairs, or export a diff/patch from step 1 and apply it under the right
> > prefixes. test, test, test.
> > - write out this tree to git using: git write-tree
> > - then commit this using: git commit-tree -m "my message" -p HEAD -p
> > origin/vendor/illumos <tree hash from previous command>
> > - bump main to point to that hash using git update-ref
> > - git log --graph and inspect the hell out of this
> > - git push, then curse that we disallow merge commits and you need to
> > `git pull --rebase` to advance to the latest published head and that
> > might mess up your merge commit pretty bad :(
> >
> >
> > Maybe 2x git subtree merge + then rewriting and squashing them into 1
> > would work. But I fear it will record 3 parents, not 2 parents.
> >
> > Whatever you do, maybe please push to your private Github clone or our
> > dev repo first and tell us where to look, so we can inspect whether it
> > looks ok.
>
> I followed your suggestions and have what looks like a clean result.
> Basically I did two subtree merges and committed the result.
>
> I pushed it here:
> https://github.com/markjdb/freebsd/tree/ff/illumos-merge
> The merge is the second-last commit on that branch.
>
> Rebasing the merge is painful, partly because there are some old ZFS
> commits in the illumos vendor branch which git tries to merge.  Rename
> detection leads to some really weird conflicts as well.  I'm not sure
> how to handle this: there's a lot of room to make mistakes when
> rebasing, so I would want to be careful and do extra testing before
> pushing the final merge, but during that window it's likely that I will
> end up having to rebase again.
>
> I should perhaps remove all ZFS bits from the vendor branch, and merge
> it into main first before importing other stuff.
>

I see only 1 merge commit in there, is that the expected outcome?

The output of
git show -p --first-parent 23a19903267ee799cbddc35eb3e6f978ac1b4f38
looks mostly sane though, does it look like the changes that you'd like to
bring into main?

Warner, could you please also have a look?

Cheers
Uli