[libcamera-devel] utils: update-mojo.sh: Add script for updating mojo
diff mbox series

Message ID 20210519052510.899058-1-paul.elder@ideasonboard.com
State Accepted
Headers show
Series
  • [libcamera-devel] utils: update-mojo.sh: Add script for updating mojo
Related show

Commit Message

Paul Elder May 19, 2021, 5:25 a.m. UTC 
Add a script to ease updating mojo from a chromium source tree.

Signed-off-by: Paul Elder <paul.elder@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

Comments

Kieran Bingham May 19, 2021, 8:43 a.m. UTC | #1 
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@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 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]
          [          [                 [  ]       ]     ]

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.

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

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


> +
> +cat <<EOF
> +------------------------------------------------------------
> +mojo updated. Please review and up-port local changes before
> +committing.
> +------------------------------------------------------------
> +EOF
>
Paul Elder May 19, 2021, 10:27 a.m. UTC | #2 
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@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@ideasonboard.com>


Thanks,

Paul

> 
> 
> > +
> > +cat <<EOF
> > +------------------------------------------------------------
> > +mojo updated. Please review and up-port local changes before
> > +committing.
> > +------------------------------------------------------------
> > +EOF
> >
Laurent Pinchart May 25, 2021, 1:04 a.m. UTC | #3 
Hi Paul,

Thank you for the patch.

On Wed, May 19, 2021 at 02:25:10PM +0900, Paul Elder wrote:
> Add a script to ease updating mojo from a chromium source tree.
> 
> Signed-off-by: Paul Elder <paul.elder@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"
> +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

You can use [ instead of test

if [ -n "$(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 \

You can drop quotes around strings that are guaranteed not to need them.

> +     -not -path "*/generators/*" \
> +     -not -path "*/fuzzers/*" \
> +     -exec cp --parents "{}" "${ipc_dir}" ";"
> +cd "$last_dir" || exit

You can create a scope instead of restoring to the last_dir manually.

(
	cd "${chromium_dir}" || exit
	find ./mojo/public/tools -type f \
	     -not -path "*/generators/*" \
	     -not -path "*/fuzzers/*" \
	     -exec cp --parents {} "${ipc_dir}" ";"
)

Should the script first empty the mojo/publics/tools/ directory, in case
upstream deletes some files ?

> +
> +# 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
> +modify them manually.
> +EOF
> +)
> +
> +echo "$readme" > "${ipc_dir}/mojo/README"
> +echo "$readme" > "${ipc_dir}/tools/README"
> +
> +# Fix mojo support for jinja2 3.0.0

With a link to
https://chromium-review.googlesource.com/c/chromium/src/+/2908396 in
order to drop this when the fix will be integrated,

Reviewed-by: Laurent Pinchart <laurent.pinchart@ideasonboard.com>

> +template_expander="${ipc_dir}/mojo/public/tools/mojom/mojom/generate/template_expander.py"
> +sed -i "/py_compile=.*$/d" "${template_expander}"
> +
> +cat <<EOF
> +------------------------------------------------------------
> +mojo updated. Please review and up-port local changes before
> +committing.
> +------------------------------------------------------------
> +EOF

Patch
diff mbox series 

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"
+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
+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}"
+
+cat <<EOF
+------------------------------------------------------------
+mojo updated. Please review and up-port local changes before
+committing.
+------------------------------------------------------------
+EOF