Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

test_extremes failing with 64 bit python 3 on Windows #32

Closed
fgregg opened this issue Apr 14, 2016 · 11 comments
Closed

test_extremes failing with 64 bit python 3 on Windows #32

fgregg opened this issue Apr 14, 2016 · 11 comments

Comments

@fgregg
Copy link
Contributor

fgregg commented Apr 14, 2016

https://ci.appveyor.com/project/mgedmin/btrees/build/1.0.6/job/qg5a0mkrfo00jyg2#L51

ERROR: test_extremes (BTrees.tests.test_OLBTree.OLBTreeTest)
----------------------------------------------------------------------
Traceback (most recent call last):
  File "c:\projects\btrees\BTrees\tests\test_OLBTree.py", line 120, in test_extremes
    btree['SMALLEST_64_BITS'] = SMALLEST_64_BITS
OverflowError: Python int too large to convert to C long

All 32 bit versions work fine, 64 bit python 2.7 passes as well. https://ci.appveyor.com/project/mgedmin/btrees/build/1.0.6

@fgregg fgregg changed the title test_extremes failing with 64 bit python 3 test_extremes failing with 64 bit python 3 on Windows Apr 14, 2016
@fgregg fgregg mentioned this issue Apr 14, 2016
@mgedmin
Copy link
Member

mgedmin commented Apr 14, 2016

@tseaver: you wrote that test, do you have any opinions on this?

My gut feeling is that this exposes a real bug in OLBTree implementation: it can hold 64-bit values on 32-bit Windows, but not on 64-bit Windows? Most illogical!

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

This test is skipped for 32 bit architecture https://github.com/zopefoundation/BTrees/blob/master/BTrees/tests/test_OLBTree.py#L112

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

I believe this error is due to a Windows oddity: long is 32-bit on 64-bit Windows.

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

Which is maybe the stupidest design choice I can think of: likely due to some crazy notion of preserving ABI with Windows 95. :(

@fgregg
Copy link
Contributor Author

fgregg commented Apr 14, 2016

That is weird!

@mgedmin
Copy link
Member

mgedmin commented Apr 14, 2016

Shall we declare Win64 to be a 32-bit architecture and skip this test too?

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

What does platform.architecture() return on Win64? [1]

[1] https://docs.python.org/2/library/platform.html#platform.architecture

Maybe we should be using the workaround from that API reference:

def _skip_on_32_bits(wrapped):
    if sys.maxsize == 2**32:
        def _dummy(*args):
            pass
        return _dummy
    return test_method

@mgedmin
Copy link
Member

mgedmin commented Apr 14, 2016

+1 for the sys.maxsize check, but I'm tempted to make it sys.maxsize <= 2**32 -- just in case someone ports Python to a 16-bit machine ;)

tseaver added a commit that referenced this issue Apr 14, 2016
Works around the fact that 'platform.architecture()' reports size of
*pointers*, rather than size of *longs*, which matters on Win64.

Closes: #32.
@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

LOL

@tseaver
Copy link
Member

tseaver commented Apr 14, 2016

I think I have the culprit: the longlong_check function in BTrees.BTreeModuleTemplate was not using PyLong_LongLongOverflow (as is used in longlong_convert). See #35 for my attempt at a fix.

tseaver added a commit that referenced this issue Apr 15, 2016
Replace with 'PyLong_AsLongLongOverflow'.

Toward a fix for #32.
@tseaver
Copy link
Member

tseaver commented Apr 15, 2016

Fixed in #36.

@tseaver tseaver closed this as completed Apr 15, 2016
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants