All of lore.kernel.org
 help / color / mirror / Atom feed
* [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
@ 2025-10-16 11:22 Jan Beulich
  2025-10-16 11:45 ` Andrew Cooper
                   ` (2 more replies)
  0 siblings, 3 replies; 12+ messages in thread
From: Jan Beulich @ 2025-10-16 11:22 UTC (permalink / raw)
  To: xen-devel@lists.xenproject.org
  Cc: Anthony PERARD, Juergen Gross, Oleksii Kurochko

removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
always going to be there anyway, switch to a less specific but more
compatible construct.

Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
Signed-off-by: Jan Beulich <jbeulich@suse.com>
---
Sadly this is only the tip of the iceberg. Some minimum version of the
json-c library is apparently needed for the toolstack to build, but no
minimum version is being checked for.

--- a/tools/libs/light/gentypes.py
+++ b/tools/libs/light/gentypes.py
@@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
         s += "int rc;\n"
         sub_scope_object = "jso_sub_1"
     else:
-        sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
+        sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
 
     if isinstance(ty, idl.Array):
         if parent is None:


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
  2025-10-16 11:22 [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Jan Beulich
@ 2025-10-16 11:45 ` Andrew Cooper
  2025-10-16 11:51   ` Jan Beulich
  2025-10-16 15:28 ` [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Oleksii Kurochko
  2025-10-21 14:51 ` Anthony PERARD
  2 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2025-10-16 11:45 UTC (permalink / raw)
  To: Jan Beulich, xen-devel@lists.xenproject.org
  Cc: Anthony PERARD, Juergen Gross, Oleksii Kurochko

On 16/10/2025 12:22 pm, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
>
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> ---
> Sadly this is only the tip of the iceberg. Some minimum version of the
> json-c library is apparently needed for the toolstack to build, but no
> minimum version is being checked for.

Well, this is why we have release candidates, and a bug queue.

>
> --- a/tools/libs/light/gentypes.py
> +++ b/tools/libs/light/gentypes.py
> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
>          s += "int rc;\n"
>          sub_scope_object = "jso_sub_1"
>      else:
> -        sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
> +        sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))

This isn't quite an equivalent change.  You want:

def removeprefix(s, p): # Py < 3.9 compat
    if s.startswith(p):
        return s[len(p):]
    return s

at the top level somewhere, and to call removeprefix(scope_object,
"jso_sub_") here.

Sadly, because string is a builtin type, you can't make the 3.9 syntax
work on older versions of python by extending the string type.

~Andrew


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
  2025-10-16 11:45 ` Andrew Cooper
@ 2025-10-16 11:51   ` Jan Beulich
  2025-10-17  2:06     ` [PATCH] tools/libxl: Make gentypes.py compatible with older Python Jason Andryuk
       [not found]     ` <0a78d0993fc44d6cae0a363cc875b6da@DM4PR03MB7015.namprd03.prod.outlook.com>
  0 siblings, 2 replies; 12+ messages in thread
From: Jan Beulich @ 2025-10-16 11:51 UTC (permalink / raw)
  To: Andrew Cooper
  Cc: Anthony PERARD, Juergen Gross, Oleksii Kurochko,
	xen-devel@lists.xenproject.org

On 16.10.2025 13:45, Andrew Cooper wrote:
> On 16/10/2025 12:22 pm, Jan Beulich wrote:
>> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
>> always going to be there anyway, switch to a less specific but more
>> compatible construct.
>>
>> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
>> ---
>> Sadly this is only the tip of the iceberg. Some minimum version of the
>> json-c library is apparently needed for the toolstack to build, but no
>> minimum version is being checked for.
> 
> Well, this is why we have release candidates, and a bug queue.
> 
>>
>> --- a/tools/libs/light/gentypes.py
>> +++ b/tools/libs/light/gentypes.py
>> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
>>          s += "int rc;\n"
>>          sub_scope_object = "jso_sub_1"
>>      else:
>> -        sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
>> +        sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
> 
> This isn't quite an equivalent change.

Yes, as said in the description.

>  You want:
> 
> def removeprefix(s, p): # Py < 3.9 compat
>     if s.startswith(p):
>         return s[len(p):]
>     return s
> 
> at the top level somewhere, and to call removeprefix(scope_object,
> "jso_sub_") here.

I first thought of doing something like this, but then didn't really see why
we would need such. If the prefix is anything else, the original construct
wouldn't work anyway (I expect an exception would be raised unless the incoming
string was itself consisting of only digits).

Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
  2025-10-16 11:22 [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Jan Beulich
  2025-10-16 11:45 ` Andrew Cooper
@ 2025-10-16 15:28 ` Oleksii Kurochko
  2025-10-21 14:51 ` Anthony PERARD
  2 siblings, 0 replies; 12+ messages in thread
From: Oleksii Kurochko @ 2025-10-16 15:28 UTC (permalink / raw)
  To: Jan Beulich, xen-devel@lists.xenproject.org; +Cc: Anthony PERARD, Juergen Gross

[-- Attachment #1: Type: text/plain, Size: 1066 bytes --]


On 10/16/25 1:22 PM, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
>
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich<jbeulich@suse.com>
> ---
> Sadly this is only the tip of the iceberg. Some minimum version of the
> json-c library is apparently needed for the toolstack to build, but no
> minimum version is being checked for.

Release-Acked-by: Oleksii Kurochko<oleksii.kurochko@gmail.com>

Thanks.

~ Oleksii

>
> --- a/tools/libs/light/gentypes.py
> +++ b/tools/libs/light/gentypes.py
> @@ -384,7 +384,7 @@ def libxl_C_type_gen_jso(ty, v, indent =
>           s += "int rc;\n"
>           sub_scope_object = "jso_sub_1"
>       else:
> -        sub_scope_object = "jso_sub_%d" % (1+int(scope_object.removeprefix("jso_sub_")))
> +        sub_scope_object = "jso_sub_%d" % (1+int(scope_object[8:]))
>   
>       if isinstance(ty, idl.Array):
>           if parent is None:

[-- Attachment #2: Type: text/html, Size: 1718 bytes --]

^ permalink raw reply	[flat|nested] 12+ messages in thread

* [PATCH] tools/libxl: Make gentypes.py compatible with older Python
  2025-10-16 11:51   ` Jan Beulich
@ 2025-10-17  2:06     ` Jason Andryuk
  2025-10-17  6:02       ` Jan Beulich
  2025-10-21 16:23       ` Anthony PERARD
       [not found]     ` <0a78d0993fc44d6cae0a363cc875b6da@DM4PR03MB7015.namprd03.prod.outlook.com>
  1 sibling, 2 replies; 12+ messages in thread
From: Jason Andryuk @ 2025-10-17  2:06 UTC (permalink / raw)
  To: jbeulich
  Cc: andrew.cooper3, anthony.perard, jgross, oleksii.kurochko,
	xen-devel, Jason Andryuk

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@amd.com>
---
Here's an alternative approach to workaround removeprefix.

 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
 
+    sub_scope_object = "jso_sub_%d" % depth
     if isinstance(ty, idl.Array):
         if parent is None:
             raise Exception("Array type must have a parent")
@@ -398,7 +399,8 @@ def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = "
         s += "        json_object *%s;\n" % (sub_scope_object)
         # remove some indent, it's over indented at least in one case libxl_vcpu_sched_params_gen_json
         s += libxl_C_type_gen_jso(ty.elem_type, v+"[i]",
-                                   indent + "    ", parent, sub_scope_object)
+                                  indent + "    ", parent, sub_scope_object,
+                                  depth)
         s += "        if (json_object_array_add(%s, %s)) {\n" % (scope_object, sub_scope_object)
         s += "            json_object_put(%s);\n" % (sub_scope_object)
         s += "            goto out;\n"
@@ -417,7 +419,7 @@ def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = "
             (nparent,fexpr) = ty.member(v, f, parent is None)
             s += "case %s:\n" % f.enumname
             if f.type is not None:
