Commit bedebe22 authored by Kirill Smelkov's avatar Kirill Smelkov

go/zodb/fs1: Fix index save wrt py3

We were saving index data with bytestrings inside index records. This
works for py2 but on py3 it causes failure on unpickling because, by default,
*STRING opcodes are unpickled into unicode on py3:

    === RUN   TestIndexSaveToPy/py2_pickle1/py3
    UnicodeDecodeError: 'ascii' codec can't decode byte 0x8d in position 25: ordinal not in range(128)

    The above exception was the direct cause of the following exception:

    Traceback (most recent call last):
      File "/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/./py/indexcmp", line 43, in <module>
        main()
      File "/home/kirr/src/neo/src/lab.nexedi.com/kirr/neo/go/zodb/storage/fs1/./py/indexcmp", line 30, in main
        d1 = fsIndex.load(path1)
      File "/home/kirr/src/wendelin/z/ZODB/src/ZODB/fsIndex.py", line 138, in load
        v = unpickler.load()
    SystemError: <built-in method read of _io.BufferedReader object at 0x7fb631a86670> returned a result with an error set
        index_test.go:229: zodb/py read/compare index: exit status 1

-> Fix it by explicitly emitting index entries in binary form.

To be able to compare "index from py2 -> loaded into go -> saved by go ->
compared by py3" implement a bit of backward compatibility for  loading py2
index files on py3. Do this compatibility only for known-good py2 files, not
index produced by go code which is loaded by py3 without any compatibility
shims.
parent 228ce24b
...@@ -132,9 +132,10 @@ func (fsi *Index) Save(w io.Writer) (err error) { ...@@ -132,9 +132,10 @@ func (fsi *Index) Save(w io.Writer) (err error) {
if oidPrefix != oidPrefixCur || errStop != nil { if oidPrefix != oidPrefixCur || errStop != nil {
// emit (oid[0:6], oid[6:8]oid[6:8]...pos[2:8]pos[2:8]...) // emit (oid[0:6], oid[6:8]oid[6:8]...pos[2:8]pos[2:8]...)
// suffixes go as bytes so that both py2 and py3 can understand it as binary data
binary.BigEndian.PutUint64(oidb[:], uint64(oidPrefixCur)) binary.BigEndian.PutUint64(oidb[:], uint64(oidPrefixCur))
t[0] = pickle.ByteString(oidb[0:6]) t[0] = pickle.Bytes(oidb[0:6])
t[1] = pickle.ByteString(bytes.Join([][]byte{oidBuf, posBuf}, nil)) t[1] = pickle.Bytes(bytes.Join([][]byte{oidBuf, posBuf}, nil))
err = p.Encode(pickle.Tuple(t[:])) err = p.Encode(pickle.Tuple(t[:]))
if err != nil { if err != nil {
return err return err
......
...@@ -210,9 +210,6 @@ func TestIndexSaveToPy(t *testing.T) { ...@@ -210,9 +210,6 @@ func TestIndexSaveToPy(t *testing.T) {
} }
func _TestIndexSaveToPy(t *testing.T, z *ZTestData) { func _TestIndexSaveToPy(t *testing.T, z *ZTestData) {
xtesting.WithEachPy(t, func(t *testing.T) { xtesting.WithEachPy(t, func(t *testing.T) {
if strings.HasSuffix(t.Name(), "/py3") {
t.Skip("xfail")
}
xtesting.NeedPy(t, "ZODB") xtesting.NeedPy(t, "ZODB")
workdir := xworkdir(t) workdir := xworkdir(t)
...@@ -224,7 +221,9 @@ func _TestIndexSaveToPy(t *testing.T, z *ZTestData) { ...@@ -224,7 +221,9 @@ func _TestIndexSaveToPy(t *testing.T, z *ZTestData) {
} }
// now ask python part to compare testdata and saved-by-us index // now ask python part to compare testdata and saved-by-us index
cmd := exec.Command("./py/indexcmp", z.Path("1.fs.index"), workdir+"/1.fs.index") cmd := exec.Command("./py/indexcmp",
"py2compat:"+z.Path("1.fs.index"), // let py3 load index that FileStorage/py2 saved
workdir+"/1.fs.index") // no py2compat: to verify that what FileStorage/go saved can be loaded as is
cmd.Stdout = os.Stdout cmd.Stdout = os.Stdout
cmd.Stderr = os.Stderr cmd.Stderr = os.Stderr
err = cmd.Run() err = cmd.Run()
...@@ -278,9 +277,6 @@ func BenchmarkIndexSave(b *testing.B) { ...@@ -278,9 +277,6 @@ func BenchmarkIndexSave(b *testing.B) {
ztestdataReg.BenchWithEach(b, _BenchmarkIndexSave) ztestdataReg.BenchWithEach(b, _BenchmarkIndexSave)
} }
func _BenchmarkIndexSave(b *testing.B, z *ZTestData) { func _BenchmarkIndexSave(b *testing.B, z *ZTestData) {
if z.Kind == "py3_pickle3" {
b.Skip("xfail")
}
// FIXME small testdata/1.fs is not representative for benchmarks // FIXME small testdata/1.fs is not representative for benchmarks
index, err := LoadIndexFile(z.Path("1.fs.index")) index, err := LoadIndexFile(z.Path("1.fs.index"))
if err != nil { if err != nil {
......
...@@ -17,18 +17,46 @@ ...@@ -17,18 +17,46 @@
# #
# See COPYING file for full licensing terms. # See COPYING file for full licensing terms.
# See https://www.nexedi.com/licensing for rationale and options. # See https://www.nexedi.com/licensing for rationale and options.
"""compare two ZODB FileStorage v1 index files""" """compare two ZODB FileStorage v1 index files.
If a file is prefixed with py2compat: for that file index loader is adjusted to
handle py2->py3 compatibility by loading *STRING opcodes as bytes on py3.
"""
from __future__ import print_function from __future__ import print_function
from ZODB.fsIndex import fsIndex from ZODB import fsIndex
from golang import func, defer
import sys import sys
@func
def loadFSIndex(path):
# apply py2compat logic
if ':' in path:
mode, path = path.split(':', 1)
if mode != 'py2compat':
raise ValueError('invalid mode %r' % mode)
if sys.version_info.major > 2:
origUnpickler_init = fsIndex.Unpickler.__init__
def xUnpickler_init(self, f):
# ZODB._compat.Unpickler does not expose `encoding` argument
# -> use zodbpickle.Unpickler directly. This is ok to do
# because _compat.Unpickler __init__ is trivial:
# https://github.com/zopefoundation/ZODB/blob/5.8.1-0-g72cebe6bc/src/ZODB/_compat.py#L55-L57
fsIndex.Unpickler.__base__.__init__(self, f, encoding='bytes')
fsIndex.Unpickler.__init__ = xUnpickler_init
def _():
fsIndex.Unpickler.__init__ = origUnpickler_init
defer(_)
return fsIndex.fsIndex.load(path)
def main(): def main():
path1, path2 = sys.argv[1:] path1, path2 = sys.argv[1:]
d1 = fsIndex.load(path1) d1 = loadFSIndex(path1)
d2 = fsIndex.load(path2) d2 = loadFSIndex(path2)
topPos1, fsi1 = d1["pos"], d1["index"] topPos1, fsi1 = d1["pos"], d1["index"]
topPos2, fsi2 = d2["pos"], d2["index"] topPos2, fsi2 = d2["pos"], d2["index"]
......
Markdown is supported
0%
or
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment