Attempt to rewrite group-qemu-caps.py in Python

[libvirt PATCH 02/11] scripts: group-qemu-caps: store paths in helper variables

Posted by Ján Tomko 4 years, 5 months ago

Avoid repetition and specifying the path to the header file twice.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
---
 scripts/group-qemu-caps.py | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py
index ec10f24384..bd22dd992a 100755
--- a/scripts/group-qemu-caps.py
+++ b/scripts/group-qemu-caps.py
@@ -133,12 +133,15 @@ args = parser.parse_args()
 
 errs = False
 
-capsflags = load_caps_flags(args.prefix + 'src/qemu/qemu_capabilities.h',
+header_path = args.prefix + 'src/qemu/qemu_capabilities.h'
+source_path = args.prefix + 'src/qemu/qemu_capabilities.c'
+
+capsflags = load_caps_flags(header_path,
                             r'virQEMUCapsFlags grouping marker',
                             r'QEMU_CAPS_LAST \/\* this must')
 
 if not regroup_caps(args.check,
-                    args.prefix + 'src/qemu/qemu_capabilities.c',
+                    source_path,
                     r'virQEMUCaps grouping marker',
                     r'\);',
                     0,
@@ -147,7 +150,7 @@ if not regroup_caps(args.check,
     errs = True
 
 if not regroup_caps(args.check,
-                    args.prefix + 'src/qemu/qemu_capabilities.h',
+                    header_path,
                     r'virQEMUCapsFlags grouping marker',
                     r'QEMU_CAPS_LAST \/\* this must',
                     1,
-- 
2.31.1

Re: [libvirt PATCH 02/11] scripts: group-qemu-caps: store paths in helper variables

Posted by Pavel Hrdina 4 years, 5 months ago

On Tue, Aug 24, 2021 at 04:25:05PM +0200, Ján Tomko wrote:
> Avoid repetition and specifying the path to the header file twice.
> 
> Signed-off-by: Ján Tomko <jtomko@redhat.com>
> ---
>  scripts/group-qemu-caps.py | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py
> index ec10f24384..bd22dd992a 100755
> --- a/scripts/group-qemu-caps.py
> +++ b/scripts/group-qemu-caps.py
> @@ -133,12 +133,15 @@ args = parser.parse_args()
>  
>  errs = False
>  
> -capsflags = load_caps_flags(args.prefix + 'src/qemu/qemu_capabilities.h',
> +header_path = args.prefix + 'src/qemu/qemu_capabilities.h'
> +source_path = args.prefix + 'src/qemu/qemu_capabilities.c'

I know that it's preexisting but this is not recommended way how to
concatenate two or more strings in python.

I would use

    os.path.join(args.prefix, 'src/qemu/qemu_capabilities.h')

instead, or even better:

    os.path.join(args.prefix, 'src', 'qemu', 'qemu_capabilities.h')

> +capsflags = load_caps_flags(header_path,
>                              r'virQEMUCapsFlags grouping marker',
>                              r'QEMU_CAPS_LAST \/\* this must')
>  
>  if not regroup_caps(args.check,
> -                    args.prefix + 'src/qemu/qemu_capabilities.c',
> +                    source_path,
>                      r'virQEMUCaps grouping marker',
>                      r'\);',
>                      0,
> @@ -147,7 +150,7 @@ if not regroup_caps(args.check,
>      errs = True
>  
>  if not regroup_caps(args.check,
> -                    args.prefix + 'src/qemu/qemu_capabilities.h',
> +                    header_path,
>                      r'virQEMUCapsFlags grouping marker',
>                      r'QEMU_CAPS_LAST \/\* this must',
>                      1,
> -- 
> 2.31.1
>

Re: [libvirt PATCH 02/11] scripts: group-qemu-caps: store paths in helper variables

Posted by Michal Prívozník 4 years, 5 months ago

On 8/25/21 2:19 PM, Pavel Hrdina wrote:
> On Tue, Aug 24, 2021 at 04:25:05PM +0200, Ján Tomko wrote:
>> Avoid repetition and specifying the path to the header file twice.
>>
>> Signed-off-by: Ján Tomko <jtomko@redhat.com>
>> ---
>>  scripts/group-qemu-caps.py | 9 ++++++---
>>  1 file changed, 6 insertions(+), 3 deletions(-)
>>
>> diff --git a/scripts/group-qemu-caps.py b/scripts/group-qemu-caps.py
>> index ec10f24384..bd22dd992a 100755
>> --- a/scripts/group-qemu-caps.py
>> +++ b/scripts/group-qemu-caps.py
>> @@ -133,12 +133,15 @@ args = parser.parse_args()
>>  
>>  errs = False
>>  
>> -capsflags = load_caps_flags(args.prefix + 'src/qemu/qemu_capabilities.h',
>> +header_path = args.prefix + 'src/qemu/qemu_capabilities.h'
>> +source_path = args.prefix + 'src/qemu/qemu_capabilities.c'
> 
> I know that it's preexisting but this is not recommended way how to
> concatenate two or more strings in python.
> 
> I would use
> 
>     os.path.join(args.prefix, 'src/qemu/qemu_capabilities.h')
> 
> instead, or even better:
> 
>     os.path.join(args.prefix, 'src', 'qemu', 'qemu_capabilities.h')

(another non-pythonist here)

Is it so that the path is constructed correctly on OSes which use
backslash instead of forward slash?

Michal