[PATCH 0/4] scripts: More Python fixes

[PATCH 0/4] scripts: More Python fixes

[Philippe Mathieu-Daudé]

Trivial Python3 fixes, again...

Philippe Mathieu-Daudé (4):
  MAINTAINERS: Cover the GDB Python scripts in the gdbstub section
  scripts/qemugdb: Remove shebang header
  scripts/qmp: Use Python 3 interpreter
  scripts/qmp: Fix QEMU Python scripts path

 MAINTAINERS                  | 1 +
 scripts/qemugdb/__init__.py  | 3 +--
 scripts/qemugdb/aio.py       | 3 +--
 scripts/qemugdb/coroutine.py | 3 +--
 scripts/qemugdb/mtree.py     | 4 +---
 scripts/qemugdb/tcg.py       | 1 -
 scripts/qmp/qmp              | 4 +++-
 scripts/qmp/qom-fuse         | 4 +++-
 scripts/qmp/qom-get          | 6 ++++--
 scripts/qmp/qom-list         | 6 ++++--
 scripts/qmp/qom-set          | 6 ++++--
 scripts/qmp/qom-tree         | 6 ++++--
 12 files changed, 27 insertions(+), 20 deletions(-)

-- 
2.21.1

[PATCH 1/4] MAINTAINERS: Cover the GDB Python scripts in the gdbstub section

[Philippe Mathieu-Daudé]

Keep an eye on these "same same, but different" files.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
Cc: "Alex Bennée" <alex.bennee@linaro.org>
---
 MAINTAINERS | 1 +
 1 file changed, 1 insertion(+)

diff --git a/MAINTAINERS b/MAINTAINERS
index 8cbc1fac2b..7a7f2b9c31 100644
--- a/MAINTAINERS
+++ b/MAINTAINERS
@@ -2083,6 +2083,7 @@ R: Philippe Mathieu-Daudé <philmd@redhat.com>
 S: Maintained
 F: gdbstub*
 F: gdb-xml/
+F: scripts/qemugdb/
 
 Memory API
 M: Paolo Bonzini <pbonzini@redhat.com>
-- 
2.21.1

[PATCH 2/4] scripts/qemugdb: Remove shebang header

[Philippe Mathieu-Daudé]

These scripts are loaded as plugin by GDB (and they don't
have any __main__ entry point). Remove the shebang header.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 scripts/qemugdb/__init__.py  | 3 +--
 scripts/qemugdb/aio.py       | 3 +--
 scripts/qemugdb/coroutine.py | 3 +--
 scripts/qemugdb/mtree.py     | 4 +---
 scripts/qemugdb/tcg.py       | 1 -
 5 files changed, 4 insertions(+), 10 deletions(-)

diff --git a/scripts/qemugdb/__init__.py b/scripts/qemugdb/__init__.py
index 969f552b26..da8ff612e5 100644
--- a/scripts/qemugdb/__init__.py
+++ b/scripts/qemugdb/__init__.py
@@ -1,5 +1,4 @@
-#!/usr/bin/python
-
+#
 # GDB debugging support
 #
 # Copyright (c) 2015 Linaro Ltd
diff --git a/scripts/qemugdb/aio.py b/scripts/qemugdb/aio.py
index 2ba00c4444..d7c1ba0c28 100644
--- a/scripts/qemugdb/aio.py
+++ b/scripts/qemugdb/aio.py
@@ -1,5 +1,4 @@
-#!/usr/bin/python
-
+#
 # GDB debugging support: aio/iohandler debug
 #
 # Copyright (c) 2015 Red Hat, Inc.
diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py
index 41e079d0e2..db61389022 100644
--- a/scripts/qemugdb/coroutine.py
+++ b/scripts/qemugdb/coroutine.py
@@ -1,5 +1,4 @@
-#!/usr/bin/python
-
+#
 # GDB debugging support
 #
 # Copyright 2012 Red Hat, Inc. and/or its affiliates
diff --git a/scripts/qemugdb/mtree.py b/scripts/qemugdb/mtree.py
index 3030a60d3f..8fe42c3c12 100644
--- a/scripts/qemugdb/mtree.py
+++ b/scripts/qemugdb/mtree.py
@@ -1,5 +1,4 @@
-#!/usr/bin/python
-
+#
 # GDB debugging support
 #
 # Copyright 2012 Red Hat, Inc. and/or its affiliates
@@ -84,4 +83,3 @@ def print_item(self, ptr, offset = gdb.Value(0), level = 0):
         while not isnull(subregion):
             self.print_item(subregion, addr, level)
             subregion = subregion['subregions_link']['tqe_next']
