Hi Quentin,

On Tue, 23 Jul 2024 at 03:16, Quentin Schulz <quentin.sch...@cherry.de>
wrote:

> Hi Raymond,
>
> On 7/22/24 9:30 PM, Raymond Mao 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
>
> We have scripts/ and tools/ directories, this being a shell script I
> would have intuitively added it to scripts/. So my question is, what's
> the difference between those two directories :) ?
>
> Good question. Personally I think 'scripts' are for those scripts needed
by other components and 'tools' are for the general purpose utilities
(not limited to scripts). That is the reason I prefer 'tools'.

> @@ -0,0 +1,86 @@
> > +#!/bin/sh
> > +# SPDX-License-Identifier: GPL-2.0+
> > +#
> > +# Copyright (c) 2024 Linaro Limited
> > +# Author: Raymond Mao <raymond....@linaro.org>
> > +#
> > +# 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"
>
> I think pulls are supposed to be done and sent to the ML by maintainers
> only? At least that's what we're supposed to do for the dts/upstream/
> directory AFAIR. Is it the same for mbedtls and lwip? If so, maybe
> explicit this in the usage that only maintainers are supposed to run
> this script with that option? And if you need to do it, ask the
> maintainers to do it for you? Does this make sense?
>
> The rules of running this script are the same as the original one
'dts/update-dts-subtree.sh'. We have the documentation for dts now,
and will have similar ones for MbedTLS and LWIP soon.
I didn't add this part yet since both MbedTLS and LWIP patch series
are still under review.


> > +    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
> > +            repo_url=https://github.com/Mbed-TLS/mbedtls.git
> > +            remote_name="mbedtls_upstream"
> > +            ;;
> > +        dts)
> > +            path=dts/upstream
> > +            repo_url=
> https://git.kernel.org/pub/scm/linux/kernel/git/devicetree/devicetree-rebasing.git
> > +            remote_name="devicetree-rebasing"
> > +            ;;
> > +        lwip)
> > +            path=lib/lwip/lwip
> > +            repo_url=https://git.savannah.gnu.org/git/lwip.git
> > +            remote_name="lwip_upstream"
> > +            ;;
> > +        *)
> > +            echo "Invalid subtree name: $subtree_name"
> > +            print_usage
> > +            exit 1
> > +    esac
> > +}
> > +
> > +set_params
> > +
> > +merge_commit_msg=$(cat << EOF
> > +Subtree merge tag '$ref' of $subtree_name repo [1] into $path
> > +
>
> I would recommend adding at the very least the path of the tools used to
> generate this merge commit (so ./tools/update-subtree.sh or maybe just
> $0?) or maybe even the full command line that was used to do this merge?
>
> Sorry but in any case would we need this commit message only without
knowing
the variables?


> > +[1] $repo_url
> > +EOF
> > +)
> > +
> > +remote_add_and_fetch() {
> > +    if [ -z "$(git remote get-url $remote_name 2>/dev/null)" ]; then
> > +        echo "Warning: Script automatically adds new git remote via:"
> > +        echo "    git remote add $remote_name \\"
> > +        echo "        $repo_url"
> > +        git remote add $remote_name $repo_url
> > +    fi
> > +    git fetch $remote_name master
> > +}
> > +
> > +if [ "$op" = "pull" ]; then
> > +    remote_add_and_fetch
> > +    git subtree pull --prefix $path $remote_name "$ref" --squash -m
> "$merge_commit_msg"
> > +elif [ "$op" = "pick" ]; then
> > +    remote_add_and_fetch
> > +    git cherry-pick -x --strategy=subtree -Xsubtree=$path/ "$ref"
>
> Should we add -s there too? (or --signoff)
>
> As I search for all 'merge' commits in history, they have not
signed-off tags.
This patch is to extend the original script to support more git upstream
repos.
So I intended to keep the commit message format as the original one unless
we have reasons to improve it.

Regards,
Raymond

Reply via email to