Test under all py2_pickle{1,2,3} and py3_pickle3 ZODB kinds + fix discovered issues

Hello @jerome,

For ZODB/go and WCFS, in order to support Python3 and Python2 with recent
ZODB5, I need to extend generated test databases to cover all ZODB kinds
instead of only ZODB/py2 + pickle protocol=2 we had before. This resulted in
`run_with_all_zodb_pickle_kinds` utility inside gen_testdata.py and zodbtools
also becoming well tested under all kinds of ZODB databases:

    py2: ZODB 4 and ZODB5 < 5.3     (pickle protocol 1)
    py2: ZODB 5.3                   (pickle protocol 2)
    py2: ZODB ≥ 5.4                 (pickle protocol 3)
    py3: ZODB4 and ZODB5            (pickle protocol 3)

Please find the patches that do that for zodbtools attached. Some problems
popped up due to extended testing coverage and they are fixed as well.

Please see individual patches for details.

Kirill

/cc @vnmabus, @levin.zimmermann
/reviewed-by @jerome
/reviewed-on nexedi/zodbtools!29

setup.py

@@ -20,10 +20,10 @@ setup(
     keywords    = 'zodb utility tool',
 
     packages    = find_packages(),
-    install_requires = ['ZODB', 'zodburi', 'zope.interface', 'pygolang >= 0.0.0.dev6', 'six', 'dateparser'],
+    install_requires = ['ZODB', 'zodbpickle', 'zodburi', 'zope.interface', 'pygolang >= 0.0.0.dev6', 'six', 'dateparser'],
 
     extras_require = {
-                  'test': ['pytest', 'freezegun', 'pytz', 'mock;python_version<="2.7"'],
+                  'test': ['pytest', 'freezegun', 'pytz', 'mock;python_version<="2.7"', 'random2', 'ZEO[test]'],
     },
 
     entry_points= {'console_scripts': ['zodb = zodbtools.zodb:main']},

zodbtools/test/conftest.py

-# Copyright (C) 2019  Nexedi SA and Contributors.
-#                     Kirill Smelkov <kirr@nexedi.com>
+# Copyright (C) 2019-2024  Nexedi SA and Contributors.
+#                          Kirill Smelkov <kirr@nexedi.com>
 #
 # This program is free software: you can Use, Study, Modify and Redistribute
 # it under the terms of the GNU General Public License version 3, or (at your
@@ -19,13 +19,58 @@
 
 import pytest
 from zodbtools.test.testutil import zext_supported
+import os
+from os.path import basename, dirname, relpath
+from tempfile import mkdtemp
+from shutil import rmtree
+import pkg_resources
+
+testdir = dirname(__file__)
+
+
+# ztestdata is test fixture to run a test wrt particular ZODB testdata case.
+#
+# It yields all testdata cases generated by gen_testdata.py for both py2 and
+# py3 and all covered ZODB pickle kinds.
+#
+# ztestdata.prefix is where test database and other generated files live.
+# ztestdata.prefix + '/data.fs' , in particular, is the path to test database.
+@pytest.fixture(params=[
+        (name, zext, zkind)
+        # NOTE keep in sync with run_with_all_zodb_pickle_kinds
+        for name    in ('1',)
+        for zext    in (False, True)
+        for zkind   in ('py2_pickle1', 'py2_pickle2', 'py2_pickle3', 'py3_pickle3')
+    ],
+    ids = lambda _: '%s%s/%s' % (_[0], '' if _[1] else '_!zext', _[2]),
+)
+def ztestdata(request): # -> ZTestData
+    name, zext, zkind = request.param
+    _ = ZTestData()
+    _.name  = name
+    _.zext  = zext
+    _.zkind = zkind
+    return _
+
+class ZTestData(object):
+    __slots__ = (
+        'name',
+        'zext',
+        'zkind',
+    )
+
+    @property
+    def prefix(self):
+        _ = '%s/testdata/%s%s/%s' % (testdir, self.name, '' if self.zext else '_!zext', self.zkind)
+        return relpath(_)
+
 
 # zext is a test fixture function object that allows to exercise 2 cases:
 #
 # - when ZODB does not have txn.extension_bytes support
 # - when ZODB might have    txn.extension_bytes support
 #
-# in a test, zext should be used as as follows:
+# in a test, zext should be used as follows:
 #
 #   def test_something(zext):
 #       # bytes for an extension dict
@@ -54,3 +99,97 @@ def zext(request):
         if not zext_supported():
             request.applymarker(pytest.mark.xfail(reason='ZODB does not have txn.extension_bytes support'))
         return _
+
+
+
+# TestZSrv is base class for all test ZODB storages.
+class TestZSrv(object):
+    # .idname    string to use as subtest ID
+    # .zurl      URI to access the storage
+    # .all_logs  returns string with all current server logs
+    # .teardown  should be called when the server is no longer used
+    pass
+
+
+# TestFileStorage provides FileStorage for tests.
+class TestFileStorage(TestZSrv):
+    idname = 'FileStorage'
+    def __init__(self):
+        self.tmpd = mkdtemp('', 'test_filestorage.')
+        self.zurl = '%s/1.fs' % self.tmpd
+
+    def all_logs(self):
+        return "FileStorage: no logs"
+
+    def teardown(self):
+        rmtree(self.tmpd)
+
+
+# TestZEOSrv provides ZEO server for tests.
+class TestZEOSrv(TestZSrv):
+    idname = 'ZEO'
+    def __init__(self):
+        from ZEO.tests import forker
+        self.zeo_forker = forker
+
+        # .z5 represents whether we are running with ZEO5 or earlier
+        dzeo    = pkg_resources.working_set.find(pkg_resources.Requirement.parse('ZEO'))
+        v5      = pkg_resources.parse_version('5.0dev')
+
+        assert dzeo is not None
+        self.z5 = (dzeo.parsed_version >= v5)
+
+        self.tmpd = mkdtemp('', 'test_zeo.')
+        self.log  = '%s/zeo.log' % self.tmpd
+        port  = self.zeo_forker.get_port()
+        zconf = self.zeo_forker.ZEOConfig(('', port), log=self.log)
+        _ = self.zeo_forker.start_zeo_server(path='%s/1.fs' % self.tmpd, zeo_conf=zconf, port=port)
+        if self.z5:
+            self.addr, self.stop = _
+        else:
+            self.addr, self.adminaddr, self.pid, self.path = _
+
+        self.zurl = 'zeo://localhost:%d/' % port
+
+    def all_logs(self):
+        log = '%s:\n\n' % basename(self.log)
+        with open(self.log) as f:
+            log += f.read()
+        return log
+
+    def teardown(self):
+        if self.z5:
+            self.stop()
+        else:
+            self.zeo_forker.shutdown_zeo_server(self.adminaddr)
+            os.waitpid(self.pid, 0)
+
+        rmtree(self.tmpd)
+
+
+# zsrv is test fixture to run a test wrt particular ZODB storage server.
+#
+# It currently yields FileStorage and ZEO.
+#
+# Clients should use zsrv.zurl to connect to the storage.
+# See TestZSrv and its children classes for details.
+@pytest.fixture(params=[TestFileStorage, TestZEOSrv],
+                ids = lambda _: _.idname)
+def zsrv(request): # -> ~TestZSrv
+    nfail = request.session.testsfailed
+    zsrv = request.param()
+
+    yield zsrv
+
+    # see if current test failed
+    # https://stackoverflow.com/a/43268134/9456786
+    failed = False
+    if request.session.testsfailed > nfail:
+        failed = True
+
+    # dump server logs on test failure
+    if failed:
+        print()
+        print(zsrv.all_logs())
+
+    zsrv.teardown()

zodbtools/test/py2py3-venv

+#!/bin/bash -e
+
+# Copyright (C) 2024  Nexedi SA and Contributors.
+#                     Kirill Smelkov <kirr@nexedi.com>
+#
+# This program is free software: you can Use, Study, Modify and Redistribute
+# it under the terms of the GNU General Public License version 3, or (at your
+# option) any later version, as published by the Free Software Foundation.
+#
+# You can also Link and Combine this program with other software covered by
+# the terms of any of the Free Software licenses or any of the Open Source
+# Initiative approved licenses and Convey the resulting work. Corresponding
+# source of such a combination shall include the source code for all other
+# software used.
+#
+# This program is distributed WITHOUT ANY WARRANTY; without even the implied
+# warranty of MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.
+#
+# See COPYING file for full licensing terms.
+# See https://www.nexedi.com/licensing for rationale and options.
+
+# py2py3-venv combines two virtual environments into one.
+#
+# The first virtual environment should be created with python2, and the second with python3.
+# In the destination environment:
+#
+# - program python2 becomes python interpreter with access to eggs from py2 environment.
+# - program python3 becomes python interpreter with access to eggs from py3 environment.
+#
+# Similarly programs pip2 and pip3 refer to pip in py2 and py3 environment correspondingly.
+#
+# Default programs python and pip without version suffix point to python2 and pip2 correspondingly.
+
+if test $# != 3; then
+    echo "Usage: py2py3-venv <py2-venv> <py3-venv> <py2py3-venv>" 1>&2
+    exit 1
+fi
+
+py2_venv="$1"
+py3_venv="$2"
+py2py3_venv="$3"
+
+# die <msg>
+die() {
+    echo "$@" 1>&2; exit 1
+}
+
+test -e "$py2_venv"    || die "E: $py2_venv does not exist"
+test -e "$py3_venv"    || die "E: $py3_venv does not exist"
+test -e "$py2py3_venv" && die "E: $py2py3_venv already exists"
+
+test -e "$py2_venv/bin/python2" || die "E: $py2_venv is not a python2 venv"
+test -e "$py3_venv/bin/python3" || die "E: $py3_venv is not a python3 venv"
+
+py2_venv=$(cd "$py2_venv" && pwd)   # abspath
+py3_venv=$(cd "$py3_venv" && pwd)   # abspath
+
+mkdir "$py2py3_venv"
+
+# python2/python3 do not correctly activate their environments when symlinked
+cat > "$py2py3_venv/python2" <<EOF
+#!/bin/sh
+exec "$py2_venv/bin/python2" "\$@"
+EOF
+cat > "$py2py3_venv/python3" <<EOF
+#!/bin/sh
+exec "$py3_venv/bin/python3" "\$@"
+EOF
+chmod a+x "$py2py3_venv/python2"
+chmod a+x "$py2py3_venv/python3"
+
+# for pip it is ok to symlink as pip itself is a program referring to abspath of underlying python
+ln -s -T "$py2_venv/bin/pip2"    "$py2py3_venv/pip2"
+ln -s -T "$py3_venv/bin/pip3"    "$py2py3_venv/pip3"
+
+# default python / pip
+ln -sT python2 "$py2py3_venv/python"
+ln -sT pip2    "$py2py3_venv/pip"
+
+# env.sh
+cat > "$py2py3_venv/env.sh" <<EOF
+X=\${1:-\${BASH_SOURCE[0]}}       # path to original env.sh is explicitly passed
+X=\$(cd \$(dirname \$X) && pwd)    # when there is other env.sh wrapping us
+
+export PATH="\$X:\$PATH"
+export PS1="(\$(basename \$X)) \$PS1"
+EOF

zodbtools/test/test_analyze.py

@@ -24,9 +24,9 @@ import os.path
 from golang import b
 
 
-def test_zodbanalyze(tmpdir, capsys):
+def test_zodbanalyze(tmpdir, ztestdata, capsys):
     tfs1 = fs1_testdata_py23(tmpdir,
-                    os.path.join(os.path.dirname(__file__), "testdata", "1.fs"))
+                    os.path.join(ztestdata.prefix, "data.fs"))
 
     for use_dbm in (False, True):
         report(
@@ -55,13 +55,11 @@ def test_zodbanalyze(tmpdir, capsys):
         csv=True,
     )
     captured = capsys.readouterr()
-    assert (
-        """Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
-persistent.mapping.PersistentMapping,3,639,22.468354%,213.000000,1,213,2,426
-__main__.Object,65,2205,77.531646%,33.923077,9,310,56,1895
-"""
-        == captured.out
-    )
+
+    with open('%s/zanalyze.csv.ok' % ztestdata.prefix, 'r') as f:
+        zanalyze_csv_ok = f.read()
+
+    assert captured.out == zanalyze_csv_ok
     assert captured.err == ""
 
     # empty range

zodbtools/test/test_commit.py

 # -*- coding: utf-8 -*-
-# Copyright (C) 2018-2022  Nexedi SA and Contributors.
+# Copyright (C) 2018-2024  Nexedi SA and Contributors.
 #                          Kirill Smelkov <kirr@nexedi.com>
 #                          Jérome Perrin <jerome@nexedi.com>
 #
@@ -21,24 +21,24 @@
 
 from zodbtools.zodbcommit import zodbcommit
 from zodbtools.zodbdump import zodbdump, Transaction, ObjectData, ObjectDelete, ObjectCopy
-from zodbtools.util import storageFromURL, sha1
+from zodbtools.util import storageFromURL, sha1, ashex, fromhex
 from ZODB.utils import p64, u64, z64
 from ZODB._compat import BytesIO, dumps, _protocol   # XXX can't yet commit with arbitrary ext.bytes
 
