Re: [PATCH v4 5/6] scripts/modules/module_block: Use Python 3 interpreter & add pseudo-main

From: John Snow <jsnow@redhat.com>
To: "Philippe Mathieu-Daudé" <philmd@redhat.com>, qemu-devel@nongnu.org
Cc: "Markus Armbruster" <armbru@redhat.com>,
	"Paolo Bonzini" <pbonzini@redhat.com>,
	qemu-block@nongnu.org, qemu-trivial@nongnu.org,
	"Cleber Rosa" <crosa@redhat.com>,
	kvm@vger.kernel.org, "Eduardo Habkost" <ehabkost@redhat.com>,
	"Alex Bennée" <alex.bennee@linaro.org>,
	"Marcelo Tosatti" <mtosatti@redhat.com>,
	"Stefan Hajnoczi" <stefanha@redhat.com>,
	"Fam Zheng" <fam@euphon.net>
Subject: Re: [PATCH v4 5/6] scripts/modules/module_block: Use Python 3 interpreter & add pseudo-main
Date: Wed, 13 May 2020 17:04:59 -0400	[thread overview]
Message-ID: <fdae1c32-5f22-543d-6bef-412cde4d0b8a@redhat.com> (raw)
In-Reply-To: <20200512103238.7078-6-philmd@redhat.com>

On 5/12/20 6:32 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <philmd@redhat.com>
> ---
>  scripts/modules/module_block.py | 31 ++++++++++++++++---------------
>  1 file changed, 16 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/modules/module_block.py b/scripts/modules/module_block.py
> index f23191fac1..2e7021b952 100644
> --- a/scripts/modules/module_block.py
> +++ b/scripts/modules/module_block.py
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/env python3
>  #
>  # Module information generator
>  #
> @@ -10,7 +10,6 @@
>  # This work is licensed under the terms of the GNU GPL, version 2.
>  # See the COPYING file in the top-level directory.
>  
> -import sys
>  import os
>  
>  def get_string_struct(line):
> @@ -80,19 +79,21 @@ def print_bottom(fheader):
>  #endif
>  ''')
>  
> -# First argument: output file
> -# All other arguments: modules source files (.c)
> -output_file = sys.argv[1]
> -with open(output_file, 'w') as fheader:
> -    print_top(fheader)
> +if __name__ == '__main__':
> +    import sys

You can keep the imports at the top of the file.

If you want to split apart the code such that the core import doesn't
import it, you can create a "core module" containing the classes and
routines, and a separate script entrypoint, which imports arg parsers,
sys.argv, etc.

For this, for now, it's okay to just leave it at the top of the file.

> +    # First argument: output file
> +    # All other arguments: modules source files (.c)
> +    output_file = sys.argv[1]
> +    with open(output_file, 'w') as fheader:
> +        print_top(fheader)
>  
> -    for filename in sys.argv[2:]:
> -        if os.path.isfile(filename):
> -            process_file(fheader, filename)
> -        else:
> -            print("File " + filename + " does not exist.", file=sys.stderr)
> -            sys.exit(1)
> +        for filename in sys.argv[2:]:
> +            if os.path.isfile(filename):
> +                process_file(fheader, filename)
> +            else:
> +                print("File " + filename + " does not exist.", file=sys.stderr)
> +                sys.exit(1)
>  
> -    print_bottom(fheader)
> +        print_bottom(fheader)
>  
> -sys.exit(0)
> +    sys.exit(0)
> 

But, well. It's nitpicky and I'm not sure it matters just yet. It might,
as we start to expand pylint to more places, but we're not being
rigorous about that just yet. So either way, I know this works:

Reviewed-by: John Snow <jsnow@redhat.com>