| Message ID | 20230902162246.15672-1-javi.merino@cloud.com (mailing list archive) |
|---|---|
| State | Superseded |
| Headers | show |
| Series | tools/misc/xencov_split: Add python 3 compatibility | expand |
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
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
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
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,
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
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)
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(-)