Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python

[Jason Andryuk <jason.andryuk@xxxxxxx>]

On 2025-10-21 12:31, Andrew Cooper wrote:
> On 21/10/2025 5:23 pm, Anthony PERARD wrote:
> > On Thu, Oct 16, 2025 at 10:06:13PM -0400, Jason Andryuk wrote:
> > > removeprefix is only added in Python 3.9.
> > >
> > > Instead of the prefix removal, switch to passing in a "depth" parameter,
> > > and incrementing it for each level.
> > >
> > > There is a slight change in the generated _libxl_types.c.  instances of
> > > KeyedUnion increment depth without outputing any code.  The net result
> > > is some cases where jso_sub_1 is followed by jso_sub_3.  As an example:
> > >
> > > _libxl_types.c
> > > _libxl_types.c
> > > @@ -5535,12 +5535,12 @@
> > >                   if (!jso_sub_1)
> > >                       goto out;
> > >                   if (!libxl__string_is_default(&p->u.pty.path)) {
> > > -                    json_object *jso_sub_2 = NULL;
> > > -                    rc = libxl__string_gen_jso(&jso_sub_2, p->u.pty.path);
> > > +                    json_object *jso_sub_3 = NULL;
> > > +                    rc = libxl__string_gen_jso(&jso_sub_3, p->u.pty.path);
> > >                       if (rc)
> > >                           goto out;
> > > -                    if (json_object_object_add(jso_sub_1, "path", jso_sub_2)) {
> > > -                        json_object_put(jso_sub_2);
> > > +                    if (json_object_object_add(jso_sub_1, "path", jso_sub_3)) {
> > > +                        json_object_put(jso_sub_3);
> > >                           goto out;
> > >                       }
> > >                   }
> > >
> > > Signed-off-by: Jason Andryuk <jason.andryuk@xxxxxxx>
> > > ---
> > > Here's an alternative approach to workaround removeprefix.
> > Yeah, this version is less obscure about what's going on. Let's go for
> > it.
> >
> > Reviewed-by: Anthony PERARD <anthony.perard@xxxxxxxxxx>
> Thanks.  I'll take this version, and take the liberty of assuming that
> the Release Ack is transferable to whichever solution the maintainers
> prefer in the end.
>
> > >   tools/libs/light/gentypes.py | 14 ++++++++------
> > >   1 file changed, 8 insertions(+), 6 deletions(-)
> > >
> > > diff --git a/tools/libs/light/gentypes.py b/tools/libs/light/gentypes.py
> > > index 006bea170a..0e45c04f49 100644
> > > --- a/tools/libs/light/gentypes.py
> > > +++ b/tools/libs/light/gentypes.py
> > > @@ -377,15 +377,16 @@ def get_default_expr(f, nparent, fexpr):
> > >       return "%s" % fexpr
> > >   
> > >   # For json-c gen_json functions
> > > -def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = 
> > > "jso"):
> > > +def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = 
> > > "jso", depth = 0):
> > >       s = ""
> > >       if parent is None:
> > >           s += "json_object *jso;\n"
> > >           s += "int rc;\n"
> > > -        sub_scope_object = "jso_sub_1"
> > > +        depth = 1
> > >       else:
> > > -        sub_scope_object = "jso_sub_%d" % 
> > > (1+int(scope_object.removeprefix("jso_sub_")))
> > > +        depth += 1
> > We could simply do `depth += 1` regardless of the value of parent, it
> > would have the same effect, since depth start at 0.
> That makes the code even more simple, because it takes out the else.
> The net hunk is:
>
> @@ -377,15 +377,14 @@ def get_default_expr(f, nparent, fexpr):
>       return "%s" % fexpr
>   
>   # For json-c gen_json functions
> -def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = 
> "jso"):
> +def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = 
> "jso", depth = 0):
>       s = ""
>       if parent is None:
>           s += "json_object *jso;\n"
>           s += "int rc;\n"
> -        sub_scope_object = "jso_sub_1"
> -    else:
> -        sub_scope_object = "jso_sub_%d" % 
> (1+int(scope_object.removeprefix("jso_sub_")))
>   
> +    depth += 1
> +    sub_scope_object = "jso_sub_%d" % depth
>       if isinstance(ty, idl.Array):
>           if parent is None:
>               raise Exception("Array type must have a parent")
This works for me, thanks - I even tried it at some point while tracking 
down the jso_sub_2/3 difference from KeyedUnion.  I should have gone 
back to it :)
Regards,
Jason