-from tempfile import mkdtemp
-from shutil import rmtree
 from golang import func, defer, b
+from golang.gcompat import qq
+
+import sys
+from subprocess import Popen, PIPE
+
 
 # verify zodbcommit.
 @func
-def test_zodbcommit(zext):
-    tmpd = mkdtemp('', 'zodbcommit.')
-    defer(lambda: rmtree(tmpd))
-
-    stor = storageFromURL('%s/2.fs' % tmpd)
+def test_zodbcommit(zsrv, zext):
+    stor = storageFromURL(zsrv.zurl)
     defer(stor.close)
 
-    head = stor.lastTransaction()
+    at0 = stor.lastTransaction()
 
     # commit some transactions via zodbcommit and verify if storage dump gives
     # what is expected.
@@ -46,7 +46,7 @@ def test_zodbcommit(zext):
         ObjectData(p64(1), b'data1', b('sha1'), sha1(b'data1')),
         ObjectData(p64(2), b'data2', b('sha1'), sha1(b'data2'))])
 
-    t1.tid = zodbcommit(stor, head, t1)
+    t1.tid = zodbcommit(stor, at0, t1)
 
     t2 = Transaction(z64, ' ', b'user2', b'desc2', b'', [
         ObjectDelete(p64(2))])
@@ -55,7 +55,7 @@ def test_zodbcommit(zext):
 
 
     buf = BytesIO()
-    zodbdump(stor, p64(u64(head)+1), None, out=buf)
+    zodbdump(stor, p64(u64(at0)+1), None, out=buf)
     dumped = buf.getvalue()
 
     assert dumped == b''.join([_.zdump() for _ in (t1, t2)])
@@ -71,3 +71,76 @@ def test_zodbcommit(zext):
     data1_3, _, _ = stor.loadBefore(p64(1), p64(u64(t3.tid)+1))
     assert data1_1 == data1_3
     assert data1_1 == b'data1'  # just in case
+
+
+# verify zodbcommit via commandline / stdin.
+def test_zodbcommit_cmd(zsrv, zext):
+    # for ZEO sync is noop unless server_sync=True is specified in options,
+    # but server_sync is available only on ZEO5, not ZEO4. Work it around with
+    # opening/closing the storage on every query.
+    @func
+    def zsrv_do(f):
+        stor = storageFromURL(zsrv.zurl)
+        defer(stor.close)
+        return f(stor)
+    at0 = zsrv_do(lambda stor: stor.lastTransaction())
+
+    # zodbcommit_cmd does `zodb commit` via command line and returns TID of
+    # committed transction.
+    def zodbcommit_cmd(at, stdin): # -> tid
+        p = Popen([sys.executable, '-m', 'zodbtools.zodb', 'commit',
+                   zsrv.zurl, ashex(at)], stdin=PIPE, stdout=PIPE)
+        stdout, _ = p.communicate(stdin)
+        assert p.returncode == 0,  stdout
+        return fromhex(stdout.rstrip())
+
+    t1 = b'''\
+user "user name"
+description "description ..."
+extension %s
+obj 0000000000000001 5 sha1:%s
+data1
+obj 0000000000000002 5 sha1:%s
+data2
+
+''' % (qq(zext(dumps({'a': 'b'}, _protocol))), ashex(sha1(b'data1')), ashex(sha1(b'data2')))
+
+    t2 = b'''\
+user "user2"
+description "desc2"
+extension ""
+obj 0000000000000002 delete
+
+'''
+
+    at1 = zodbcommit_cmd(at0, t1)
+    at2 = zodbcommit_cmd(at1, t2)
+
+    t1 = (b'txn %s " "\n' % ashex(at1)) + t1
+    t2 = (b'txn %s " "\n' % ashex(at2)) + t2
+
+    def _(stor):
+        buf = BytesIO()
+        zodbdump(stor, p64(u64(at0)+1), None, out=buf)
+        return buf.getvalue()
+    dumped = zsrv_do(_)
+
+    assert dumped == b''.join([t1, t2])
+
+    t3 = b'''\
+user "user3"
+description "desc3"
+extension ""
+obj 0000000000000001 from %s
+
+''' % ashex(at1)
+
+    # XXX see note about ObjectCopy in test_zodbcommit
+    at3 = zodbcommit_cmd(at1, t3)
+
+    def _(stor):
+        data1_1, _, _ = stor.loadBefore(p64(1), p64(u64(at1)+1))
+        data1_3, _, _ = stor.loadBefore(p64(1), p64(u64(at3)+1))
+        assert data1_1 == data1_3
+        assert data1_1 == b'data1'  # just in case
+    zsrv_do(_)

