[libcamera-devel] [PATCH 09/38] utils: ipc: add generator script

[Laurent Pinchart]

Hi Paul,

Thank you for the patch.

On Tue, Sep 22, 2020 at 10:35:08PM +0900, Paul Elder wrote:
> We want to avoid changing our copy of mojo to make updates easier. Some
> parameters in the mojo generator script needs to be changed though; add

s/needs/need/

> a wrapper script that sets these parameters.
> 
> Signed-off-by: Paul Elder <paul.elder at ideasonboard.com>
> 
> ---
> Changes in v2:
> - add descriptions to python setup
> - disable pycache
> ---
>  utils/ipc/generate.py | 21 +++++++++++++++++++++
>  1 file changed, 21 insertions(+)
>  create mode 100755 utils/ipc/generate.py
> 
> diff --git a/utils/ipc/generate.py b/utils/ipc/generate.py
> new file mode 100755
> index 00000000..3a76c37c
> --- /dev/null
> +++ b/utils/ipc/generate.py
> @@ -0,0 +1,21 @@
> +#!/usr/bin/env python3

Missing SPDX header.

A small comment block to tell what this script does would be useful.

#
# generator.py - Wrapper around mojo to override its default behaviour without
# modifying mojo itself.
#

> +
> +import os
> +import sys
> +

# Importing python code will create __pycache__ directories in the source tree,
# which can cause issues when the build is sandboxed (for Chrome OS workon
# ebuilds for instance). Let's disable bytecode cache to avoid this.

> +sys.dont_write_bytecode = True

Starting with python3.8, you could also set sys.pycache_prefix. Should
this be recorded in a TODO comment ?

> +
> +import mojo.public.tools.bindings.mojom_bindings_generator as generator
> +
> +def _GetModulePath(path, output_dir):
> +  return os.path.join(output_dir, path.relative_path())
> +
> +# Override the mojo code generator's generator list to only contain our
> +# libcamera generator
> +generator._BUILTIN_GENERATORS = {'libcamera': 'mojom_libcamera_generator'}
> +
> +# Override the mojo code generator's _GetModulePath method to not add
> +# the '-module' suffix when searching for mojo modules

Could you expend this comment to explain why this is needed ?

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

> +generator._GetModulePath = _GetModulePath
> +
> +generator.main()

-- 
Regards,

Laurent Pinchart