python3 script header and rhel8

python3 script header and rhel8

[Vladimir Sementsov-Ogievskiy]

Hi all!

Hmm Qemu dropped support for python2, and anyway python2 is not supported now at all.

Still, we have a lot of "#!/usr/bin/env python" headings in our scripts, which is
unsupported by rhel8 by default. So, for example, because of such line in
tests/qemu-iotests/nbd-fault-injector.py, iotest 277 fails.

Of course, it's simple to set python in rhel8 to be python3... But should we fix
all the headings to be "#!/usr/bin/env python3"? Or what is the correct heading
for scripts in a new python3 world?


# git grep -l '^#!/usr/bin/env python$'
scripts/analyse-9p-simpletrace.py
scripts/analyse-locks-simpletrace.py
scripts/decodetree.py
scripts/device-crash-test
scripts/kvm/kvm_flightrecorder
scripts/qapi-gen.py
scripts/render_block_graph.py
scripts/replay-dump.py
scripts/simpletrace.py
scripts/tracetool.py
scripts/tracetool/__init__.py
scripts/tracetool/backend/__init__.py
scripts/tracetool/backend/dtrace.py
scripts/tracetool/backend/ftrace.py
scripts/tracetool/backend/log.py
scripts/tracetool/backend/simple.py
scripts/tracetool/backend/syslog.py
scripts/tracetool/backend/ust.py
scripts/tracetool/format/__init__.py
scripts/tracetool/format/c.py
scripts/tracetool/format/d.py
scripts/tracetool/format/h.py
scripts/tracetool/format/log_stap.py
scripts/tracetool/format/simpletrace_stap.py
scripts/tracetool/format/stap.py
scripts/tracetool/format/tcg_h.py
scripts/tracetool/format/tcg_helper_c.py
scripts/tracetool/format/tcg_helper_h.py
scripts/tracetool/format/tcg_helper_wrapper_h.py
scripts/tracetool/format/ust_events_c.py
scripts/tracetool/format/ust_events_h.py
scripts/tracetool/transform.py
scripts/tracetool/vcpu.py
tests/acceptance/virtio_seg_max_adjust.py
tests/acceptance/x86_cpu_model_versions.py
tests/docker/travis.py
tests/qapi-schema/test-qapi.py
tests/qemu-iotests/030
tests/qemu-iotests/040
tests/qemu-iotests/041
tests/qemu-iotests/044
tests/qemu-iotests/045
tests/qemu-iotests/055
tests/qemu-iotests/056
tests/qemu-iotests/057
tests/qemu-iotests/065
tests/qemu-iotests/093
tests/qemu-iotests/096
tests/qemu-iotests/118
tests/qemu-iotests/124
tests/qemu-iotests/129
tests/qemu-iotests/132
tests/qemu-iotests/136
tests/qemu-iotests/139
tests/qemu-iotests/147
tests/qemu-iotests/148
tests/qemu-iotests/149
tests/qemu-iotests/151
tests/qemu-iotests/152
tests/qemu-iotests/155
tests/qemu-iotests/163
tests/qemu-iotests/165
tests/qemu-iotests/169
tests/qemu-iotests/194
tests/qemu-iotests/196
tests/qemu-iotests/199
tests/qemu-iotests/202
tests/qemu-iotests/203
tests/qemu-iotests/205
tests/qemu-iotests/206
tests/qemu-iotests/207
tests/qemu-iotests/208
tests/qemu-iotests/209
tests/qemu-iotests/210
tests/qemu-iotests/211
tests/qemu-iotests/212
tests/qemu-iotests/213
tests/qemu-iotests/216
tests/qemu-iotests/218
tests/qemu-iotests/219
tests/qemu-iotests/222
tests/qemu-iotests/224
tests/qemu-iotests/228
tests/qemu-iotests/234
tests/qemu-iotests/235
tests/qemu-iotests/236
tests/qemu-iotests/237
tests/qemu-iotests/238
tests/qemu-iotests/242
tests/qemu-iotests/245
tests/qemu-iotests/246
tests/qemu-iotests/248
tests/qemu-iotests/254
tests/qemu-iotests/255
tests/qemu-iotests/256
tests/qemu-iotests/257
tests/qemu-iotests/258
tests/qemu-iotests/260
tests/qemu-iotests/262
tests/qemu-iotests/264
tests/qemu-iotests/266
tests/qemu-iotests/277
tests/qemu-iotests/280
tests/qemu-iotests/nbd-fault-injector.py
tests/qemu-iotests/qcow2.py
tests/qemu-iotests/qed.py
tests/vm/basevm.py
tests/vm/centos
tests/vm/fedora
tests/vm/freebsd
tests/vm/netbsd
tests/vm/openbsd
tests/vm/ubuntu.i386



-- 
Best regards,
Vladimir

Re: python3 script header and rhel8

[Philippe Mathieu-Daudé]