zodbtools/test/test_dump.py

 # -*- coding: utf-8 -*-
-# Copyright (C) 2017-2022  Nexedi SA and Contributors.
+# Copyright (C) 2017-2024  Nexedi SA and Contributors.
 #                          Kirill Smelkov <kirr@nexedi.com>
 #                          Jérome Perrin <jerome@nexedi.com>
 #
@@ -28,22 +28,36 @@ from ZODB.FileStorage import FileStorage
 from ZODB.utils import p64
 from io import BytesIO
 
-from os.path import dirname
+from zodbtools.test.testutil import fs1_testdata_py23
+from pytest import mark, raises
+
+from six import PY3
 
-from zodbtools.test.testutil import zext_supported, fs1_testdata_py23
-from pytest import mark, raises, xfail
 
 # verify zodbdump output against golden
 @mark.parametrize('pretty', ('raw', 'zpickledis'))
-def test_zodbdump(tmpdir, zext, pretty):
-    tdir  = dirname(__file__)
-    zkind = '_!zext' if zext.disabled else ''
-    tfs1  = fs1_testdata_py23(tmpdir, '%s/testdata/1%s.fs' % (tdir, zkind))
+def test_zodbdump(tmpdir, ztestdata, pretty):
+    tfs1  = fs1_testdata_py23(tmpdir, '%s/data.fs' % ztestdata.prefix)
     stor  = FileStorage(tfs1, read_only=True)
 
-    with open('%s/testdata/1%s.zdump.%s.ok' % (tdir, zkind, pretty), 'rb') as f:
+    with open('%s/zdump.%s.ok' % (ztestdata.prefix, pretty), 'rb') as f:
         dumpok = f.read()
 
+    # normalize zpickledis.ok to current python:
+    # unicode comes as *UNICODE u'... on py2 and *UNICODE '... on py3
+    # bytes   comes as *BYTES '...    on py2 and *BYTES b'...  on py3
+    if pretty == 'zpickledis':
+        if PY3:
+            dumpok = dumpok.replace(b"UNICODE u'", b"UNICODE '")
+            dumpok = dumpok.replace(b'UNICODE u"', b'UNICODE "')
+            dumpok = dumpok.replace(b"BYTES '",    b"BYTES b'")
+            dumpok = dumpok.replace(b'BYTES "',    b'BYTES b"')
+        else:
+            dumpok = dumpok.replace(b"UNICODE '",  b"UNICODE u'")
+            dumpok = dumpok.replace(b'UNICODE "',  b'UNICODE u"')
+            dumpok = dumpok.replace(b"BYTES b'",   b"BYTES '")
+            dumpok = dumpok.replace(b'BYTES b"',   b'BYTES "')
+
     out = BytesIO()
     zodbdump(stor, None, None, pretty=pretty, out=out)
 

zodbtools/test/test_restore.py

 # -*- coding: utf-8 -*-
-# Copyright (C) 2021-2022  Nexedi SA and Contributors.
+# Copyright (C) 2021-2024  Nexedi SA and Contributors.
 #                          Kirill Smelkov <kirr@nexedi.com>
 #
 # This program is free software: you can Use, Study, Modify and Redistribute
@@ -24,22 +24,16 @@ from zodbtools.zodbrestore import zodbrestore
 from zodbtools.util import storageFromURL, readfile
 from zodbtools.test.testutil import fs1_testdata_py23
 
-from os.path import dirname
-from tempfile import mkdtemp
-from shutil import rmtree
 from golang import func, defer
 
 # verify zodbrestore.
 @func
-def test_zodbrestore(tmpdir, zext):
-    zkind = '_!zext' if zext.disabled else ''
-
-    # restore from testdata/1.zdump.ok and verify it gives result that is
-    # bit-to-bit identical to testdata/1.fs
-    tdata = dirname(__file__) + "/testdata"
+def test_zodbrestore(tmpdir, ztestdata):
+    # restore from zdump.ok and verify it gives result that is
+    # bit-to-bit identical to data.fs
     @func
     def _():
-        zdump = open("%s/1%s.zdump.raw.ok" % (tdata, zkind), 'rb')
+        zdump = open("%s/zdump.raw.ok" % ztestdata.prefix, 'rb')
         defer(zdump.close)
 
         stor = storageFromURL('%s/2.fs' % tmpdir)
@@ -48,6 +42,6 @@ def test_zodbrestore(tmpdir, zext):
         zodbrestore(stor, zdump)
     _()
 
-    zfs1 = readfile(fs1_testdata_py23(tmpdir, "%s/1%s.fs" % (tdata, zkind)))
+    zfs1 = readfile(fs1_testdata_py23(tmpdir, "%s/data.fs" % ztestdata.prefix))
     zfs2 = readfile("%s/2.fs" % tmpdir)
     assert zfs1 == zfs2

zodbtools/test/testdata/.gitignore

