[PATCH v3 02/25] mbedtls: Add script to update MbedTLS subtree
Andy Shevchenko
andriy.shevchenko at linux.intel.com
Wed Jun 5 11:27:46 CEST 2024
On Wed, Jun 05, 2024 at 10:11:04AM +0300, Ilias Apalodimas wrote:
> On Tue, 4 Jun 2024 at 23:10, Andy Shevchenko
> <andriy.shevchenko at linux.intel.com> wrote:
> > On Fri, May 31, 2024 at 09:32:38AM +0300, Ilias Apalodimas wrote:
> > > On Tue, 28 May 2024 at 17:10, Raymond Mao <raymond.mao at linaro.org> 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 <op> <ref>"
> > > > + echo " <op> pull or pick"
> > > > + echo " <ref> release tag [pull] or commit id [pick]"
> > > > +fi
> >
> > Sheel should be written as much as portable and less verbose.
--
With Best Regards,
Andy Shevchenko
More information about the U-Boot
mailing list