[PATCH v3 5/8] doc: devicetree: Updates for devicetree-rebasing subtree

[Tom Rini]

On Thu, Jan 04, 2024 at 04:50:14PM +0100, Michal Simek wrote:
> 
> 
> On 1/4/24 16:30, Tom Rini wrote:
> > On Thu, Jan 04, 2024 at 03:45:10PM +0100, Michal Simek wrote:
> > > Hi Tom,
> > > 
> > > On 1/4/24 14:50, Tom Rini wrote:
> > > > On Thu, Jan 04, 2024 at 06:52:32PM +0530, Sumit Garg wrote:
> > > > > Hi Michal,
> > > > > 
> > > > > On Thu, 4 Jan 2024 at 15:39, Michal Simek <michal.simek at amd.com> wrote:
> > > > > > 
> > > > > > 
> > > > > > 
> > > > > > On 1/3/24 17:32, Tom Rini wrote:
> > > > > > > On Wed, Jan 03, 2024 at 09:19:40AM -0700, Rob Herring wrote:
> > > > > > > > On Thu, Dec 28, 2023 at 4:59 AM Sumit Garg <sumit.garg at linaro.org> wrote:
> > > > > > > [snip]
> > > > > > > > > +In order to maintain devicetree files sync, U-Boot maintains a Git subtree for
> > > > > > > > > +devicetee-rebasing repo as `devicetee-rebasing/` sub-directory. It is regularly
> > > > > > > > > +kept updated with every new kernel major release via subtree pull as follows::
> > > > > > > > 
> > > > > > > > I would suggest dropping "-rebasing" in the u-boot tree. (I wish we
> > > > > > > > had in the original repo). I don't think it's relevant.
> > > > > > > > 
> > > > > > > > We're not likely to regenerate the tree, but any clue what 'git
> > > > > > > > subtree pull' would do in this case? It could happen if we switched to
> > > > > > > > git-filter-repo.
> > > > > > > > 
> > > > > > > > > +
> > > > > > > > > +    git subtree pull --prefix devicetree-rebasing \
> > > > > > > > > +        git://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git \
> > > > > > > > > +        <release-tag> --squash
> > > > > > > > 
> > > > > > > > I'd put this in a script to run. Documentation tends to be not quite
> > > > > > > > correct. A script could also be smarter and figure out <release-tag>.
> > > > > > > 
> > > > > > > Since you had mentioned elsewhere moving the kernel scripts/dtc/ to
> > > > > > > something using subtree, I do hope to learn some lessons from what you
> > > > > > > do there. The first thing is that given the size/nature of the commits,
> > > > > > > I had figured I'd be the one doing this as a subtree pull+squash then
> > > > > > > push, rather than posting to the ML since it'll be huge and
> > > > > > > un-reviewable, but with a note sent off to the ML that people should be
> > > > > > > aware the next sync has been done and retest as needed. But Sumit, were
> > > > > > > you planning to do some of this instead?  The second thing is that if we
> > > > > > > move the subtree part in to dts/ instead (where we will still have the
> > > > > > > Makefile/Kconfig we have today and then our Makefiles within the
> > > > > > > directories, this might get more complex and so really require a script
> > > > > > > to keep it from getting error prone?
> > > > > > 
> > > > > > I played with this series and didn't really have time to dig into subtree
> > > > > > mechanics but one thing I see is that when squash and merge happens you can't do
> > > > > > simple rebase anymore which is time to time very useful.
> > > > > > I see some different rebase strategies and --rebase-merges option but if this is
> > > > > > adopted it should be properly described how to rebase u-boot tree which contains
> > > > > > some subtrees.
> > > > > 
> > > > > Did you observe any specific difference with respect to subtree merge
> > > > > commits (Tom will likely do those in the beginning of next release
> > > > > cycle) when compared with normal merge commits when Tom pulls in code
> > > > > from different custodians like one example below? The same rebasing
> > > > > rules should apply to both types of merge commits.
> > > > > 
> > > > > commit dffa6d0210f57793f1e4e1e209d91ca5642e4d05 (origin/next)
> > > > > Merge: 2b28c3b871c e266d273114
> > > > > Author: Tom Rini <trini at konsulko.com>
> > > > > Date:   Mon Jan 1 12:38:15 2024 -0500
> > > > > 
> > > > >       Merge tag 'dm-next-1124' of
> > > > > https://source.denx.de/u-boot/custodians/u-boot-dm into next
> > > > > 
> > > > >       support propagating supernode properties with bootph schema
> > > > >       align bloblist with v0.9 of Firmware Handoff spec
> > > > 
> > > > There's two cases. The first and uncommon case is that if you want to do
> > > > something like:
> > > > - Branch next
> > > > - Add this subtree, do a few subtree updates on it (like my experiment
> > > >     of starting with v6.1-dts and merging up to v6.6-dts)
> > > > - Wait a while, have changes happen in next
> > > > - Rebase on to current next
> > > > 
> > > > It gets painful and odd, but I don't know that this will ever really
> > > > happen.
> > > > 
> > > > The second, common case is:
> > > > - Assume "next" already has this subtree + some merge commits in it.
> > > > - Make a new local branch.
> > > > - Make change in your new local branch.
> > > > - This hypothetical "next" branch moves forward with changes
> > > > - Rebase your local branch on to this "next" branch
> > > > 
> > > > In this case things work as expected. Even if  "next" in that case gets
> > > > a new subtree merge that you need to rebase in.
> > > > 
> > > > A third case that I'm not sure if really will happen or is a case where
> > > > the custodian should rework their tree instead (to drop the subtree
> > > > merge) is:
> > > > - Assume "next" already has this subtree + some merge commits in it.
> > > > - Make a new local branch.
> > > > - Make a subtree merge+squash in your local branch.
> > > > - This hypothetical "next" branch moves forward with changes
> > > > - Rebase your local branch on to this "next" branch.
> > > > 
> > > > This will then make you try and resolve some conflicts from that subtree
> > > > merge.  But is this a valid use case? Yes, I can see wanting to get a
> > > > start on testing a dts merge before it happens, but is this a tree which
> > > > needs to be rebased, or just re-created as needed, if the parent branch
> > > > moves forward?
> > > > 
> > > > Can you explain a bit more your case Michal since it sounded to me like
> > > > you had a problem that happened rather than a worry about what might
> > > > happen?
> > > 
> > > I remember couple of bisects which I was doing and it works fine on the tree
> > > till the point you have 2 issues to deal with. It means pretty much you have
> > > config change/fix which has to be applied to see second issue.
> > > 
> > > The normal way what I have done was simply find major version apply that fix
> > > and then take the latest tree and rebase latest on the top of it. There were
> > > normally not so many conflicts to resolved (or easy to resolve/ignore). Then
> > > because the first fix is present all the time bisect is able to help me to
> > > find second issue.
> > > 
> > > If there is subtree update the whole rebase will horribly fail.
> > > 
> > > I see that Rob and you in DTC case just simply do c&p and create regular
> > > commit with all changes inside. From my perspective this is more
> > > straightforward to deal with which will handle all cases and it is also
> > > proven over that years.
> > 
> > Ah, bisecting.  Maybe the (annoying) answer is to turn that merge commit
> > in to a regular commit in the bisect branch?  This is a good point, and
> > I need to go and construct a bigger example tree and check around at
> > what happens.
> 
> Sure you can do whatever you like. But I expect that at the end if this
> started to be used you will have subtree for devicetree, dtc, lwip and maybe
> something else. It means you have to start to keep track where that subtree
> are and what version is used at what time.

Yes, lwip would be a subtree too, dtc is trickier but I see and agree
with your point.

> Because dt binding is extending often likely you will need to merge it every
> Linux release. It means you would have to do it multiple times depends on
> how many releases you are going over.

Yes, the plan is to re-sync every time our next opens with the kernel
release at the time (which maybe needs to be clearer in the docs).

> I know that Qemu is using gitmodules that's definitely another option but
> not sure if better or worse.

Yeah, submodules is harder and we're not going that path.

> I can also imagine that companies with SR IR certified firmware would keep
> downstream version where they want to just take code fixes, DT fixes and
> devicetree-rebasing tree to make sure that they are still aligned with DT
> schema. Can you simply just cherry pick that merged commit without any side
> effect?

Life downstream is always fun. Personally, I would re-run the "git
subtree" command rather than cherry-pick from U-Boot itself in that
case, but it should be no different than any other commit.

> 
> Back to my point. At the end what we need to is to document it properly how
> to deal with it.

So, here's the test I did. I went back to v2023.10, then added v6.1-dts
as a subtree.  I rebased v2024.01-rc1 on top of that, added a fake bad
commit, synced up subtrees to v6.6, and then rebased the tags along the
way to v2024.01-rc6.  I then went for a bisect back to the fake bad
commit.  Everything went fine, git didn't have troubles rebasing things
as it went.

But to re-iterate, yes, the policy around when dts and bindings will be
resynced needs to be clear and documented. Does that cover it?

-- 
Tom
-------------- next part --------------
A non-text attachment was scrubbed...
Name: signature.asc
Type: application/pgp-signature
Size: 659 bytes
Desc: not available
URL: <https://lists.denx.de/pipermail/u-boot/attachments/20240104/019b01a3/attachment.sig>