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

[Michal Simek]

On 1/4/24 19:03, Tom Rini wrote:
> 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?

I am fine with it when we have documentation around.

Thanks,
Michal