On Wed, 24 Jul 2024 at 19:40, Raymond Mao <raymond....@linaro.org> wrote:
>
> Hi Sumit,
>
> On Wed, 24 Jul 2024 at 03:08, Sumit Garg <sumit.g...@linaro.org> wrote:
>>
>> Hi Raymond,
>>
>> Thanks for doing this.
>>
>> On Tue, 23 Jul 2024 at 01:01, Raymond Mao <raymond....@linaro.org> wrote:
>> >
>> > Recently we are introducing multiple git subtree projects and
>> > it is the right time to have a universal script to update
>> > various subtrees and replace the dts/update-dts-subtree.sh.
>> >
>> > update-subtree.sh is a wrapper of git subtree commands.
>> >
>> > Usage: From U-Boot top directory,
>> > run
>> > $ ./tools/update-subtree.sh pull <subtree-name> <release-tag>
>> > for pulling a tag from the upstream.
>> > Or run
>> > $ ./tools/update-subtree.sh pick <subtree-name> <commit-id>
>> > for cherry-pick a commit from the upstream.
>> >
>> > Currently <subtree-name> supports dts, mbedtls and lwip.
>> >
>> > Signed-off-by: Raymond Mao <raymond....@linaro.org>
>> > ---
>> > Changes in v2
>> > - Refactored the script.
>> > - Update the control doc.
>> >
>> > doc/develop/devicetree/control.rst | 8 +--
>> > dts/update-dts-subtree.sh | 48 -----------------
>> > tools/update-subtree.sh | 86 ++++++++++++++++++++++++++++++
>> > 3 files changed, 90 insertions(+), 52 deletions(-)
>> > delete mode 100755 dts/update-dts-subtree.sh
>> > create mode 100755 tools/update-subtree.sh
>> >
>> > diff --git a/doc/develop/devicetree/control.rst
>> > b/doc/develop/devicetree/control.rst
>> > index ca4fb0b5b10..211f7e4909c 100644
>> > --- a/doc/develop/devicetree/control.rst
>> > +++ b/doc/develop/devicetree/control.rst
>> > @@ -96,12 +96,12 @@ sync the `dts/upstream/` subtree from the
>> > devicetree-rebasing repo whenever
>> > the next branch opens (refer: :doc:`../release_cycle`) with the latest
>> > mainline
>> > Linux kernel release. To sync the `dts/upstream/` subtree, run::
>> >
>> > - ./dts/update-dts-subtree.sh pull <devicetree-rebasing-release-tag>
>> > + ./tools/update-subtree.sh pull dts <devicetree-rebasing-release-tag>
>> >
>> > If required it is also possible to cherry-pick fixes from the
>> > devicetree-rebasing repository prior to next sync, usage::
>> >
>> > - ./dts/update-dts-subtree.sh pick <devicetree-rebasing-commit-id>
>> > + ./tools/update-subtree.sh pick dts <devicetree-rebasing-commit-id>
>> >
>> >
>> > Configuration
>> > @@ -116,8 +116,8 @@ However, if `dts/upstream/` hasn't yet received
>> > devicetree source file for your
>> > newly added board support then one option is that you can add the
>> > corresponding
>> > devicetree source file as `arch/<arch>/dts/<name>.dts`. To select that
>> > add `#
>> > CONFIG_OF_UPSTREAM is not set` and set `DEFAULT_DEVICE_TREE=<name>` when
>> > -prompted by Kconfig. Another option is that you can use use the "pick"
>> > option of
>> > -`dts/update-dts-subtree.sh` mentioned above to bring in the commits that
>> > you
>> > +prompted by Kconfig. Another option is that you can use the "pick" option
>> > of
>> > +`tools/update-subtree.sh` mentioned above to bring in the commits that you
>> > need.
>> >
>> > This should include your CPU or SoC's devicetree file. On top of that any
>> > U-Boot
>> > diff --git a/dts/update-dts-subtree.sh b/dts/update-dts-subtree.sh
>> > deleted file mode 100755
>> > index a57b78a41d3..00000000000
>> > --- a/dts/update-dts-subtree.sh
>> > +++ /dev/null
>> > @@ -1,48 +0,0 @@
>> > -#!/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_and_fetch() {
>> > - if ! git remote get-url devicetree-rebasing 2>/dev/null
>> > - then
>> > - echo "Warning: Script automatically adds new git remote via:"
>> > - echo " git remote add devicetree-rebasing \\"
>> > - echo "
>> > https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git";
>> > - git remote add devicetree-rebasing \
>> > -
>> > https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
>> > - fi
>> > - git fetch devicetree-rebasing master
>> > -}
>> > -
>> > -if [ "$1" = "pull" ]
>> > -then
>> > - remote_add_and_fetch
>> > - git subtree pull --prefix dts/upstream devicetree-rebasing \
>> > - "$2" --squash -m "${merge_commit_msg}"
>> > -elif [ "$1" = "pick" ]
>> > -then
>> > - remote_add_and_fetch
>> > - git cherry-pick -x --strategy=subtree -Xsubtree=dts/upstream/ "$2"
>> > -else
>> > - echo "usage: $0 <op> <ref>"
>> > - echo " <op> pull or pick"
>> > - echo " <ref> release tag [pull] or commit id [pick]"
>> > -fi
>> > diff --git a/tools/update-subtree.sh b/tools/update-subtree.sh
>> > new file mode 100755
>> > index 00000000000..5946aa1a9e2
>> > --- /dev/null
>> > +++ b/tools/update-subtree.sh
>> > @@ -0,0 +1,86 @@
>> > +#!/bin/sh
>> > +# SPDX-License-Identifier: GPL-2.0+
>> > +#
>> > +# Copyright (c) 2024 Linaro Limited
>> > +# Author: Raymond Mao <raymond....@linaro.org>
Something I forgot earlier, if you are going to add explicit
authorship then you should retain the prior author too from where this
script is derived.
>> > +#
>> > +# Usage: from the top level U-Boot source tree, run:
>> > +# $ ./tools/update-subtree.sh pull <subtree-name> <release-tag>
>> > +# Or:
>> > +# $ ./tools/update-subtree.sh pick <subtree-name> <commit-id>
>> > +#
>> > +# The script will pull changes from subtree repo into U-Boot.
>> > +# It will automatically create a squash/merge commit listing the commits
>> > +# imported.
>> > +
>> > +set -e
>> > +
>> > +print_usage() {
>> > + echo "usage: $0 <op> <subtree-name> <ref>"
>> > + echo " <op> pull or pick"
>> > + echo " <subtree-name> mbedtls or dts or lwip"
>> > + echo " <ref> release tag [pull] or commit id [pick]"
>> > +}
>> > +
>> > +if [ $# -ne 3 ]; then
>> > + print_usage
>> > + exit 1
>> > +fi
>> > +
>> > +op=$1
>> > +subtree_name=$2
>> > +ref=$3
>> > +
>> > +set_params() {
>> > + case "$subtree_name" in
>> > + mbedtls)
>> > + path=lib/mbedtls/external/mbedtls
>>
>> Although I am in favour of making this script generic, this subdir
>> path doesn't exist yet. Wouldn't it make more sense to add mbedtls
>> here alongside the corresponding patch-set? Otherwise the script will
>> just fail.
>>
> Actually this patch is split from the MbedTLS series which is under review.
> I think this should be fine before merging the MbedTLS/LWIP series, since
> it is supposed to be run by maintainers only.
Developers also use this script to cherry-pick DTS patches from
upstream to enable certain features in U-Boot.
> And the failing message explains
> that the sub-project dir does not exist which should be sufficient at the
> moment.
Sure, I am only afraid that if this patch lands in a prior U-Boot
release than the actual MbedTLS/LWIP series then we are just giving
users a somewhat broken tool. If your worry is about merge conflicts
then I suppose that should be taken care off while merging
corresponding patch series.
-Sumit
>
> Regards,
> Raymond
