Date: Wed, 19 May 2021 21:04:57 +0200 From: =?UTF-8?Q?Ulrich_Sp=C3=B6rlein?= <uqs@freebsd.org> To: Mark Johnston <markj@freebsd.org> Cc: freebsd-git@freebsd.org Subject: Re: vendor/illumos merges Message-ID: <CAJ9axoTDeK37%2B8Jjjih%2BVHqq4k%2B%2BKqC7cqw5sR=EyBKDUrzyPQ@mail.gmail.com> In-Reply-To: <YKP9hxYDz2D3/dZX@nuc> References: <YIM7iaptOgsWyxse@nuc> <YIP2mE%2B0lKB8pLTK@acme.spoerlein.net> <YIQ0ilbqOM4/cTE4@nuc> <YIV2bqPnzX0faSMo@acme.spoerlein.net> <YKP9hxYDz2D3/dZX@nuc>
index | next in thread | previous in thread | raw e-mail
[-- Attachment #1 --] 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 [-- Attachment #2 --] <div dir="ltr"><div dir="ltr"><br></div><br><div class="gmail_quote"><div dir="ltr" class="gmail_attr">On Tue, May 18, 2021 at 7:49 PM Mark Johnston <<a href="mailto:markj@freebsd.org">markj@freebsd.org</a>> wrote:<br></div><blockquote class="gmail_quote" style="margin:0px 0px 0px 0.8ex;border-left:1px solid rgb(204,204,204);padding-left:1ex">On Sun, Apr 25, 2021 at 04:02:22PM +0200, Ulrich Spörlein wrote:<br> > On Sat, 2021-04-24 at 11:08:58 -0400, Mark Johnston wrote:<br> > >On Sat, Apr 24, 2021 at 12:44:40PM +0200, Ulrich Spörlein wrote:<br> > >> On Fri, 2021-04-23 at 17:26:33 -0400, Mark Johnston wrote:<br> > >> >Hi,<br> > >> ><br> > >> >Now that FreeBSD uses OpenZFS as the upstream for ZFS, vendor/illumos is<br> > >> >mostly unused. However, we still use illumos as an upstream for CTF<br> > >> >tools and DTrace, though there haven't been any imports in a while.<br> > >> ><br> > >> >illumos has put a lot of work into their CTF toolchain, and I'd like to<br> > >> >import that. There are a couple of snags that I'd appreciate some<br> > >> >guidance on.<br> > >> ><br> > >> >First, I believe I should delete now-unused ZFS code from the vendor<br> > >> >branch and merge the result to main. I did this locally and got an<br> > >> >empty merge, which is what I'd expect. Is there any problem with this?<br> > >><br> > >> Why would you record this empty merge? If you clean up vendor/foo, just<br> > >> do that but don't merge a no-op back into main (nothing changed, after<br> > >> all).<br> > ><br> > >Ok, I guess there is no reason to merge that change separately. It<br> > >will end up being merged with subsequent imports though.<br> > ><br> > >> >Second, with Subversion we had both vendor/illumos and<br> > >> >vendor-sys/illumos, and now we just have the former, seemingly with<br> > >> >sys/* bits imported from vendor-sys. Some of the upstream commits touch<br> > >> >both userspace and kernel bits, but the merge targets for these in<br> > >> >FreeBSD are different: cddl/contrib/opensolaris vs.<br> > >> >sys/cddl/contrib/opensolaris. How should I merge into main in this<br> > >> >case? I don't really see any options other than to split each offending<br> > >> >upstream commit into two parts, one for userspace and one for the<br> > >> >kernel, and merge them separately.<br> > >> ><br> > >> >If it helps to look at the branch where I staged the upstream commits,<br> > >> >I've pushed it to vendor/illumos2 in <a href="https://github.com/markjdb/freebsd" rel="noreferrer" target="_blank">https://github.com/markjdb/freebsd</a><br>; > >> >.<br> > >><br> > >> Can you clarify why the merging of the two might be an issue? Note that<br> > >> unlike subversion, in git there's no "merge a certain subtree" handling,<br> > >> all that is recorded is a tree of some form and then a set of parents or<br> > >> ancestor commits. (git is a content tracker, not really a VCS :)<br> > >><br> > >> I was under the impression that userland and kernel imports/merges need<br> > >> to happen at the same time anyway, so I assume you would import all the<br> > >> bits under vendor/foo in 1 commit and then merge them in 1 commit into<br> > >> main. Is that not how it goes?<br> > ><br> > >How can I do that with git subtree merge? Suppose an illumos commit<br> > >modifies cmd/dtrace/foo.c (userspace) and uts/common/dtrace/foo.c<br> > >(kernel). That maps to cddl/contrib/opensolaris/cmd/dtrace/foo.c and<br> > >sys/cddl/contrib/opensolaris/uts/common/dtrace/foo.c in FreeBSD,<br> > >respectively. So to do a subtree merge, I need to use distinct prefixes<br> > >depending on whether I'm importing userspace or kernel changes. When<br> > >they are mixed together, it's not clear to me how I can merge at all.<br> > ><br> > >I see that for OpenZFS we keep all code, including userspace code, under<br> > >sys/contrib/openzfs, so it doesn't have this problem.<br> > <br> > I don't think you want a subtree merge, especially as things are <br> > scattered all over the place. Also note that none of this subtree magic <br> > is in any way recorded in the git data, all it does is help you with the <br> > 3-way merges (or whatever).<br> > <br> > So I would do:<br> > - import whatever you need into contrib/foo, commit normally.<br> > - munge /usr/src to have every kernel and userland stuff (not sure what <br> > other merge tools exist, just make sure to copy over file deletions as <br> > well :). You could rsync --del two times with the right source/dest <br> > pairs, or export a diff/patch from step 1 and apply it under the right <br> > prefixes. test, test, test.<br> > - write out this tree to git using: git write-tree<br> > - then commit this using: git commit-tree -m "my message" -p HEAD -p <br> > origin/vendor/illumos <tree hash from previous command><br> > - bump main to point to that hash using git update-ref<br> > - git log --graph and inspect the hell out of this<br> > - git push, then curse that we disallow merge commits and you need to <br> > `git pull --rebase` to advance to the latest published head and that <br> > might mess up your merge commit pretty bad :(<br> > <br> > <br> > Maybe 2x git subtree merge + then rewriting and squashing them into 1 <br> > would work. But I fear it will record 3 parents, not 2 parents.<br> > <br> > Whatever you do, maybe please push to your private Github clone or our <br> > dev repo first and tell us where to look, so we can inspect whether it <br> > looks ok.<br> <br> I followed your suggestions and have what looks like a clean result.<br> Basically I did two subtree merges and committed the result.<br> <br> I pushed it here:<br> <a href="https://github.com/markjdb/freebsd/tree/ff/illumos-merge" rel="noreferrer" target="_blank">https://github.com/markjdb/freebsd/tree/ff/illumos-merge</a><br>; The merge is the second-last commit on that branch.<br> <br> Rebasing the merge is painful, partly because there are some old ZFS<br> commits in the illumos vendor branch which git tries to merge. Rename<br> detection leads to some really weird conflicts as well. I'm not sure<br> how to handle this: there's a lot of room to make mistakes when<br> rebasing, so I would want to be careful and do extra testing before<br> pushing the final merge, but during that window it's likely that I will<br> end up having to rebase again.<br> <br> I should perhaps remove all ZFS bits from the vendor branch, and merge<br> it into main first before importing other stuff.<br></blockquote><div><br></div><div>I see only 1 merge commit in there, is that the expected outcome?</div><div><br></div><div>The output of</div><div>git show -p --first-parent 23a19903267ee799cbddc35eb3e6f978ac1b4f38<br></div><div>looks mostly sane though, does it look like the changes that you'd like to bring into main?</div><div><br></div><div>Warner, could you please also have a look?</div><div><br></div><div>Cheers</div><div>Uli</div></div></div>help
Want to link to this message? Use this URL: <https://mail-archive.FreeBSD.org/cgi/mid.cgi?CAJ9axoTDeK37%2B8Jjjih%2BVHqq4k%2B%2BKqC7cqw5sR=EyBKDUrzyPQ>