[v1,3/8] utils: Add script to generate control_ids_debug.yaml

21474 diff mbox series

For flexible debugging it is helpful to minimize the roundtrip time.
This scipts parses the sourcetree and loos for usages of

set<type>(controls::debug::Something, ...)

and adds (or removes) the controls as necessary from the yaml
description.

Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
---
 utils/gen-debug-controls.py | 161 ++++++++++++++++++++++++++++++++++++
 1 file changed, 161 insertions(+)
 create mode 100755 utils/gen-debug-controls.py

Hi Stefan,

Thank you for the patch.

On Wed, Oct 02, 2024 at 06:19:21PM +0200, Stefan Klug wrote:
> For flexible debugging it is helpful to minimize the roundtrip time.
> This scipts parses the sourcetree and loos for usages of

s/scipts/script/
s/loos/looks/

> 
> set<type>(controls::debug::Something, ...)
> 
> and adds (or removes) the controls as necessary from the yaml
> description.

It's worth mentioning it (and in the script itself too) that this is
meant as a development tool, to ease updating the yaml file when making
changes to IPA modules.

> 
> Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> ---
>  utils/gen-debug-controls.py | 161 ++++++++++++++++++++++++++++++++++++
>  1 file changed, 161 insertions(+)
>  create mode 100755 utils/gen-debug-controls.py
> 
> diff --git a/utils/gen-debug-controls.py b/utils/gen-debug-controls.py
> new file mode 100755
> index 000000000000..6cb087e1e0ae
> --- /dev/null
> +++ b/utils/gen-debug-controls.py
> @@ -0,0 +1,161 @@
> +#!/usr/bin/env python3
> +# SPDX-License-Identifier: GPL-2.0-or-later
> +# Copyright (C) 2024, Ideas on Board Inc.
> +#
> +# Author: Stefan Klug <stefan.klug@ideasonboard.com>
> +#
> +# This script looks for occurrences of the debug metadata controls in the source
> +# tree and updates src/libcamera/control_ids_debug.yaml accordingly
> +
> +import argparse
> +import logging
> +import os
> +import re
> +import sys
> +from dataclasses import dataclass
> +from pathlib import Path
> +
> +logger = logging.getLogger(__name__)
> +logging.basicConfig(level=logging.INFO, format='%(levelname)s: %(message)s')
> +
> +try:
> +    import ruyaml
> +except:
> +    logger.error(
> +        f'Failed to import ruyaml. Please install the ruyaml package.')

This is only available through pip and not packaged by Debian. Please
use the yaml module instead, I'd like to avoid adding another
dependency.

I suspect you may tell me that pyyaml doesn't preserve comments while
ruyaml does. Could we at least use ruamel then, which is packaged by
Debian ? Or reconsider the need to preserve comments ? This tool is
meant to assist developers, so I think we can do a best effort and
assume usage of 'git add -p' for the time being.

> +    sys.exit(1)
> +
> +
> +@dataclass
> +class FoundMatch:
> +    file: os.PathLike
> +    whole_match: str
> +    line: int
> +    type: str
> +    name: str
> +    size: str = None
> +
> +
> +def get_control_name(control):
> +    k = list(control.keys())
> +    if len(k) != 1:
> +        raise Exception(f"Can't handle control entry with {len(k)} keys")
> +    return k[0]
> +
> +
> +def find_debug_controls(dir):
> +    extensions = ['.cpp', '.h']
> +    files = [p for p in dir.rglob('*') if p.suffix in extensions]
> +
> +    # The following regex was tested on
> +    # set<Span<type>>( controls::debug::something , static_cast<type>(var) )
> +    # set<>( controls::debug::something , static_cast<type>(var) )
> +    # set( controls::debug::something , static_cast<type> (var) )
> +    exp = re.compile(r'set'  # set function
> +                     # possibly followed by template param
> +                     r'(?:\<((?:[^)(])*)\>)?'
> +                     # referencing a debug control
> +                     r'\(\s*controls::debug::(\w+)\s*,.*\)'
> +                     )
> +    matches = []
> +    for p in files:
> +        with p.open('r') as f:
> +            for idx, line in enumerate(f):
> +                match = exp.search(line)
> +                if match:
> +                    m = FoundMatch(file=p, line=idx, type=match.group(1),
> +                                   name=match.group(2), whole_match=match.group(0))
> +                    if m.type is not None and m.type.startswith('Span'):
> +                        # simple span type detection treating the last word inside <> as type
> +                        r = re.match(r'Span<(?:.*\s+)(.*)>', m.type)
> +                        m.type = r.group(1)
> +                        m.size = '[n]'
> +                    matches.append(m)
> +    return matches
> +
> +
> +def main(argv):
> +    parser = argparse.ArgumentParser(
> +        description='Automatically updates control_ids_debug.yaml',)

I think you can drop the trailing comma.

> +    args = parser.parse_args(argv[1:])

args seems unused. I assume this is meant to print a help text. You can
drop the args variable and just call parse_args() then.

> +
> +    yaml = ruyaml.YAML()
> +    root_dir = Path(__file__).resolve().parent.parent
> +    output = root_dir.joinpath('src/libcamera/control_ids_debug.yaml')

Hmmmm... I suppose there's no real use case for specifying the file as a
comment line argument.

I would rename 'output' as it's both an input and an output file. Maybe
ctrl_file or something similar.

> +
> +    matches = find_debug_controls(root_dir.joinpath('src'))
> +
> +    doc = yaml.load(output)
> +
> +    controls = doc['controls']
> +
> +    # create a map of names in the existing yaml for easier updating

    # Create a map of names in the existing yaml for easier updating.

> +    controls_map = {}
> +    for control in controls:
> +        for k, v in control.items():
> +            controls_map[k] = v
> +
> +    obsolete_names = list(controls_map.keys())
> +
> +    for m in matches:
> +        if not m.type:
> +            p = m.file.relative_to(Path.cwd(), walk_up=True)
> +            logger.warning(
> +                f'{p}:{m.line + 1}: Failed to deduce type from {m.whole_match} ... skipping')
> +            continue
> +
> +        p = m.file.relative_to(root_dir)
> +        desc = {'type': m.type,
> +                'description': f'Debug control {m.name} found in {p}:{m.line}'}
> +        if m.size is not None:
> +            desc['size'] = m.size
> +
> +        if m.name in controls_map:
> +            # Can't use == for modified check because of the special yaml dicts.
> +            update_needed = False
> +            if list(controls_map[m.name].keys()) != list(desc.keys()):
> +                update_needed = True
> +            else:
> +                for k, v in controls_map[m.name].items():
> +                    if v != desc[k]:
> +                        update_needed = True
> +                        break
> +
> +            if update_needed:
> +                logger.info(f"Update control '{m.name}'")
> +                controls_map[m.name].clear()
> +                controls_map[m.name].update(desc)
> +
> +            obsolete_names.remove(m.name)
> +        else:
> +            logger.info(f"Add control '{m.name}'")
> +            insert_before = len(controls)
> +            for idx, control in enumerate(controls):
> +                if get_control_name(control).lower() > m.name.lower():
> +                    insert_before = idx
> +                    break
> +            controls.insert(insert_before, {m.name: desc})
> +
> +    # Remove elements from controls without recreating the list (to keep comments etc.)

    # Remove elements from controls without recreating the list (to keep
    # comments etc.).

> +    idx = 0
> +    while idx < len(controls):
> +        name = get_control_name(controls[idx])
> +        if name in obsolete_names:
> +            logger.info(f"Remove control '{name}'")
> +            controls.pop(idx)
> +        else:
> +            idx += 1
> +
> +    with output.open('w') as f:
> +        # ruyaml looses the copyright header
> +        f.write(("# SPDX-License-Identifier: LGPL-2.1-or-later\n"
> +                 "#\n"
> +                 "# Copyright (C) 2024, Ideas on Board Oy\n"

Maybe print the current year (or "2024-current year") to be more
future-proof ?

> +                 "#\n"))
> +        yaml.dump(doc, f)
> +
> +    return 0
> +
> +
> +if __name__ == '__main__':
> +    sys.exit(main(sys.argv))

Hi Laurent,

Thank you for the review. 

On Thu, Oct 03, 2024 at 11:26:43PM +0300, Laurent Pinchart wrote:
> Hi Stefan,
> 
> Thank you for the patch.
> 
> On Wed, Oct 02, 2024 at 06:19:21PM +0200, Stefan Klug wrote:
> > For flexible debugging it is helpful to minimize the roundtrip time.
> > This scipts parses the sourcetree and loos for usages of
> 
> s/scipts/script/
> s/loos/looks/
> 
> > 
> > set<type>(controls::debug::Something, ...)
> > 
> > and adds (or removes) the controls as necessary from the yaml
> > description.
> 
> It's worth mentioning it (and in the script itself too) that this is
> meant as a development tool, to ease updating the yaml file when making
> changes to IPA modules.
> 