-
diff --git a/scripts/qemugdb/tcg.py b/scripts/qemugdb/tcg.py
index 18880fc9a7..16c03c06a9 100644
--- a/scripts/qemugdb/tcg.py
+++ b/scripts/qemugdb/tcg.py
@@ -1,4 +1,3 @@
-#!/usr/bin/python
 # -*- coding: utf-8 -*-
 #
 # GDB debugging support, TCG status
-- 
2.21.1

[PATCH 3/4] scripts/qmp: Use Python 3 interpreter

[Philippe Mathieu-Daudé]

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 scripts/qmp/qom-get  | 2 +-
 scripts/qmp/qom-list | 2 +-
 scripts/qmp/qom-set  | 2 +-
 scripts/qmp/qom-tree | 2 +-
 4 files changed, 4 insertions(+), 4 deletions(-)

diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
index 007b4cd442..72ccd79330 100755
--- a/scripts/qmp/qom-get
+++ b/scripts/qmp/qom-get
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 ##
 # QEMU Object Model test tools
 #
diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
index 03bda3446b..5b8f9fd855 100755
--- a/scripts/qmp/qom-list
+++ b/scripts/qmp/qom-list
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 ##
 # QEMU Object Model test tools
 #
diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
index c37fe78b00..b475e397fc 100755
--- a/scripts/qmp/qom-set
+++ b/scripts/qmp/qom-set
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 ##
 # QEMU Object Model test tools
 #
diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
index 1c8acf61e7..86233fa211 100755
--- a/scripts/qmp/qom-tree
+++ b/scripts/qmp/qom-tree
@@ -1,4 +1,4 @@
-#!/usr/bin/python
+#!/usr/bin/python3
 ##
 # QEMU Object Model test tools
 #
-- 
2.21.1

[PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[Philippe Mathieu-Daudé]

QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
Python module structure"). Use the same sys.path modification used
in the referenced commit to be able to use these scripts again.

Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
---
 scripts/qmp/qmp      | 4 +++-
 scripts/qmp/qom-fuse | 4 +++-
 scripts/qmp/qom-get  | 4 +++-
 scripts/qmp/qom-list | 4 +++-
 scripts/qmp/qom-set  | 4 +++-
 scripts/qmp/qom-tree | 4 +++-
 6 files changed, 18 insertions(+), 6 deletions(-)

diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
index 0625fc2aba..8e52e4a54d 100755
--- a/scripts/qmp/qmp
+++ b/scripts/qmp/qmp
@@ -11,7 +11,9 @@
 # See the COPYING file in the top-level directory.
 
 import sys, os
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 def print_response(rsp, prefix=[]):
     if type(rsp) == list:
diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
index 6bada2c33d..5fa6b3bf64 100755
--- a/scripts/qmp/qom-fuse
+++ b/scripts/qmp/qom-fuse
@@ -15,7 +15,9 @@ import fuse, stat
 from fuse import Fuse
 import os, posix
 from errno import *
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 fuse.fuse_python_api = (0, 2)
 
diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
index 72ccd79330..59090069dc 100755
--- a/scripts/qmp/qom-get
+++ b/scripts/qmp/qom-get
@@ -13,7 +13,9 @@
 
 import sys
 import os
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 cmd, args = sys.argv[0], sys.argv[1:]
 socket_path = None
diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
index 5b8f9fd855..c5d0c8127d 100755
--- a/scripts/qmp/qom-list
+++ b/scripts/qmp/qom-list
@@ -13,7 +13,9 @@
 
 import sys
 import os
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 cmd, args = sys.argv[0], sys.argv[1:]
 socket_path = None
diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
index b475e397fc..e9d7e0b054 100755
--- a/scripts/qmp/qom-set
+++ b/scripts/qmp/qom-set
@@ -13,7 +13,9 @@
 
 import sys
 import os
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 cmd, args = sys.argv[0], sys.argv[1:]
 socket_path = None
diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
index 86233fa211..d96b17256e 100755
--- a/scripts/qmp/qom-tree
+++ b/scripts/qmp/qom-tree
@@ -15,7 +15,9 @@
 
 import sys
 import os
-from qmp import QEMUMonitorProtocol
+
+sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
+from qemu.qmp import QEMUMonitorProtocol
 
 cmd, args = sys.argv[0], sys.argv[1:]
 socket_path = None
-- 
2.21.1

Re: [PATCH 2/4] scripts/qemugdb: Remove shebang header

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> These scripts are loaded as plugin by GDB (and they don't
> have any __main__ entry point). Remove the shebang header.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  scripts/qemugdb/__init__.py  | 3 +--
>  scripts/qemugdb/aio.py       | 3 +--
>  scripts/qemugdb/coroutine.py | 3 +--
>  scripts/qemugdb/mtree.py     | 4 +---
>  scripts/qemugdb/tcg.py       | 1 -
>  5 files changed, 4 insertions(+), 10 deletions(-)
>
> diff --git a/scripts/qemugdb/__init__.py b/scripts/qemugdb/__init__.py
> index 969f552b26..da8ff612e5 100644
> --- a/scripts/qemugdb/__init__.py
> +++ b/scripts/qemugdb/__init__.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/python
> -
> +#
>  # GDB debugging support
>  #
>  # Copyright (c) 2015 Linaro Ltd
> diff --git a/scripts/qemugdb/aio.py b/scripts/qemugdb/aio.py
> index 2ba00c4444..d7c1ba0c28 100644
> --- a/scripts/qemugdb/aio.py
> +++ b/scripts/qemugdb/aio.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/python
> -
> +#
>  # GDB debugging support: aio/iohandler debug
>  #
>  # Copyright (c) 2015 Red Hat, Inc.
> diff --git a/scripts/qemugdb/coroutine.py b/scripts/qemugdb/coroutine.py
> index 41e079d0e2..db61389022 100644
> --- a/scripts/qemugdb/coroutine.py
> +++ b/scripts/qemugdb/coroutine.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/python
> -
> +#
>  # GDB debugging support
>  #
>  # Copyright 2012 Red Hat, Inc. and/or its affiliates
> diff --git a/scripts/qemugdb/mtree.py b/scripts/qemugdb/mtree.py
> index 3030a60d3f..8fe42c3c12 100644
> --- a/scripts/qemugdb/mtree.py
> +++ b/scripts/qemugdb/mtree.py
> @@ -1,5 +1,4 @@
> -#!/usr/bin/python
> -
> +#
>  # GDB debugging support
>  #
>  # Copyright 2012 Red Hat, Inc. and/or its affiliates
> @@ -84,4 +83,3 @@ def print_item(self, ptr, offset = gdb.Value(0), level = 0):
>          while not isnull(subregion):
>              self.print_item(subregion, addr, level)
>              subregion = subregion['subregions_link']['tqe_next']
> -
> diff --git a/scripts/qemugdb/tcg.py b/scripts/qemugdb/tcg.py
> index 18880fc9a7..16c03c06a9 100644
> --- a/scripts/qemugdb/tcg.py
> +++ b/scripts/qemugdb/tcg.py
> @@ -1,4 +1,3 @@
> -#!/usr/bin/python
>  # -*- coding: utf-8 -*-
>  #
>  # GDB debugging support, TCG status


-- 
Alex Bennée

Re: [PATCH 1/4] MAINTAINERS: Cover the GDB Python scripts in the gdbstub section

[Alex Bennée]

Philippe Mathieu-Daudé <f4bug@amsat.org> writes:

> Keep an eye on these "same same, but different" files.
>
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
> Cc: "Alex Bennée" <alex.bennee@linaro.org>

Acked-by: Alex Bennée <alex.bennee@linaro.org>

> ---
>  MAINTAINERS | 1 +
>  1 file changed, 1 insertion(+)
>
> diff --git a/MAINTAINERS b/MAINTAINERS
> index 8cbc1fac2b..7a7f2b9c31 100644
> --- a/MAINTAINERS
> +++ b/MAINTAINERS
> @@ -2083,6 +2083,7 @@ R: Philippe Mathieu-Daudé <philmd@redhat.com>
>  S: Maintained
>  F: gdbstub*
>  F: gdb-xml/
> +F: scripts/qemugdb/
>  
>  Memory API
>  M: Paolo Bonzini <pbonzini@redhat.com>


-- 
Alex Bennée

Re: [PATCH 3/4] scripts/qmp: Use Python 3 interpreter

[John Snow]

On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  scripts/qmp/qom-get  | 2 +-
>  scripts/qmp/qom-list | 2 +-
>  scripts/qmp/qom-set  | 2 +-
>  scripts/qmp/qom-tree | 2 +-
>  4 files changed, 4 insertions(+), 4 deletions(-)
> 
> diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
> index 007b4cd442..72ccd79330 100755
> --- a/scripts/qmp/qom-get
> +++ b/scripts/qmp/qom-get
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3

Please use #!/usr/bin/env python3 to target the venv python binary when
being used instead of the static system python3 binary.

--js

>  ##
>  # QEMU Object Model test tools
>  #
> diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
> index 03bda3446b..5b8f9fd855 100755
> --- a/scripts/qmp/qom-list
> +++ b/scripts/qmp/qom-list
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  ##
>  # QEMU Object Model test tools
>  #
> diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
> index c37fe78b00..b475e397fc 100755
> --- a/scripts/qmp/qom-set
> +++ b/scripts/qmp/qom-set
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  ##
>  # QEMU Object Model test tools
>  #
> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
> index 1c8acf61e7..86233fa211 100755
> --- a/scripts/qmp/qom-tree
> +++ b/scripts/qmp/qom-tree
> @@ -1,4 +1,4 @@
> -#!/usr/bin/python
> +#!/usr/bin/python3
>  ##
>  # QEMU Object Model test tools
>  #
> 

-- 
—js

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[John Snow]

On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
> Python module structure"). Use the same sys.path modification used
> in the referenced commit to be able to use these scripts again.
> 
> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
> ---
>  scripts/qmp/qmp      | 4 +++-
>  scripts/qmp/qom-fuse | 4 +++-
>  scripts/qmp/qom-get  | 4 +++-
>  scripts/qmp/qom-list | 4 +++-
>  scripts/qmp/qom-set  | 4 +++-
>  scripts/qmp/qom-tree | 4 +++-
>  6 files changed, 18 insertions(+), 6 deletions(-)
> 
> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
> index 0625fc2aba..8e52e4a54d 100755
> --- a/scripts/qmp/qmp
> +++ b/scripts/qmp/qmp
> @@ -11,7 +11,9 @@
>  # See the COPYING file in the top-level directory.
>  
>  import sys, os
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  

Try to avoid using sys.path hacks; they don't work in pylint or mypy and
it provides an active barrier to CQA work here.
(They also tend to be quite fragile.)

We can discuss the right way to do this; one of those ways is to create
an installable package that we can install locally in a virtual environment.

Another way is perhaps to set PYTHONPATH in the calling environment so
that standard "import" directives will work.

Both ultimately involve changing the environment of the user to
accommodate the script.

>  def print_response(rsp, prefix=[]):
>      if type(rsp) == list:
> diff --git a/scripts/qmp/qom-fuse b/scripts/qmp/qom-fuse
> index 6bada2c33d..5fa6b3bf64 100755
> --- a/scripts/qmp/qom-fuse
> +++ b/scripts/qmp/qom-fuse
> @@ -15,7 +15,9 @@ import fuse, stat
>  from fuse import Fuse
>  import os, posix
>  from errno import *
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  
>  fuse.fuse_python_api = (0, 2)
>  
> diff --git a/scripts/qmp/qom-get b/scripts/qmp/qom-get
> index 72ccd79330..59090069dc 100755
> --- a/scripts/qmp/qom-get
> +++ b/scripts/qmp/qom-get
> @@ -13,7 +13,9 @@
>  
>  import sys
>  import os
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  
>  cmd, args = sys.argv[0], sys.argv[1:]
>  socket_path = None
> diff --git a/scripts/qmp/qom-list b/scripts/qmp/qom-list
> index 5b8f9fd855..c5d0c8127d 100755
> --- a/scripts/qmp/qom-list
> +++ b/scripts/qmp/qom-list
> @@ -13,7 +13,9 @@
>  
>  import sys
>  import os
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  
>  cmd, args = sys.argv[0], sys.argv[1:]
>  socket_path = None
> diff --git a/scripts/qmp/qom-set b/scripts/qmp/qom-set
> index b475e397fc..e9d7e0b054 100755
> --- a/scripts/qmp/qom-set
> +++ b/scripts/qmp/qom-set
> @@ -13,7 +13,9 @@
>  
>  import sys
>  import os
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  
>  cmd, args = sys.argv[0], sys.argv[1:]
>  socket_path = None
> diff --git a/scripts/qmp/qom-tree b/scripts/qmp/qom-tree
> index 86233fa211..d96b17256e 100755
> --- a/scripts/qmp/qom-tree
> +++ b/scripts/qmp/qom-tree
> @@ -15,7 +15,9 @@
>  
>  import sys
>  import os
> -from qmp import QEMUMonitorProtocol
> +
> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
> +from qemu.qmp import QEMUMonitorProtocol
>  
>  cmd, args = sys.argv[0], sys.argv[1:]
>  socket_path = None
> 

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>> Python module structure"). Use the same sys.path modification used
>> in the referenced commit to be able to use these scripts again.
>> 
>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>> ---
>>  scripts/qmp/qmp      | 4 +++-
>>  scripts/qmp/qom-fuse | 4 +++-
>>  scripts/qmp/qom-get  | 4 +++-
>>  scripts/qmp/qom-list | 4 +++-
>>  scripts/qmp/qom-set  | 4 +++-
>>  scripts/qmp/qom-tree | 4 +++-
>>  6 files changed, 18 insertions(+), 6 deletions(-)
>> 
>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>> index 0625fc2aba..8e52e4a54d 100755
>> --- a/scripts/qmp/qmp
>> +++ b/scripts/qmp/qmp
>> @@ -11,7 +11,9 @@
>>  # See the COPYING file in the top-level directory.
>>  
>>  import sys, os
>> -from qmp import QEMUMonitorProtocol
>> +
>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>> +from qemu.qmp import QEMUMonitorProtocol
>>  
>
> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
> it provides an active barrier to CQA work here.
> (They also tend to be quite fragile.)
>
> We can discuss the right way to do this; one of those ways is to create
> an installable package that we can install locally in a virtual environment.
>
> Another way is perhaps to set PYTHONPATH in the calling environment so
> that standard "import" directives will work.
>
> Both ultimately involve changing the environment of the user to
> accommodate the script.

For what it's worth, tests/Makefile.involve does the latter for
tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
invocation inconvenient.

Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
finds qmp right in scripts/qmp/.

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[John Snow]

On 4/30/20 1:04 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>>> Python module structure"). Use the same sys.path modification used
>>> in the referenced commit to be able to use these scripts again.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  scripts/qmp/qmp      | 4 +++-
>>>  scripts/qmp/qom-fuse | 4 +++-
>>>  scripts/qmp/qom-get  | 4 +++-
>>>  scripts/qmp/qom-list | 4 +++-
>>>  scripts/qmp/qom-set  | 4 +++-
>>>  scripts/qmp/qom-tree | 4 +++-
>>>  6 files changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>>> index 0625fc2aba..8e52e4a54d 100755
>>> --- a/scripts/qmp/qmp
>>> +++ b/scripts/qmp/qmp
>>> @@ -11,7 +11,9 @@
>>>  # See the COPYING file in the top-level directory.
>>>  
>>>  import sys, os
>>> -from qmp import QEMUMonitorProtocol
>>> +
>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>>> +from qemu.qmp import QEMUMonitorProtocol
>>>  
>>
>> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
>> it provides an active barrier to CQA work here.
>> (They also tend to be quite fragile.)
>>
>> We can discuss the right way to do this; one of those ways is to create
>> an installable package that we can install locally in a virtual environment.
>>
>> Another way is perhaps to set PYTHONPATH in the calling environment so
>> that standard "import" directives will work.
>>
>> Both ultimately involve changing the environment of the user to
>> accommodate the script.
> 
> For what it's worth, tests/Makefile.involve does the latter for
> tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
> invocation inconvenient.
> 
> Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
> finds qmp right in scripts/qmp/.
> 

Yes, using "proper" package hierarchies often means the loss of being
able to invoke the scripts directly, unless you are careful to organize
the package such that the scripts can run both in an "unpackaged" and a
"packaged" mode.

It can be done, but it's tricky and can be prone to error. Let's take a
look at how to do it!

Let's imagine we have an honest-to-goodness QAPI parser module. In
isolation, the layout for such a package would probably look like this:

qapi.git/
  setup.py
  qapi-gen.py
  README.rst
  qapi/
    __init__.py
    parser.py
    schema.py
    ...etc


Now, anything inside of qapi/ is considered the "qapi module" and you
will be unable to directly execute anything inside of this folder,
unless it does not depend on anything else in the "qapi module".

i.e. "import qapi.x" will work, but only from the executing context of a
thread that understands how to find "qapi". If you are IN this
directory, you do not have that context, so those directives will not work.

Python imports are always handled relative to the importing file, not
the imported file.

qapi-gen in the parent directory, however, can use "from qapi import
parser" without any problem, because if you are executing it directly,
it will be able to see the "qapi module" as a folder.

When packaging this for installed environments, there are some changes
we need to make.

(1) Move qapi-gen.py into the module itself! Remove the shebang, the
chmod, and any __main__ scaffolding. Name it something like "script.py".
It should look like this:

```
from qapi import parser, schema

def main():
    print("running qapi-gen!")
```

(2) Create a new shim executable where qapi-gen.py used to be. Give it
chmod u+x and name it qapi-gen.py, and it should look like this:

```
#!/usr/bin/env python3

from qapi import script

if __name__ == '__main__':
    script.main()
```

Now, at this point, you should be able to execute "qapi-gen.py" no
matter what your CWD is, as long as the qapi-gen.py shim is in your PATH.

To finish packaging this for installation purposes, though, we'll amend
setup.py to contain something like this:

setup_kwargs = {
    'entry_points': {
        'console_scripts': [
            'qapi-gen = qapi.script:main',
        ],
    },
}

Next, when you *install* this package, let's say by doing this:

> cd ~/src/qapi.git/

# Make a new virtual environment and enter it as a shell
> pipenv shell

# (If you skip the above step or don't want a venv, do this:)
# pip install --user -e .

# If you are inside the venv, do this:
pip install -e .

(This will install the python package in an "editable" or "develop"
mode, which creates a fake package header that simply redirects to the
real, live files. This means as you edit the files, the python package
is updated in real time. It's a lifesaver trick if you don't know about
it already.)


From here, the "qapi" package is "installed" to your environment. You
should be able to pop open a python3 terminal anywhere and "import qapi"
and have that work.

You should also now have the "qapi-gen" executable in your path (or in
~/.local/bin/ if you are not in a venv) that will invoke the entrypoint
directly.

At this point, both the shim executable and the "installed version" of
the script should work correctly; offering a dual-paradigm package where
you can run and edit it in-tree, or in an installed environment.

It takes a bit more elbow grease, but tools like mypy, pylint and flake8
will better be able to understand and follow the code. You will also not
lose the ability to run scripts quickly in the source tree, but I must
caution that I don't believe this to be a viable strategy long term.

As soon as we have two or more packages that each need "common imports",
it's no longer possible to structure all dependencies as subfolders of
the common scripts, and we lose the ability to perform this "trick".

The only way to accomplish that complex structure is by installing the
packages to the venv; or using PYTHONPATH to amend the search path
outside of the script directly.

--js


P.S.: Of course, I could be wrong. Python packaging changes a lot and
there are a lot of caveats and weird hacks and ways to do almost
anything you want, but it's a matter of what will work well Out Of The
Box with common tooling. For that purpose, I generally want things to
work with pycharm, flake8, mypy, and pylint.

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[Markus Armbruster]

John Snow <jsnow@redhat.com> writes:

> On 4/30/20 1:04 AM, Markus Armbruster wrote:
>> John Snow <jsnow@redhat.com> writes:
>> 
>>> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>>>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>>>> Python module structure"). Use the same sys.path modification used
>>>> in the referenced commit to be able to use these scripts again.
>>>>
>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>> ---
>>>>  scripts/qmp/qmp      | 4 +++-
>>>>  scripts/qmp/qom-fuse | 4 +++-
>>>>  scripts/qmp/qom-get  | 4 +++-
>>>>  scripts/qmp/qom-list | 4 +++-
>>>>  scripts/qmp/qom-set  | 4 +++-
>>>>  scripts/qmp/qom-tree | 4 +++-
>>>>  6 files changed, 18 insertions(+), 6 deletions(-)
>>>>
>>>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>>>> index 0625fc2aba..8e52e4a54d 100755
>>>> --- a/scripts/qmp/qmp
>>>> +++ b/scripts/qmp/qmp
>>>> @@ -11,7 +11,9 @@
>>>>  # See the COPYING file in the top-level directory.
>>>>  
>>>>  import sys, os
>>>> -from qmp import QEMUMonitorProtocol
>>>> +
>>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>>>> +from qemu.qmp import QEMUMonitorProtocol
>>>>  
>>>
>>> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
>>> it provides an active barrier to CQA work here.
>>> (They also tend to be quite fragile.)
>>>
>>> We can discuss the right way to do this; one of those ways is to create
>>> an installable package that we can install locally in a virtual environment.
>>>
>>> Another way is perhaps to set PYTHONPATH in the calling environment so
>>> that standard "import" directives will work.
>>>
>>> Both ultimately involve changing the environment of the user to
>>> accommodate the script.
>> 
>> For what it's worth, tests/Makefile.involve does the latter for
>> tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
>> invocation inconvenient.
>> 
>> Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
>> finds qmp right in scripts/qmp/.
>> 
>
> Yes, using "proper" package hierarchies often means the loss of being
> able to invoke the scripts directly, unless you are careful to organize
> the package such that the scripts can run both in an "unpackaged" and a
> "packaged" mode.
>
> It can be done, but it's tricky and can be prone to error. Let's take a
> look at how to do it!
>
> Let's imagine we have an honest-to-goodness QAPI parser module. In
> isolation, the layout for such a package would probably look like this:
>
> qapi.git/
>   setup.py
>   qapi-gen.py
>   README.rst
>   qapi/
>     __init__.py
>     parser.py
>     schema.py
>     ...etc
>
>
> Now, anything inside of qapi/ is considered the "qapi module" and you
> will be unable to directly execute anything inside of this folder,
> unless it does not depend on anything else in the "qapi module".
>
> i.e. "import qapi.x" will work, but only from the executing context of a
> thread that understands how to find "qapi". If you are IN this
> directory, you do not have that context, so those directives will not work.
>
> Python imports are always handled relative to the importing file, not
> the imported file.
>
> qapi-gen in the parent directory, however, can use "from qapi import
> parser" without any problem, because if you are executing it directly,
> it will be able to see the "qapi module" as a folder.

Hmm...

    $ git-grep '^from.*schema' scripts/
    scripts/qapi-gen.py:from qapi.schema import QAPIError, QAPISchema
    scripts/qapi/events.py:from qapi.schema import QAPISchemaEnumMember
    scripts/qapi/gen.py:from qapi.schema import QAPISchemaVisitor
    scripts/qapi/introspect.py:from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
    scripts/qapi/types.py:from qapi.schema import QAPISchemaEnumMember, QAPISchemaObjectType
    scripts/qapi/visit.py:from qapi.schema import QAPISchemaObjectType

How come importing from qapi. works in scripts/qapi/*.py, too?

[...]

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[John Snow]

On 5/2/20 1:54 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 4/30/20 1:04 AM, Markus Armbruster wrote:
>>> John Snow <jsnow@redhat.com> writes:
>>>
>>>> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>>>>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>>>>> Python module structure"). Use the same sys.path modification used
>>>>> in the referenced commit to be able to use these scripts again.
>>>>>
>>>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>>>> ---
>>>>>  scripts/qmp/qmp      | 4 +++-
>>>>>  scripts/qmp/qom-fuse | 4 +++-
>>>>>  scripts/qmp/qom-get  | 4 +++-
>>>>>  scripts/qmp/qom-list | 4 +++-
>>>>>  scripts/qmp/qom-set  | 4 +++-
>>>>>  scripts/qmp/qom-tree | 4 +++-
>>>>>  6 files changed, 18 insertions(+), 6 deletions(-)
>>>>>
>>>>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>>>>> index 0625fc2aba..8e52e4a54d 100755
>>>>> --- a/scripts/qmp/qmp
>>>>> +++ b/scripts/qmp/qmp
>>>>> @@ -11,7 +11,9 @@
>>>>>  # See the COPYING file in the top-level directory.
>>>>>  
>>>>>  import sys, os
>>>>> -from qmp import QEMUMonitorProtocol
>>>>> +
>>>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>>>>> +from qemu.qmp import QEMUMonitorProtocol
>>>>>  
>>>>
>>>> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
>>>> it provides an active barrier to CQA work here.
>>>> (They also tend to be quite fragile.)
>>>>
>>>> We can discuss the right way to do this; one of those ways is to create
>>>> an installable package that we can install locally in a virtual environment.
>>>>
>>>> Another way is perhaps to set PYTHONPATH in the calling environment so
>>>> that standard "import" directives will work.
>>>>
>>>> Both ultimately involve changing the environment of the user to
>>>> accommodate the script.
>>>
>>> For what it's worth, tests/Makefile.involve does the latter for
>>> tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
>>> invocation inconvenient.
>>>
>>> Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
>>> finds qmp right in scripts/qmp/.
>>>
>>
>> Yes, using "proper" package hierarchies often means the loss of being
>> able to invoke the scripts directly, unless you are careful to organize
>> the package such that the scripts can run both in an "unpackaged" and a
>> "packaged" mode.
>>
>> It can be done, but it's tricky and can be prone to error. Let's take a
>> look at how to do it!
>>
>> Let's imagine we have an honest-to-goodness QAPI parser module. In
>> isolation, the layout for such a package would probably look like this:
>>
>> qapi.git/
>>   setup.py
>>   qapi-gen.py
>>   README.rst
>>   qapi/
>>     __init__.py
>>     parser.py
>>     schema.py
>>     ...etc
>>
>>
>> Now, anything inside of qapi/ is considered the "qapi module" and you
>> will be unable to directly execute anything inside of this folder,
>> unless it does not depend on anything else in the "qapi module".
>>
>> i.e. "import qapi.x" will work, but only from the executing context of a
>> thread that understands how to find "qapi". If you are IN this
>> directory, you do not have that context, so those directives will not work.
>>
>> Python imports are always handled relative to the importing file, not
>> the imported file.
>>
>> qapi-gen in the parent directory, however, can use "from qapi import
>> parser" without any problem, because if you are executing it directly,
>> it will be able to see the "qapi module" as a folder.
> 
> Hmm...
> 
>     $ git-grep '^from.*schema' scripts/
>     scripts/qapi-gen.py:from qapi.schema import QAPIError, QAPISchema
>     scripts/qapi/events.py:from qapi.schema import QAPISchemaEnumMember
>     scripts/qapi/gen.py:from qapi.schema import QAPISchemaVisitor
>     scripts/qapi/introspect.py:from qapi.schema import (QAPISchemaArrayType, QAPISchemaBuiltinType,
>     scripts/qapi/types.py:from qapi.schema import QAPISchemaEnumMember, QAPISchemaObjectType
>     scripts/qapi/visit.py:from qapi.schema import QAPISchemaObjectType
> 
> How come importing from qapi. works in scripts/qapi/*.py, too?
> 
> [...]
> 

Because they're being consumed from the POV of the entry point, which is
scripts/qapi-gen.py.

That is to say, if in scripts/qapi/ you create a new file:

helloworld.py:
```
#!/usr/bin/env python3

from qapi.schema import QAPIError
```

And then execute it:

jsnow@probe ~/s/q/s/qapi ((v5.0.0))> ./helloworld.py
Traceback (most recent call last):
  File "./helloworld.py", line 3, in <module>
    from qapi.schema import QAPIError
ModuleNotFoundError: No module named 'qapi'


It doesn't know what qapi is. Python modules do not carry the
information or metadata themselves inherently to know that they are "in
a module."

That information is known only to qapi-gen.py, and files it brings in
use the namespace set up by qapi-gen.py -- not their own.

This is what I was trying to explain in the last mail about how to lay
out a python module -- and why it's tricky to write imports in such a
way that they preserve "run as script" semantics while also providing
"run as installed package semantics".

Put another way:

qapi-gen.py is considered a /client of/ the qapi module when it is in a
folder above/outside of the QAPI module. If you refactor the module to
include such a script, by moving it into that module's directory -- you
lose your "client" and you no longer have an entry point. (i.e. you
cannot just run the script in the folder any more.)

You can remedy this in several ways:

1. Set PYTHONPATH to include the folder that the `qapi` folder is in, so
that even when executing e.g. somedir/qapi/scripts.py, it knows to look
in "somedir" when evaluating imports.

2. Create a new shim that exists outside of the module proper that does
nothing but import and then execute the script. E.g., "somedir/shim.py"
that just loads the script from qapi/scripts.py and then runs it. This
is basically equivalent to #1, but doesn't require a special ENV for the
user.

3. Use the package installation facilities (setuptools) to define a
"script entry point", which in practice just uses setuptools to
automatically generate a shim for you that gets placed in e.g.
~/.local/bin/ and makes the script available on the command-line.


So I just want to point out that packaging python modules is possible,
and it's a good thing to do, but "runnable scripts" present a problem
WRT lookup paths depending on how they are written!

--js

Re: [PATCH 4/4] scripts/qmp: Fix QEMU Python scripts path

[John Snow]

On 4/30/20 1:04 AM, Markus Armbruster wrote:
> John Snow <jsnow@redhat.com> writes:
> 
>> On 4/21/20 5:42 AM, Philippe Mathieu-Daudé wrote:
>>> QEMU Python scripts have been moved in commit 8f8fd9edba4 ("Introduce
>>> Python module structure"). Use the same sys.path modification used
>>> in the referenced commit to be able to use these scripts again.
>>>
>>> Signed-off-by: Philippe Mathieu-Daudé <f4bug@amsat.org>
>>> ---
>>>  scripts/qmp/qmp      | 4 +++-
>>>  scripts/qmp/qom-fuse | 4 +++-
>>>  scripts/qmp/qom-get  | 4 +++-
>>>  scripts/qmp/qom-list | 4 +++-
>>>  scripts/qmp/qom-set  | 4 +++-
>>>  scripts/qmp/qom-tree | 4 +++-
>>>  6 files changed, 18 insertions(+), 6 deletions(-)
>>>
>>> diff --git a/scripts/qmp/qmp b/scripts/qmp/qmp
>>> index 0625fc2aba..8e52e4a54d 100755
>>> --- a/scripts/qmp/qmp
>>> +++ b/scripts/qmp/qmp
>>> @@ -11,7 +11,9 @@
>>>  # See the COPYING file in the top-level directory.
>>>  
>>>  import sys, os
>>> -from qmp import QEMUMonitorProtocol
>>> +
>>> +sys.path.append(os.path.join(os.path.dirname(__file__), '..', '..', 'python'))
>>> +from qemu.qmp import QEMUMonitorProtocol
>>>  
>>
>> Try to avoid using sys.path hacks; they don't work in pylint or mypy and
>> it provides an active barrier to CQA work here.
>> (They also tend to be quite fragile.)
>>
>> We can discuss the right way to do this; one of those ways is to create
>> an installable package that we can install locally in a virtual environment.
>>
>> Another way is perhaps to set PYTHONPATH in the calling environment so
>> that standard "import" directives will work.
>>
>> Both ultimately involve changing the environment of the user to
>> accommodate the script.
> 
> For what it's worth, tests/Makefile.involve does the latter for
> tests/qapi-schema/test-qapi.py.  Simple enough, but makes manual
> invocation inconvenient.
> 
> Not necessary for scripts/qapi-gen.py, because its "import qmp.FOO"
> finds qmp right in scripts/qmp/.
> 

Yeah, I should be clear here: this is actually kind of a hard thing to
fix tree-wide right now.

What I didn't realize when I was reviewing this patch is that these
imports are *already broken* and these sys.path hacks actually make it
*work again*.

Under that premise, I rescind my feedback, and offer instead:

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

But we should look into how to fix the sys.path problems more long-term;
as part of the packaging work we should investigate this. I believe
similar problems are cropping up in iotests.

Sorry for the noise.

--js