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