Ack.

> > 
> > Signed-off-by: Stefan Klug <stefan.klug@ideasonboard.com>
> > ---
> >  utils/gen-debug-controls.py | 161 ++++++++++++++++++++++++++++++++++++
> >  1 file changed, 161 insertions(+)
> >  create mode 100755 utils/gen-debug-controls.py
> > 
> > diff --git a/utils/gen-debug-controls.py b/utils/gen-debug-controls.py
> > new file mode 100755
> > index 000000000000..6cb087e1e0ae
> > --- /dev/null
> > +++ b/utils/gen-debug-controls.py
> > @@ -0,0 +1,161 @@
> > +#!/usr/bin/env python3
> > +# SPDX-License-Identifier: GPL-2.0-or-later
> > +# Copyright (C) 2024, Ideas on Board Inc.
> > +#
> > +# Author: Stefan Klug <stefan.klug@ideasonboard.com>
> > +#
> > +# This script looks for occurrences of the debug metadata controls in the source
> > +# tree and updates src/libcamera/control_ids_debug.yaml accordingly
> > +
> > +import argparse
> > +import logging
> > +import os
> > +import re
> > +import sys
> > +from dataclasses import dataclass
> > +from pathlib import Path
> > +
> > +logger = logging.getLogger(__name__)
> > +logging.basicConfig(level=logging.INFO, format='%(levelname)s: %(message)s')
> > +
> > +try:
> > +    import ruyaml
> > +except:
> > +    logger.error(
> > +        f'Failed to import ruyaml. Please install the ruyaml package.')
> 
> This is only available through pip and not packaged by Debian. Please
> use the yaml module instead, I'd like to avoid adding another
> dependency.
> 

In ubuntu it is available as python3-ruyaml. ruamel seems quite
unmaintained. I could support both. Or go back to yaml (see below).

> I suspect you may tell me that pyyaml doesn't preserve comments while
> ruyaml does. Could we at least use ruamel then, which is packaged by
> Debian ? Or reconsider the need to preserve comments ? This tool is
> meant to assist developers, so I think we can do a best effort and
> assume usage of 'git add -p' for the time being.

Yes, the preserved comments were the main reason. But ruyaml also has
line breaks that differ from the libcamera style. So maybe it doesn't
matter in the end. I'll test with yaml and see how it looks.

> 
> > +    sys.exit(1)
> > +
> > +
> > +@dataclass
> > +class FoundMatch:
> > +    file: os.PathLike
> > +    whole_match: str
> > +    line: int
> > +    type: str
> > +    name: str
> > +    size: str = None
> > +
> > +
> > +def get_control_name(control):
> > +    k = list(control.keys())
> > +    if len(k) != 1:
> > +        raise Exception(f"Can't handle control entry with {len(k)} keys")
> > +    return k[0]
> > +
> > +
> > +def find_debug_controls(dir):
> > +    extensions = ['.cpp', '.h']
> > +    files = [p for p in dir.rglob('*') if p.suffix in extensions]
> > +
> > +    # The following regex was tested on
> > +    # set<Span<type>>( controls::debug::something , static_cast<type>(var) )
> > +    # set<>( controls::debug::something , static_cast<type>(var) )
> > +    # set( controls::debug::something , static_cast<type> (var) )
> > +    exp = re.compile(r'set'  # set function
> > +                     # possibly followed by template param
> > +                     r'(?:\<((?:[^)(])*)\>)?'
> > +                     # referencing a debug control
> > +                     r'\(\s*controls::debug::(\w+)\s*,.*\)'
> > +                     )
> > +    matches = []
> > +    for p in files:
> > +        with p.open('r') as f:
> > +            for idx, line in enumerate(f):
> > +                match = exp.search(line)
> > +                if match:
> > +                    m = FoundMatch(file=p, line=idx, type=match.group(1),
> > +                                   name=match.group(2), whole_match=match.group(0))
> > +                    if m.type is not None and m.type.startswith('Span'):
> > +                        # simple span type detection treating the last word inside <> as type
> > +                        r = re.match(r'Span<(?:.*\s+)(.*)>', m.type)
> > +                        m.type = r.group(1)
> > +                        m.size = '[n]'
> > +                    matches.append(m)
> > +    return matches
> > +
> > +
> > +def main(argv):
> > +    parser = argparse.ArgumentParser(
> > +        description='Automatically updates control_ids_debug.yaml',)
> 
> I think you can drop the trailing comma.
> 
> > +    args = parser.parse_args(argv[1:])
> 
> args seems unused. I assume this is meant to print a help text. You can
> drop the args variable and just call parse_args() then.

