[libcamera-devel] [PATCH] utils: update-mojo.sh: Add script for updating mojo

[paul.elder at ideasonboard.com]

Hi Kieran,

On Wed, May 19, 2021 at 09:43:15AM +0100, Kieran Bingham wrote:
> Hi Paul,
> 
> On 19/05/2021 06:25, Paul Elder wrote:
> > Add a script to ease updating mojo from a chromium source tree.
> 
> This sounds better than manually trying to import each time, and ensures
> the process and files required are (self)documented.
> 
> 
> > 
> > Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> > 
> > ---
> > Note that this script includes the little patch to template_expander.py
> > to enable it to work with jinja2 3.0.0 (it still works with jinja2
> > 2.10.x).
> > ---
> >  utils/update-mojo.sh | 68 ++++++++++++++++++++++++++++++++++++++++++++
> >  1 file changed, 68 insertions(+)
> >  create mode 100755 utils/update-mojo.sh
> > 
> > diff --git a/utils/update-mojo.sh b/utils/update-mojo.sh
> > new file mode 100755
> > index 00000000..a57ea968
> > --- /dev/null
> > +++ b/utils/update-mojo.sh
> > @@ -0,0 +1,68 @@
> > +#!/bin/sh
> > +
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Update mojo copy from a chromium source tree
> > +
> > +if [ $# != 1 ] ; then
> > +	echo "Usage: $0 <chromium dir>"
> > +	exit 1
> > +fi
> > +
> > +ipc_dir="$(dirname "$(realpath "$0")")/ipc"
> 
> Wow, that's some interesting quoting.
> i didn't realise you could split things up quite like that though.

I copied it from what Laurent wrote in update-kernel-headers.sh :D

> 
> I can't tell how quotes get parsed ... (swapping " " for [ ]) :
> 
> A)
>   ipc_dir="$(dirname "   $(realpath    "$0"   )   ")/ipc"
>   ipc_dir=[$(dirname ]   $(realpath    [$0]   )   [)/ipc]
>           [          ]                 [  ]       [     ]
> 
> or
> B)
> 
>   ipc_dir="$(dirname "   $(realpath    "$0"   )   ")/ipc"
>   ipc_dir=[$(dirname [   $(realpath    [$0]   )   ])/ipc]
>           [          [                 [  ]       ]     ]

I've noticed elsewhere in this script though that it seems to nest
double quotes properly.

> 
> Or maybe it doesn't matter and it works...
> 
> I always thought quotes were parsed as A (sequential quotes open and
> close the quote), but that wouldn't make sense as a functional line to
> me, so I would expect B) to be the desired quoting, where a balanced
> string opens the quotes then closes them again...
> 
> 
> 
> 
> 
> > +chromium_dir="$1"
> > +
> > +if [ ! -d "${chromium_dir}/mojo" ] ; then
> > +	echo "Directory ${chromium_dir} doesn't contain mojo"
> > +	exit 1
> > +fi
> > +
> > +if [ ! -d "${chromium_dir}/.git" ] ; then
> > +	echo "Directory ${chromium_dir} doesn't contain a git tree"
> > +	exit 1
> > +fi
> > +
> > +# Get the chromium commit id
> > +version=$(git -C "${chromium_dir}" rev-parse --short HEAD)
> > +
> > +# Reject dirty trees
> > +if ! test -z "$(git -C "${chromium_dir}" status --porcelain)" ; then
> > +	echo "Chromium tree in ${chromium_dir} is dirty"
> > +	exit 1
> > +fi
> > +
> > +# Copy the diagnosis file
> > +cp "${chromium_dir}/tools/diagnosis/crbug_1001171.py" "${ipc_dir}/tools/diagnosis"
> > +
> > +# Copy the rest of mojo
> > +cp "${chromium_dir}/mojo/public/LICENSE" "${ipc_dir}/mojo/public"
> > +
> > +last_dir="$(pwd)"
> > +cd "${chromium_dir}" || exit
> > +find "./mojo/public/tools" -type f \
> > +     -not -path "*/generators/*" \
> > +     -not -path "*/fuzzers/*" \
> > +     -exec cp --parents "{}" "${ipc_dir}" ";"
> > +cd "$last_dir" || exit
> > +
> > +# Update the README files
> > +readme=$(cat <<EOF
> > +# SPDX-License-Identifier: CC0-1.0
> > +
> > +Files in this directory are imported from ${version} of Chromium. Do not
> 
> Nice touch automatically specifying the imported version!
> 
> > +modify them manually.
> > +EOF
> > +)
> > +
> > +echo "$readme" > "${ipc_dir}/mojo/README"
> > +echo "$readme" > "${ipc_dir}/tools/README"
> > +
> > +# Fix mojo support for jinja2 3.0.0
> > +template_expander="${ipc_dir}/mojo/public/tools/mojom/mojom/generate/template_expander.py"
> > +sed -i "/py_compile=.*$/d" "${template_expander}"
> 
> Interesting that we would need to carry a manual patch like that - but
> if that's the solution, I think it's fine for now.

I still need to update the bugzilla on this, but basically this
parameter exists in jinja2 2.10.x but not jinja2 3.0.0 which causes it
to break, but the parameter is ignored in Python 3 anyway so it should
be fine to remove it.

Still trying to figure out how I can send a patch upstream to
chromium...

> 
> Hopefully upstream will deal with this in the future, assuming it
> affects other users, at which point this can be removed.
> 

They had a patch that prepared mojo to be compatible with Python 3 but
still left that flag as a possibility for Python 2, so not sure what
they were trying to do there.

> Reviewed-by: Kieran Bingham <kieran.bingham at ideasonboard.com>


Thanks,

Paul

> 
> 
> > +
> > +cat <<EOF
> > +------------------------------------------------------------
> > +mojo updated. Please review and up-port local changes before
> > +committing.
> > +------------------------------------------------------------
> > +EOF
> >