Closes #154

Signed-off-by: Javi Merino <javi.merino@cloud.com>
---

Generating coverage data is a bit broken at the moment.  Depending on
the compiler you are using, you would need "coverage: update gcov info
for newer versions of gcc" (Message-Id:
20230902151351.10325-1-javi.merino@cloud.com) which I just sent to the
list. On top of that, you would have to comment out the freeing of the
init sections due to #168 .

I have tested it with these two fixes and the python2 and python3
create the same outputs.

 tools/misc/xencov_split | 17 ++++++++++-------
 1 file changed, 10 insertions(+), 7 deletions(-)

diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
index e4f68ebb6e..8f1271bfe7 100755
--- a/tools/misc/xencov_split
+++ b/tools/misc/xencov_split
@@ -1,5 +1,7 @@
-#!/usr/bin/env python
+#!/usr/bin/env python3
 
+from __future__ import print_function
+from builtins import str
 import sys, os, os.path as path, struct, errno
 from optparse import OptionParser
 
@@ -16,7 +18,7 @@ def xencov_split(opts):
 
     input_file = opts.args[0]
 
-    f = open(input_file)
+    f = open(input_file, "rb")
 
     # Magic number
     s = f.read(4)
@@ -31,9 +33,10 @@ def xencov_split(opts):
     f.close()
 
     while content:
-        off = content.find('\x00')
+        off = content.find(b'\x00')
         fmt = bo_prefix + str(off) + 's'
         fn, = struct.unpack_from(fmt, content)
+        fn = fn.decode('utf-8')
         content = content[off+1:]
 
         fmt = bo_prefix + 'I'
@@ -51,14 +54,14 @@ def xencov_split(opts):
         dir = opts.output_dir + path.dirname(fn)
         try:
             os.makedirs(dir)
-        except OSError, e:
+        except OSError as e:
             if e.errno == errno.EEXIST and os.path.isdir(dir):
                 pass
             else:
                 raise
 
         full_path = dir + '/' + path.basename(fn)
-        f = open(full_path, "w")
+        f = open(full_path, "wb")
         f.write(payload)
         f.close()
 
@@ -89,8 +92,8 @@ def main():
 if __name__ == "__main__":
     try:
         sys.exit(main())
-    except Exception, e:
-        print >>sys.stderr, "Error:", e
+    except Exception as e:
+        print("Error:", e, file=sys.stderr)
         sys.exit(1)
     except KeyboardInterrupt:
         sys.exit(1)
-- 
2.41.0

On Sat, Sep 02, 2023 at 05:21:08PM +0100, Javi Merino wrote:
> diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
> index e4f68ebb6e..8f1271bfe7 100755
> --- a/tools/misc/xencov_split
> +++ b/tools/misc/xencov_split
> @@ -1,5 +1,7 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3

Beside this shebang change, the patch looks good.
With the shebang change reverted: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

Thanks,

-- 
Anthony PERARD

On Mon, Sep 11, 2023 at 11:40:59AM +0100, Anthony PERARD wrote:
> On Sat, Sep 02, 2023 at 05:21:08PM +0100, Javi Merino wrote:
> > diff --git a/tools/misc/xencov_split b/tools/misc/xencov_split
> > index e4f68ebb6e..8f1271bfe7 100755
> > --- a/tools/misc/xencov_split
> > +++ b/tools/misc/xencov_split
> > @@ -1,5 +1,7 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
> 
> Beside this shebang change, the patch looks good.
> With the shebang change reverted: Acked-by: Anthony PERARD <anthony.perard@citrix.com>

In v2 I did not change the shebang:
https://lore.kernel.org/xen-devel/20230905201653.98425-1-javi.merino@cloud.com/

Cheers,
Javi

On 02.09.2023 18:21, Javi Merino wrote:
> Closes #154
> 
> Signed-off-by: Javi Merino <javi.merino@cloud.com>

The title isn't really in line with ...

> --- a/tools/misc/xencov_split
> +++ b/tools/misc/xencov_split
> @@ -1,5 +1,7 @@
> -#!/usr/bin/env python
> +#!/usr/bin/env python3

... this part of the change, and making Py3 a requirement here (assuming
that's indeed necessary) surely wants adding a few words as description.
Grep-ing the tools/ sub-tree I notice that so far we've avoided explicit
uses of "python3", and I assume we would better continue doing so as on
a distro with only Py3 a "python3" alias may legitimately not exist.

Jan

On Mon, Sep 04, 2023 at 08:51:31AM +0200, Jan Beulich wrote:
> On 02.09.2023 18:21, Javi Merino wrote:
> > Closes #154
> > 
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
> 
> The title isn't really in line with ...
> 
> > --- a/tools/misc/xencov_split
> > +++ b/tools/misc/xencov_split
> > @@ -1,5 +1,7 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
> 
> ... this part of the change, and making Py3 a requirement here (assuming
> that's indeed necessary) surely wants adding a few words as description.
> Grep-ing the tools/ sub-tree I notice that so far we've avoided explicit
> uses of "python3", and I assume we would better continue doing so as on
> a distro with only Py3 a "python3" alias may legitimately not exist.

A distro with only Python 3 would still have to provide a python3 binary:

  We expect Unix-like software distributions (including systems like
  macOS and Cygwin) to install the python2 command into the default
  path whenever a version of the Python 2 interpreter is installed,
  and the same for python3 and the Python 3 interpreter.

https://peps.python.org/pep-0394/#for-python-runtime-distributors

Having said that, PEP 394 recommends script publishers to leave the
shebang line as "#!/usr/bin/env python" so I will revert this change in
v2.

Cheers,
Javi

Hi,

On Mon, Sep 04, 2023 at 08:51:31AM +0200, Jan Beulich wrote:
> On 02.09.2023 18:21, Javi Merino wrote:
> > Closes #154
> > 
> > Signed-off-by: Javi Merino <javi.merino@cloud.com>
> 
> The title isn't really in line with ...
> 
> > --- a/tools/misc/xencov_split
> > +++ b/tools/misc/xencov_split
> > @@ -1,5 +1,7 @@
> > -#!/usr/bin/env python
> > +#!/usr/bin/env python3
> 
> ... this part of the change, and making Py3 a requirement here (assuming
> that's indeed necessary) surely wants adding a few words as description.
> Grep-ing the tools/ sub-tree I notice that so far we've avoided explicit
> uses of "python3", and I assume we would better continue doing so as on
> a distro with only Py3 a "python3" alias may legitimately not exist.
> 
> Jan
> 
The only credible reason for doing that would be to have scripts compatible
with both python2 and python3. Python2 has been deprecated since 2020 and
it's no longer security supported[1]. No one should be even remotely
encouraged to use that interpreter. The libs it uses are also in similar
positions. The recommended approach is to migrate as much as possible to
python3 and live happily ever after.

On that topic, the official PEP states:

(From https://peps.python.org/pep-0394/)

  Some Linux distributions will not provide a python command at all by
  default, but will provide a python3 command by default.

Which seems to imply the correct shebang is indeed `/usr/bin/env python3`

Thanks,
Alejandro