-                s += libxl_C_type_gen_jso(f.type, fexpr, indent + "    ", nparent, scope_object)
+                s += libxl_C_type_gen_jso(f.type, fexpr, indent + "    ", nparent, scope_object, depth)
             else:
                 s += "    %s = json_object_new_object();\n" % (scope_object)
                 s += "    if (!%s)\n" % (scope_object)
@@ -433,7 +435,7 @@ def libxl_C_type_gen_jso(ty, v, indent = "    ", parent = None, scope_object = "
             default_expr = get_default_expr(f, nparent, fexpr)
             s += "if (%s) {\n" % default_expr
             s += "    json_object *%s = NULL;\n" % (sub_scope_object)
-            s += libxl_C_type_gen_jso(f.type, fexpr, "    ", nparent, sub_scope_object)
+            s += libxl_C_type_gen_jso(f.type, fexpr, "    ", nparent, sub_scope_object, depth)
             s += libxl_C_type_gen_jso_map_key(f, nparent, "    ", scope_object, sub_scope_object)
 
             s += "}\n"
-- 
2.51.0



^ permalink raw reply related	[flat|nested] 12+ messages in thread

* Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python
  2025-10-17  2:06     ` [PATCH] tools/libxl: Make gentypes.py compatible with older Python Jason Andryuk
@ 2025-10-17  6:02       ` Jan Beulich
  2025-10-21 16:23       ` Anthony PERARD
  1 sibling, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2025-10-17  6:02 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: andrew.cooper3, anthony.perard, jgross, oleksii.kurochko,
	xen-devel

On 17.10.2025 04:06, 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@amd.com>
> ---
> Here's an alternative approach to workaround removeprefix.

Fine with me; it's really Anthony's call. (He'll be back on Monday, aiui.)

Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
  2025-10-16 11:22 [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Jan Beulich
  2025-10-16 11:45 ` Andrew Cooper
  2025-10-16 15:28 ` [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Oleksii Kurochko
@ 2025-10-21 14:51 ` Anthony PERARD
  2025-10-21 15:21   ` Jan Beulich
  2 siblings, 1 reply; 12+ messages in thread
From: Anthony PERARD @ 2025-10-21 14:51 UTC (permalink / raw)
  To: Jan Beulich
  Cc: xen-devel@lists.xenproject.org, Anthony PERARD, Juergen Gross,
	Oleksii Kurochko

On Thu, Oct 16, 2025 at 01:22:30PM +0200, Jan Beulich wrote:
> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
> always going to be there anyway, switch to a less specific but more
> compatible construct.
> 
> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
> Signed-off-by: Jan Beulich <jbeulich@suse.com>

Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9
  2025-10-21 14:51 ` Anthony PERARD
@ 2025-10-21 15:21   ` Jan Beulich
  0 siblings, 0 replies; 12+ messages in thread
From: Jan Beulich @ 2025-10-21 15:21 UTC (permalink / raw)
  To: Anthony PERARD
  Cc: xen-devel@lists.xenproject.org, Anthony PERARD, Juergen Gross,
	Oleksii Kurochko

On 21.10.2025 16:51, Anthony PERARD wrote:
> On Thu, Oct 16, 2025 at 01:22:30PM +0200, Jan Beulich wrote:
>> removeprefix() was added only in 3.9. As long as the "jso_sub_" prefix is
>> always going to be there anyway, switch to a less specific but more
>> compatible construct.
>>
>> Fixes: f6c6f2679d49 ("libxl: libxl__object_to_json() to json-c")
>> Signed-off-by: Jan Beulich <jbeulich@suse.com>
> 
> Acked-by: Anthony PERARD <anthony.perard@vates.tech>

Thanks, but: So you prefer this variant over Jason's?

Jan


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python
       [not found]     ` <0a78d0993fc44d6cae0a363cc875b6da@DM4PR03MB7015.namprd03.prod.outlook.com>
@ 2025-10-21 16:00       ` Andrew Cooper
  0 siblings, 0 replies; 12+ messages in thread
From: Andrew Cooper @ 2025-10-21 16:00 UTC (permalink / raw)
  To: Jason Andryuk, jbeulich@suse.com
  Cc: Andrew Cooper, anthony.perard@vates.tech, jgross@suse.com,
	oleksii.kurochko@gmail.com, xen-devel@lists.xenproject.org

On 17/10/2025 3:06 am, 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@amd.com>

Reviewed-by: Andrew Cooper <andrew.cooper3@citrix.com>

Personally, I think this is the nicest of the options posted.


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python
  2025-10-17  2:06     ` [PATCH] tools/libxl: Make gentypes.py compatible with older Python Jason Andryuk
  2025-10-17  6:02       ` Jan Beulich
@ 2025-10-21 16:23       ` Anthony PERARD
  2025-10-21 16:31         ` Andrew Cooper
  1 sibling, 1 reply; 12+ messages in thread
From: Anthony PERARD @ 2025-10-21 16:23 UTC (permalink / raw)
  To: Jason Andryuk
  Cc: jbeulich, andrew.cooper3, anthony.perard, jgross,
	oleksii.kurochko, xen-devel

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@amd.com>
> ---
> 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@vates.tech>

>  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. We just need to
create a new variable name that is different from the one created by the
callers (`depth=random.randrange(9999, 99999999)` actually works :-D,
but no guaranty of having different values). Anyway, the code is fine as
is, no need to make last minute edit.

Cheers,

-- 
Anthony PERARD


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python
  2025-10-21 16:23       ` Anthony PERARD
@ 2025-10-21 16:31         ` Andrew Cooper
  2025-10-21 17:20           ` Jason Andryuk
  0 siblings, 1 reply; 12+ messages in thread
From: Andrew Cooper @ 2025-10-21 16:31 UTC (permalink / raw)
  To: Anthony PERARD, Jason Andryuk
  Cc: jbeulich, anthony.perard, jgross, oleksii.kurochko, xen-devel

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@amd.com>
>> ---
>> 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@vates.tech>

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")


~Andrew


^ permalink raw reply	[flat|nested] 12+ messages in thread

* Re: [PATCH] tools/libxl: Make gentypes.py compatible with older Python
  2025-10-21 16:31         ` Andrew Cooper
@ 2025-10-21 17:20           ` Jason Andryuk
  0 siblings, 0 replies; 12+ messages in thread
From: Jason Andryuk @ 2025-10-21 17:20 UTC (permalink / raw)
  To: Andrew Cooper, Anthony PERARD
  Cc: jbeulich, anthony.perard, jgross, oleksii.kurochko, xen-devel

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@amd.com>
>>> ---
>>> 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@vates.tech>
> 
> 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


^ permalink raw reply	[flat|nested] 12+ messages in thread

end of thread, other threads:[~2025-10-21 17:21 UTC | newest]

Thread overview: 12+ messages (download: mbox.gz follow: Atom feed
-- links below jump to the message on this page --
2025-10-16 11:22 [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Jan Beulich
2025-10-16 11:45 ` Andrew Cooper
2025-10-16 11:51   ` Jan Beulich
2025-10-17  2:06     ` [PATCH] tools/libxl: Make gentypes.py compatible with older Python Jason Andryuk
2025-10-17  6:02       ` Jan Beulich
2025-10-21 16:23       ` Anthony PERARD
2025-10-21 16:31         ` Andrew Cooper
2025-10-21 17:20           ` Jason Andryuk
     [not found]     ` <0a78d0993fc44d6cae0a363cc875b6da@DM4PR03MB7015.namprd03.prod.outlook.com>
2025-10-21 16:00       ` Andrew Cooper
2025-10-16 15:28 ` [PATCH for-4.21] libxl: make gentypes.py compatible with Python older than 3.9 Oleksii Kurochko
2025-10-21 14:51 ` Anthony PERARD
2025-10-21 15:21   ` Jan Beulich

This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.