Re: [PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
On Wed, Jun 05, 2024 at 10:11:04AM +0300, Ilias Apalodimas wrote: > On Tue, 4 Jun 2024 at 23:10, Andy Shevchenko > wrote: > > On Fri, May 31, 2024 at 09:32:38AM +0300, Ilias Apalodimas wrote: > > > On Tue, 28 May 2024 at 17:10, Raymond Mao wrote: ... > > > > +if ! git remote get-url mbedtls_upstream 2>/dev/null > > > > > > if [ -z "$(git remote get-url rigin 2>/dev/null)" ]; then > > > > Why? I mean why do we need an additional `test` call? Above can be > > transformed > > to `foo && {}` notation to get rid of if completely. > > That's the usual syntax we have in other scripts Maybe, but it doesn't mean that "usual" syntax is not suboptimal or well portable. > > > > +then > > > > +echo "Warning: Script automatically adds new git remote via:" > > > > +echo "git remote add mbedtls_upstream \\" > > > > +echo "https://github.com/Mbed-TLS/mbedtls.git"; > > > > +git remote add mbedtls_upstream \ > > > > +https://github.com/Mbed-TLS/mbedtls.git > > > > +fi > > > > +git fetch mbedtls_upstream master > > > > +} ... > > > > +if [ "$1" = "pull" ] > > > > > > "$1" == 'pull' > > > > Why? Isn't this bashism? > > You don't need variable expansion here, so I don't see why you need > "". Unless you mean the ==, I am fine leaving that to = The latter one. Since the script is shebanged with sh, it should be compatible with any shell. Shell is hard to learn programming language, more than 98% people can't write shell scripts properly, unfortunately. But this is our legacy... > > > Also on string literals, you don't need "", 'pull' is enough That's okay. ... > > > > +elif [ "$1" = "pick" ] > > > move then 'then' one line up and add a ; > > > > > == 'pick' > > > > Ditto. > > > > > > +then > > > > +remote_add_and_fetch > > > > +git cherry-pick -x --strategy=subtree \ > > > > +-Xsubtree=lib/mbedtls/external/mbedtls/ "$2" > > > > +else > > > > +echo "usage: $0 " > > > > +echo " pull or pick" > > > > +echo " release tag [pull] or commit id [pick]" > > > > +fi > > > > Sheel should be written as much as portable and less verbose. -- With Best Regards, Andy Shevchenko
Re: [PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
On Tue, 4 Jun 2024 at 23:10, Andy Shevchenko wrote: > > On Fri, May 31, 2024 at 09:32:38AM +0300, Ilias Apalodimas wrote: > > On Tue, 28 May 2024 at 17:10, Raymond Mao wrote: > > > > > > lib/mbedtls/update-mbedtls-subtree.sh is a wrapper of git subtree > > > commands. > > > Usage from U-Boot top directory, run: > > ... > > > > +if ! git remote get-url mbedtls_upstream 2>/dev/null > > > > if [ -z "$(git remote get-url rigin 2>/dev/null)" ]; then > > Why? I mean why do we need an additional `test` call? Above can be transformed > to `foo && {}` notation to get rid of if completely. That's the usual syntax we have in other scripts > > > > +then > > > +echo "Warning: Script automatically adds new git remote via:" > > > +echo "git remote add mbedtls_upstream \\" > > > +echo "https://github.com/Mbed-TLS/mbedtls.git"; > > > +git remote add mbedtls_upstream \ > > > +https://github.com/Mbed-TLS/mbedtls.git > > > +fi > > > +git fetch mbedtls_upstream master > > > +} > > ... > > > > +if [ "$1" = "pull" ] > > > > "$1" == 'pull' > > Why? Isn't this bashism? You don't need variable expansion here, so I don't see why you need "". Unless you mean the ==, I am fine leaving that to = > > > Also on string literals, you don't need "", 'pull' is enough > > > > > +then > > > +remote_add_and_fetch > > > +git subtree pull --prefix lib/mbedtls/external/mbedtls > > > mbedtls_upstream \ > > > +"$2" --squash -m "${merge_commit_msg}" > > > +elif [ "$1" = "pick" ] > > move then 'then' one line up and add a ; > > > == 'pick' > > Ditto. > > > > +then > > > +remote_add_and_fetch > > > +git cherry-pick -x --strategy=subtree \ > > > +-Xsubtree=lib/mbedtls/external/mbedtls/ "$2" > > > +else > > > +echo "usage: $0 " > > > +echo " pull or pick" > > > +echo " release tag [pull] or commit id [pick]" > > > +fi > > Sheel should be written as much as portable and less verbose. > > -- > With Best Regards, > Andy Shevchenko > >
Re: [PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
On Fri, May 31, 2024 at 09:32:38AM +0300, Ilias Apalodimas wrote: > On Tue, 28 May 2024 at 17:10, Raymond Mao wrote: > > > > lib/mbedtls/update-mbedtls-subtree.sh is a wrapper of git subtree > > commands. > > Usage from U-Boot top directory, run: ... > > +if ! git remote get-url mbedtls_upstream 2>/dev/null > > if [ -z "$(git remote get-url rigin 2>/dev/null)" ]; then Why? I mean why do we need an additional `test` call? Above can be transformed to `foo && {}` notation to get rid of if completely. > > +then > > +echo "Warning: Script automatically adds new git remote via:" > > +echo "git remote add mbedtls_upstream \\" > > +echo "https://github.com/Mbed-TLS/mbedtls.git"; > > +git remote add mbedtls_upstream \ > > +https://github.com/Mbed-TLS/mbedtls.git > > +fi > > +git fetch mbedtls_upstream master > > +} ... > > +if [ "$1" = "pull" ] > > "$1" == 'pull' Why? Isn't this bashism? > Also on string literals, you don't need "", 'pull' is enough > > > +then > > +remote_add_and_fetch > > +git subtree pull --prefix lib/mbedtls/external/mbedtls > > mbedtls_upstream \ > > +"$2" --squash -m "${merge_commit_msg}" > > +elif [ "$1" = "pick" ] > move then 'then' one line up and add a ; > == 'pick' Ditto. > > +then > > +remote_add_and_fetch > > +git cherry-pick -x --strategy=subtree \ > > +-Xsubtree=lib/mbedtls/external/mbedtls/ "$2" > > +else > > +echo "usage: $0 " > > +echo " pull or pick" > > +echo " release tag [pull] or commit id [pick]" > > +fi Sheel should be written as much as portable and less verbose. -- With Best Regards, Andy Shevchenko
Re: [PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
On Tue, 28 May 2024 at 17:10, Raymond Mao wrote: > > lib/mbedtls/update-mbedtls-subtree.sh is a wrapper of git subtree > commands. > Usage from U-Boot top directory, run: > > $ ./lib/mbedtls/update-mbedtls-subtree.sh pull > $ ./lib/mbedtls/update-mbedtls-subtree.sh pick > > Signed-off-by: Raymond Mao > --- > Changes in v2 > - Initial patch. > Changes in v3 > - None. > > lib/mbedtls/update-mbedtls-subtree.sh | 50 +++ > 1 file changed, 50 insertions(+) > create mode 100755 lib/mbedtls/update-mbedtls-subtree.sh > > diff --git a/lib/mbedtls/update-mbedtls-subtree.sh > b/lib/mbedtls/update-mbedtls-subtree.sh > new file mode 100755 > index 000..f208e54a5af > --- /dev/null > +++ b/lib/mbedtls/update-mbedtls-subtree.sh > @@ -0,0 +1,50 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0+ > +# > +# Copyright 2024 Linaro Ltd. > +# > +# Usage: from the top level U-Boot source tree, run: > +# $ ./lib/mbedtls/update-mbedtls-subtree.sh pull > +# $ ./lib/mbedtls/update-mbedtls-subtree.sh pick LWIP will add similar machinery, we want to merge these in a single script. So, please move this to tools/ and adjust it accordingly as a first step > +# > +# The script will pull changes from MbedTLS repo into U-Boot > +# as a subtree located as /lib/mbedtls/external/mbedtls > 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 MbedTLS repo [1] into lib/mbedtls/external/mbedtls > + > +[1] https://github.com/Mbed-TLS/mbedtls.git > +EOF > +) > + > +remote_add_and_fetch() { > +if ! git remote get-url mbedtls_upstream 2>/dev/null if [ -z "$(git remote get-url rigin 2>/dev/null)" ]; then > +then > +echo "Warning: Script automatically adds new git remote via:" > +echo "git remote add mbedtls_upstream \\" > +echo "https://github.com/Mbed-TLS/mbedtls.git"; > +git remote add mbedtls_upstream \ > +https://github.com/Mbed-TLS/mbedtls.git > +fi > +git fetch mbedtls_upstream master > +} > + > +if [ "$1" = "pull" ] "$1" == 'pull' Also on string literals, you don't need "", 'pull' is enough > +then > +remote_add_and_fetch > +git subtree pull --prefix lib/mbedtls/external/mbedtls mbedtls_upstream \ > +"$2" --squash -m "${merge_commit_msg}" > +elif [ "$1" = "pick" ] move then 'then' one line up and add a ; == 'pick' > +then > +remote_add_and_fetch > +git cherry-pick -x --strategy=subtree \ > +-Xsubtree=lib/mbedtls/external/mbedtls/ "$2" > +else > +echo "usage: $0 " > +echo " pull or pick" > +echo " release tag [pull] or commit id [pick]" > +fi > -- > 2.25.1 > Cheers /Ilias
[PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
lib/mbedtls/update-mbedtls-subtree.sh is a wrapper of git subtree commands. Usage from U-Boot top directory, run: $ ./lib/mbedtls/update-mbedtls-subtree.sh pull $ ./lib/mbedtls/update-mbedtls-subtree.sh pick Signed-off-by: Raymond Mao --- Changes in v2 - Initial patch. Changes in v3 - None. lib/mbedtls/update-mbedtls-subtree.sh | 50 +++ 1 file changed, 50 insertions(+) create mode 100755 lib/mbedtls/update-mbedtls-subtree.sh diff --git a/lib/mbedtls/update-mbedtls-subtree.sh b/lib/mbedtls/update-mbedtls-subtree.sh new file mode 100755 index 000..f208e54a5af --- /dev/null +++ b/lib/mbedtls/update-mbedtls-subtree.sh @@ -0,0 +1,50 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# Copyright 2024 Linaro Ltd. +# +# Usage: from the top level U-Boot source tree, run: +# $ ./lib/mbedtls/update-mbedtls-subtree.sh pull +# $ ./lib/mbedtls/update-mbedtls-subtree.sh pick +# +# The script will pull changes from MbedTLS repo into U-Boot +# as a subtree located as /lib/mbedtls/external/mbedtls 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 MbedTLS repo [1] into lib/mbedtls/external/mbedtls + +[1] https://github.com/Mbed-TLS/mbedtls.git +EOF +) + +remote_add_and_fetch() { +if ! git remote get-url mbedtls_upstream 2>/dev/null +then +echo "Warning: Script automatically adds new git remote via:" +echo "git remote add mbedtls_upstream \\" +echo "https://github.com/Mbed-TLS/mbedtls.git"; +git remote add mbedtls_upstream \ +https://github.com/Mbed-TLS/mbedtls.git +fi +git fetch mbedtls_upstream master +} + +if [ "$1" = "pull" ] +then +remote_add_and_fetch +git subtree pull --prefix lib/mbedtls/external/mbedtls mbedtls_upstream \ +"$2" --squash -m "${merge_commit_msg}" +elif [ "$1" = "pick" ] +then +remote_add_and_fetch +git cherry-pick -x --strategy=subtree \ +-Xsubtree=lib/mbedtls/external/mbedtls/ "$2" +else +echo "usage: $0 " +echo " pull or pick" +echo " release tag [pull] or commit id [pick]" +fi -- 2.25.1