[PATCH v5 06/11] dts: Add script to uprev dts/upstream subtree

[Sumit Garg]

Hi Marek,

On Tue, 6 Feb 2024 at 05:51, Marek Vasut <marex at denx.de> wrote:
>
> On 2/2/24 14:05, Sumit Garg wrote:
> > dts/update-dts-subtree.sh is just a wrapper around git subtree commands.
> > Usage from the top level U-Boot source tree, run:
> >
> > $ ./dts/update-dts-subtree.sh pull <release-tag>
> > $ ./dts/update-dts-subtree.sh pick <commit-id>
> >
> > Signed-off-by: Sumit Garg <sumit.garg at linaro.org>
>
> How are the fixes which land in linux-stable handled now ?

Firstly, all the fixes land in the Linux mainline tree, then at every
rc* release those would be mirrored into devicetree-rebasing repo. And
then if there is a critical fix to address DT ABI breakage for U-Boot
(for at least a single board) then they just need to notify us with
the fix commit ID to be picked up. Or if people are willing to use the
dts/update-dts-subtree.sh script themselves then we are happy to
accept patches too.

BTW, we will also work with Linux DT maintainers to improve DT ABI
maintenance towards U-Boot.

>
> Do I have to duplicate the work which is already being done by the
> linux-stable maintainers ?

No, we don't have to backport all the fixes as they will make their
way automatically via the next subtree pull. As above we are only
concerned about fixes which are required to maintain DT ABI towards
U-Boot.

>
> > ---
> >
> > Changes in v5:
> > - Added support to cherry-pick fixes in subtree update script. Also, used
> >    https:// instead of git://.
> >
> > Changes in v4:
> > - New patch to add script dts/update-dts-subtree.sh as per Rob's comments.
> >
> >   dts/update-dts-subtree.sh | 45 +++++++++++++++++++++++++++++++++++++++
> >   1 file changed, 45 insertions(+)
> >   create mode 100755 dts/update-dts-subtree.sh
> >
> > diff --git a/dts/update-dts-subtree.sh b/dts/update-dts-subtree.sh
> > new file mode 100755
> > index 000000000000..b781bf710025
> > --- /dev/null
> > +++ b/dts/update-dts-subtree.sh
> > @@ -0,0 +1,45 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright 2024 Linaro Ltd.
> > +#
> > +# Usage: from the top level U-Boot source tree, run:
> > +# $ ./dts/update-dts-subtree.sh pull <release-tag>
> > +# $ ./dts/update-dts-subtree.sh pick <commit-id>
> > +#
> > +# The script will pull changes from devicetree-rebasing repo into U-Boot
> > +# as a subtree located as <U-Boot>/dts/upstream sub-directory. It will
> > +# automatically create a squash/merge commit listing the commits imported.
> > +
> > +set -e
> > +
> > +merge_commit_msg=$(cat << EOF
> > +Subtree merge tag '$2' of devicetree-rebasing repo [1] into dts/upstream
> > +
> > +[1] https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git/
> > +EOF
> > +)
> > +
> > +remote_add() {
> > +    if ! git remote | grep -w devicetree-rebasing
>
> I think you are looking for git-remote get-url here.

Ack.

>
> > +    then
> > +        git remote add devicetree-rebasing \
> > +            https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
>
> Rather than reconfigure user git remotes like this without user
> knowledge, either ask them, or print a warning and let the user
> configure their git remotes knowingly as they need to. Include the
> command in the warning to make it easier on the user.

Fair, let me add a message specifying the command used to add a git
remote automatically for a user.

>
> > +    fi
> > +    git fetch devicetree-rebasing master
> > +}
> > +
> > +if [ ! -z $1 ] && [ $1 = "pull" ]
> > +then
> > +    remote_add
>
> remote_add_and_fetch , since this does more then remote_add .

Ack.

>
> > +    git subtree pull --prefix dts/upstream devicetree-rebasing \
> > +        $2 --squash -m "${merge_commit_msg}"
> > +elif [ ! -z $1 ] && [ $1 = "pick" ]
>
> The non-zero test is not useful, just add missing quotes around "$1" and
> compare:
>
> elif [ "$1" = "pick" ]

Looks better.

>
> > +then
> > +    remote_add
> > +    git cherry-pick -x --strategy=subtree -Xsubtree=dts/upstream/ $2
> > +else
> > +    echo "usage: $0 param1 param2"
>
> The param names could really use improvement, instead of 'param1' that
> could be some '<op>' and 'param2' could be some '<url>' or something.

Sure, how about the following?

'param1' -> '<op>'
'param2' -> '<ref>'

>
> > +    echo "  param1    pull or pick"
> > +    echo "  param2    release tag [pull] or commit id [pick]"
> > +fi
>
> Please run shellcheck on this .

Sure.

-Sumit