On 1/28/20 4:44 PM, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Hmm Qemu dropped support for python2, and anyway python2 is not supported now at all.
> 
> Still, we have a lot of "#!/usr/bin/env python" headings in our scripts, which is
> unsupported by rhel8 by default. So, for example, because of such line in
> tests/qemu-iotests/nbd-fault-injector.py, iotest 277 fails.
> 
> Of course, it's simple to set python in rhel8 to be python3... But should we fix
> all the headings to be "#!/usr/bin/env python3"? Or what is the correct heading
> for scripts in a new python3 world?

I'd go for a codebase wide cleanup for once, and use python3. That way 
we are ready for when Python4 goes out, we won't suffer the same pain. 
That also mean we'll have to explicitly support Python4.

Eventually add modify scripts/checkpatch.pl to refuse commits 
introducing unversioned python back.

Re: python3 script header and rhel8

[Daniel P. Berrangé]

On Tue, Jan 28, 2020 at 03:44:04PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> Hi all!
> 
> Hmm Qemu dropped support for python2, and anyway python2 is not supported now at all.
> 
> Still, we have a lot of "#!/usr/bin/env python" headings in our scripts, which is
> unsupported by rhel8 by default. So, for example, because of such line in
> tests/qemu-iotests/nbd-fault-injector.py, iotest 277 fails.
> 
> Of course, it's simple to set python in rhel8 to be python3... But should we fix
> all the headings to be "#!/usr/bin/env python3"? Or what is the correct heading
> for scripts in a new python3 world?

Using "#!/usr/bin/env python3" is probably our best choice, as it
makes it absolutely clear we're not happy with a python2 impl.

> # git grep -l '^#!/usr/bin/env python$'
> scripts/analyse-9p-simpletrace.py
> scripts/analyse-locks-simpletrace.py
> scripts/decodetree.py
> scripts/device-crash-test
> scripts/kvm/kvm_flightrecorder
> scripts/qapi-gen.py
> scripts/render_block_graph.py
> scripts/replay-dump.py
> scripts/simpletrace.py
> scripts/tracetool.py
> scripts/tracetool/__init__.py
> scripts/tracetool/backend/__init__.py
> scripts/tracetool/backend/dtrace.py
> scripts/tracetool/backend/ftrace.py
> scripts/tracetool/backend/log.py
> scripts/tracetool/backend/simple.py
> scripts/tracetool/backend/syslog.py
> scripts/tracetool/backend/ust.py
> scripts/tracetool/format/__init__.py
> scripts/tracetool/format/c.py
> scripts/tracetool/format/d.py
> scripts/tracetool/format/h.py
> scripts/tracetool/format/log_stap.py
> scripts/tracetool/format/simpletrace_stap.py
> scripts/tracetool/format/stap.py
> scripts/tracetool/format/tcg_h.py
> scripts/tracetool/format/tcg_helper_c.py
> scripts/tracetool/format/tcg_helper_h.py
> scripts/tracetool/format/tcg_helper_wrapper_h.py
> scripts/tracetool/format/ust_events_c.py
> scripts/tracetool/format/ust_events_h.py
> scripts/tracetool/transform.py
> scripts/tracetool/vcpu.py

IMHO the '#!/usr/bin/env' header should only be added to files
which are executable scripts - ie a main() method. IOW, it
ought to be removed from all these tracetool modules.

> tests/acceptance/virtio_seg_max_adjust.py
> tests/acceptance/x86_cpu_model_versions.py
> tests/docker/travis.py
> tests/qapi-schema/test-qapi.py
> tests/qemu-iotests/030
> tests/qemu-iotests/040
> tests/qemu-iotests/041
> tests/qemu-iotests/044
> tests/qemu-iotests/045
> tests/qemu-iotests/055
> tests/qemu-iotests/056
> tests/qemu-iotests/057
> tests/qemu-iotests/065
> tests/qemu-iotests/093
> tests/qemu-iotests/096
> tests/qemu-iotests/118
> tests/qemu-iotests/124
> tests/qemu-iotests/129
> tests/qemu-iotests/132
> tests/qemu-iotests/136
> tests/qemu-iotests/139
> tests/qemu-iotests/147
> tests/qemu-iotests/148
> tests/qemu-iotests/149
> tests/qemu-iotests/151
> tests/qemu-iotests/152
> tests/qemu-iotests/155
> tests/qemu-iotests/163
> tests/qemu-iotests/165
> tests/qemu-iotests/169
> tests/qemu-iotests/194
> tests/qemu-iotests/196
> tests/qemu-iotests/199
> tests/qemu-iotests/202
> tests/qemu-iotests/203
> tests/qemu-iotests/205
> tests/qemu-iotests/206
> tests/qemu-iotests/207
> tests/qemu-iotests/208
> tests/qemu-iotests/209
> tests/qemu-iotests/210
> tests/qemu-iotests/211
> tests/qemu-iotests/212
> tests/qemu-iotests/213
> tests/qemu-iotests/216
> tests/qemu-iotests/218
> tests/qemu-iotests/219
> tests/qemu-iotests/222
> tests/qemu-iotests/224
> tests/qemu-iotests/228
> tests/qemu-iotests/234
> tests/qemu-iotests/235
> tests/qemu-iotests/236
> tests/qemu-iotests/237
> tests/qemu-iotests/238
> tests/qemu-iotests/242
> tests/qemu-iotests/245
> tests/qemu-iotests/246
> tests/qemu-iotests/248
> tests/qemu-iotests/254
> tests/qemu-iotests/255
> tests/qemu-iotests/256
> tests/qemu-iotests/257
> tests/qemu-iotests/258
> tests/qemu-iotests/260
> tests/qemu-iotests/262
> tests/qemu-iotests/264
> tests/qemu-iotests/266
> tests/qemu-iotests/277
> tests/qemu-iotests/280
> tests/qemu-iotests/nbd-fault-injector.py
> tests/qemu-iotests/qcow2.py
> tests/qemu-iotests/qed.py
> tests/vm/basevm.py
> tests/vm/centos
> tests/vm/fedora
> tests/vm/freebsd
> tests/vm/netbsd
> tests/vm/openbsd
> tests/vm/ubuntu.i386

Regards,
Daniel
-- 
|: https://berrange.com      -o-    https://www.flickr.com/photos/dberrange :|
|: https://libvirt.org         -o-            https://fstop138.berrange.com :|
|: https://entangle-photo.org    -o-    https://www.instagram.com/dberrange :|

Re: python3 script header and rhel8

[John Snow]

On 1/28/20 11:07 AM, Daniel P. Berrangé wrote:
> On Tue, Jan 28, 2020 at 03:44:04PM +0000, Vladimir Sementsov-Ogievskiy wrote:
>> Hi all!
>>
>> Hmm Qemu dropped support for python2, and anyway python2 is not supported now at all.
>>
>> Still, we have a lot of "#!/usr/bin/env python" headings in our scripts, which is
>> unsupported by rhel8 by default. So, for example, because of such line in
>> tests/qemu-iotests/nbd-fault-injector.py, iotest 277 fails.
>>
>> Of course, it's simple to set python in rhel8 to be python3... But should we fix
>> all the headings to be "#!/usr/bin/env python3"? Or what is the correct heading
>> for scripts in a new python3 world?
> 
> Using "#!/usr/bin/env python3" is probably our best choice, as it
> makes it absolutely clear we're not happy with a python2 impl.
> 

On using 'env' or not: for any script anticipated to be executed
directly from the command line, a hardcoded path will inhibit idiomatic
virtual environment usage, so 'env' is preferred.

On python vs python2 vs python3:

https://www.python.org/dev/peps/pep-0394/

PEP 0394 states that 'python3' should be available and that 'python' is
optional.

That answers that, in my opinion. Let's make `#!/usr/bin/env python3`
ubiquitous.

--js

Re: python3 script header and rhel8

[Stefan Hajnoczi]

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

On Tue, Jan 28, 2020 at 04:07:47PM +0000, Daniel P. Berrangé wrote:
> On Tue, Jan 28, 2020 at 03:44:04PM +0000, Vladimir Sementsov-Ogievskiy wrote:
> > scripts/tracetool/__init__.py
> > scripts/tracetool/backend/__init__.py
> > scripts/tracetool/backend/dtrace.py
> > scripts/tracetool/backend/ftrace.py
> > scripts/tracetool/backend/log.py
> > scripts/tracetool/backend/simple.py
> > scripts/tracetool/backend/syslog.py
> > scripts/tracetool/backend/ust.py
> > scripts/tracetool/format/__init__.py
> > scripts/tracetool/format/c.py
> > scripts/tracetool/format/d.py
> > scripts/tracetool/format/h.py
> > scripts/tracetool/format/log_stap.py
> > scripts/tracetool/format/simpletrace_stap.py
> > scripts/tracetool/format/stap.py
> > scripts/tracetool/format/tcg_h.py
> > scripts/tracetool/format/tcg_helper_c.py
> > scripts/tracetool/format/tcg_helper_h.py
> > scripts/tracetool/format/tcg_helper_wrapper_h.py
> > scripts/tracetool/format/ust_events_c.py
> > scripts/tracetool/format/ust_events_h.py
> > scripts/tracetool/transform.py
> > scripts/tracetool/vcpu.py
> 
> IMHO the '#!/usr/bin/env' header should only be added to files
> which are executable scripts - ie a main() method. IOW, it
> ought to be removed from all these tracetool modules.

I agree.

Typically any Python source file that does not have "if __name__ ==
'__main__':" is just a module and not an executable script.  That is a
good heuristic for identifying files that shouldn't have the hashbang
header, but manual review is still necessary in case one of the files is
doing something unusual.

Stefan

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 488 bytes --]