Copy paste leftover. I always waited for args to come back, but there
were none :-)

> 
> > +
> > +    yaml = ruyaml.YAML()
> > +    root_dir = Path(__file__).resolve().parent.parent
> > +    output = root_dir.joinpath('src/libcamera/control_ids_debug.yaml')
> 
> Hmmmm... I suppose there's no real use case for specifying the file as a
> comment line argument.

Jep. I wanted to keep usage as easy as possible.

> 
> I would rename 'output' as it's both an input and an output file. Maybe
> ctrl_file or something similar.

ack

> 
> > +
> > +    matches = find_debug_controls(root_dir.joinpath('src'))
> > +
> > +    doc = yaml.load(output)
> > +
> > +    controls = doc['controls']
> > +
> > +    # create a map of names in the existing yaml for easier updating
> 
>     # Create a map of names in the existing yaml for easier updating.
> 
> > +    controls_map = {}
> > +    for control in controls:
> > +        for k, v in control.items():
> > +            controls_map[k] = v
> > +
> > +    obsolete_names = list(controls_map.keys())
> > +
> > +    for m in matches:
> > +        if not m.type:
> > +            p = m.file.relative_to(Path.cwd(), walk_up=True)
> > +            logger.warning(
> > +                f'{p}:{m.line + 1}: Failed to deduce type from {m.whole_match} ... skipping')
> > +            continue
> > +
> > +        p = m.file.relative_to(root_dir)
> > +        desc = {'type': m.type,
> > +                'description': f'Debug control {m.name} found in {p}:{m.line}'}
> > +        if m.size is not None:
> > +            desc['size'] = m.size
> > +
> > +        if m.name in controls_map:
> > +            # Can't use == for modified check because of the special yaml dicts.
> > +            update_needed = False
> > +            if list(controls_map[m.name].keys()) != list(desc.keys()):
> > +                update_needed = True
> > +            else:
> > +                for k, v in controls_map[m.name].items():
> > +                    if v != desc[k]:
> > +                        update_needed = True
> > +                        break
> > +
> > +            if update_needed:
> > +                logger.info(f"Update control '{m.name}'")
> > +                controls_map[m.name].clear()
> > +                controls_map[m.name].update(desc)
> > +
> > +            obsolete_names.remove(m.name)
> > +        else:
> > +            logger.info(f"Add control '{m.name}'")
> > +            insert_before = len(controls)
> > +            for idx, control in enumerate(controls):
> > +                if get_control_name(control).lower() > m.name.lower():
> > +                    insert_before = idx
> > +                    break
> > +            controls.insert(insert_before, {m.name: desc})
> > +
> > +    # Remove elements from controls without recreating the list (to keep comments etc.)
> 
>     # Remove elements from controls without recreating the list (to keep
>     # comments etc.).
> 
> > +    idx = 0
> > +    while idx < len(controls):
> > +        name = get_control_name(controls[idx])
> > +        if name in obsolete_names:
> > +            logger.info(f"Remove control '{name}'")
> > +            controls.pop(idx)
> > +        else:
> > +            idx += 1
> > +
> > +    with output.open('w') as f:
> > +        # ruyaml looses the copyright header
> > +        f.write(("# SPDX-License-Identifier: LGPL-2.1-or-later\n"
> > +                 "#\n"
> > +                 "# Copyright (C) 2024, Ideas on Board Oy\n"
> 
> Maybe print the current year (or "2024-current year") to be more
> future-proof ?

No problem.

Regards,
Stefan

> 
> > +                 "#\n"))
> > +        yaml.dump(doc, f)
> > +
> > +    return 0
> > +
> > +
> > +if __name__ == '__main__':
> > +    sys.exit(main(sys.argv))
> 
> -- 
> Regards,
> 
> Laurent Pinchart