-/*.lock
-/*.tmp
-/*.old
+*.lock
+*.tmp
+*.old

zodbtools/test/testdata/1/py2_pickle1/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,648,25.038640%,216.000000,1,216,2,432
+__main__.Object,65,1940,74.961360%,29.846154,9,283,56,1657

zodbtools/test/testdata/1/py2_pickle2/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,639,22.523793%,213.000000,1,213,2,426
+__main__.Object,65,2198,77.476207%,33.815385,9,317,56,1881

zodbtools/test/testdata/1/py2_pickle3/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,597,21.390183%,199.000000,1,199,2,398
+__main__.Object,65,2194,78.609817%,33.753846,9,313,56,1881

zodbtools/test/testdata/1/py3_pickle3/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,795,24.952919%,265.000000,1,265,2,530
+__main__.Object,65,2391,75.047081%,36.784615,9,342,56,2049

zodbtools/test/testdata/1_!zext/py2_pickle1/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,648,25.038640%,216.000000,1,216,2,432
+__main__.Object,65,1940,74.961360%,29.846154,9,283,56,1657

zodbtools/test/testdata/1_!zext/py2_pickle2/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,639,22.523793%,213.000000,1,213,2,426
+__main__.Object,65,2198,77.476207%,33.815385,9,317,56,1881

zodbtools/test/testdata/1_!zext/py2_pickle3/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,597,21.390183%,199.000000,1,199,2,398
+__main__.Object,65,2194,78.609817%,33.753846,9,313,56,1881

zodbtools/test/testdata/1_!zext/py3_pickle3/zanalyze.csv.ok

+Class Name,T.Count,T.Bytes,Pct,AvgSize,C.Count,C.Bytes,O.Count,O.Bytes
+persistent.mapping.PersistentMapping,3,795,24.952919%,265.000000,1,265,2,530
+__main__.Object,65,2391,75.047081%,36.784615,9,342,56,2049

zodbtools/test/testutil.py

-#!/usr/bin/env python
 # -*- coding: utf-8 -*-
 # Copyright (C) 2019-2022  Nexedi SA and Contributors.
 #                          Kirill Smelkov <kirr@nexedi.com>
@@ -72,9 +71,10 @@ def _zext_supported():
 def fs1_testdata_py23(tmpdir, path):
     data  = readfile(path)
     index = readfile(path + ".index")
-    assert data[:4] == b"FS21"      # FileStorage magic for Python2
-    if PY3:
-        data = b"FS30" + data[4:]   # FileStorage magic for Python3
+    head  = data[:4]
+    assert head in (b"FS21", b"FS30")   # FileStorage magics for Python2 and Python3
+    head = b"FS30" if PY3 else b"FS21"
+    data = head + data[4:]
 
     path_ = "%s/%s" % (tmpdir, basename(path))
 

zodbtools/zodbdump.py

@@ -67,11 +67,16 @@ from zodbtools.util import ashex, fromhex, sha1, txnobjv, parse_tidrange, TidRan
         storageFromURL, hashRegistry, asbinstream
 from ZODB._compat import loads, _protocol, BytesIO
 from zodbpickle.slowpickle import Pickler as pyPickler
-import pickletools
 from ZODB.interfaces import IStorageTransactionInformation
 from zope.interface import implementer
 
 import sys
+if sys.version_info.major < 3:
+    from zodbpickle import pickletools_2 as zpickletools
+else:
+    from zodbpickle import pickletools_3 as zpickletools
+
+
 import logging as log
 import re
 from golang.gcompat import qq
@@ -124,7 +129,7 @@ def zodbdump(stor, tidmin, tidmax, hashonly=False, pretty='raw', out=asbinstream
                 out.write(b"extension\n")
                 extf = BytesIO(rawext)
                 disf = StringIO()
-                pickletools.dis(extf, disf)
+                zpickletools.dis(extf, disf)
                 out.write(b(indent(disf.getvalue(), "  ")))
                 extra = extf.read()
                 if len(extra) > 0:
@@ -165,8 +170,8 @@ def zodbdump(stor, tidmin, tidmax, hashonly=False, pretty='raw', out=asbinstream
                         dataf = BytesIO(obj.data)
                         disf  = StringIO()
                         memo = {} # memo is shared in between class and state
-                        pickletools.dis(dataf, disf, memo) # class
-                        pickletools.dis(dataf, disf, memo) # state
+                        zpickletools.dis(dataf, disf, memo) # class
+                        zpickletools.dis(dataf, disf, memo) # state
                         out.write(b(indent(disf.getvalue(), "  ")))
                         extra = dataf.read()
                         if len(extra) > 0:
@@ -265,7 +270,7 @@ def serializeext(ext):
     p = XPickler(buf, _protocol)
     p.dump(ext)
     out = buf.getvalue()
-    #out = pickletools.optimize(out) # remove unneeded PUT opcodes
+    #out = zpickletools.optimize(out) # remove unneeded PUT opcodes
     assert loads(out) == ext
     return out
 
@@ -486,11 +491,19 @@ class Transaction(object):
     # .extension_bytes  bytes       transaction extension
     # .objv             []Object*   objects changed by transaction
     def __init__(self, tid, status, user, description, extension, objv):
-        self.tid                = tid
-        self.status             = status
-        self.user               = user
-        self.description        = description
-        self.extension_bytes    = extension
+        # NOTE we convert fields covered by IStorageTransactionInformation to
+        # exact types specified by that interface to stay 100% compatible with
+        # users of the interface because on py3 e.g. (b' ' == ' ') returns
+        # False and so we need to be careful to provide .status as exactly str
+        # instead bytes, and do the similar for other fields. It also would be
+        # generally incorrect to use bstr/ustr for the fields, because e.g. ZEO
+        # rejects messages with golang.bstr/ustr objects on the basis that they
+        # do not come from allowed list of modules.
+        self.tid                = _exactly_bytes(tid)
+        self.status             = _exactly_str(status)
+        self.user               = _exactly_bytes(user)
+        self.description        = _exactly_bytes(description)
+        self.extension_bytes    = _exactly_bytes(extension)
         self.objv               = objv
 
     # ZODB wants to work with extension as {} - try to convert it on the fly.
@@ -588,3 +601,28 @@ class HashOnly(object):
 
     def __eq__(a, b):
         return isinstance(b, HashOnly) and a.size == b.size
+
+
+# _exactly_bytes returns obj as an instance of exactly type bytes.
+#
+# obj must be initially an instance of bytes, e.g. bstr.
+def _exactly_bytes(obj): # -> bytes
+    assert isinstance(obj, bytes),  type(obj)
+    if type(obj) is not bytes:
+        obj = b(obj)        # bstr
+        obj = obj.encode()  # bytes
+    assert type(obj) is bytes,  type(obj)
+    return obj
+
+
+# _exactly_str returns obj as an instance of exactly type str.
+#
+# obj must be initially an instance of bytes or str/unicode, e.g. bstr or ustr.
+def _exactly_str(obj): # -> str
+    if type(obj) is not str:
+        obj = b(obj)        # bstr
+        obj = obj.encode()  # bytes
+        if str is not bytes:
+            obj = obj.decode('UTF-8')
+    assert type(obj) is str,  type(obj)
+    return obj